Don't include redundant locs in wf-errors

[BenMusch]

Fixes #1436

No tests included because the test suite seems to check that C.wf-errs are thrown, but not their specific contents. Happy to find a way include tests if y'all want them

Example of an error message now:

Well-formedness: (at file:///Users/ben/src/pyret-lang/build/t.arr:2:2-2:9) A standalone variable name probably isn't intentional.

cc @blerner

[blerner]

You missed the other sentence in the bug report, though: "There's a bunch of these ED.cmcode(self.loc) expressions in many of the wf-errors' render-reason methods." Take a quick look to spot other redundant ones, if you can.

[BenMusch]

Will do. Is there an easy way to test render-fancy-reason? I noticed some that appear redundant there, but can't tell if they're potentially used to parse the errors in the UI, so I held off from changing those

[blerner]

That's used within CPO online, rather than at the command line.

…

On Fri, May 24, 2019, 1:04 PM Ben Muschol ***@***.***> wrote: Will do. Is there an easy way to test render-fancy-reason? I noticed some that appear redundant there, but can't tell if they're potentially used to parse the errors in the UI, so I held off from changing those — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1451?email_source=notifications&email_token=AAHAHQGG33UVFF36MA63FFLPXAN3TA5CNFSM4HPHNTIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWF7M7Y#issuecomment-495711871>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHAHQC6YMU4GNMH2FXKW7TPXAN3TANCNFSM4HPHNTIA> .

[BenMusch]

As far as I can tell, all of the remaining duplicate loc's are in the render-fancy-reason method, so I tested code.pyret.org locally with the duplicate loc's removed and it seems to change the behavior substantially. A local version running my pyret is on the left, production version with both loc's is on the right:

Maybe it's worth keeping the duplicate locs since it seems like it makes the UI nicer? At least in the context of larger programs, the code block display seems handy

[jswrenn]

Ideally we would use cmcode in the render-reason methods, too, and the CLI would display code snippets there too. Unfortunately, CLI error display rendering system doesn't any way to render snippets and it didn't seem trivial to add last time I looked into it, iirc.

In the absence of such a mechanism, any appearance of cmcode in render-reason (but not render-fancy-reason) should be regarded as a bug.

[safwansamsudeen]

Why wasn't this merged?