search

bsermons / flycheck-elm

Flycheck support for the elm language
GNU General Public License v3.0
18 stars 8 forks source link

Support no json edge case #14

Closed lookyhooky closed 5 years ago

lookyhooky commented 8 years ago

So I had trouble fitting in flycheck-elm-parse-error-data. This branch uses flycheck-elm-read-json directly in flycheck-elm-parse-errors. So I moved the let expression with (json-array-type 'list) to flycheck-elm-read-json.

In flycheck-elm-parse-errors, first check for valid json if so do everything the normal way. If that case fails, check for elm-make to emit success message, if not decode the plain message for row and column data if available. In some messages the location data is not available so I defaulted to 0,0. Build the message while parsing the data out and then return a list with the single edge case error emitted by elm-make. The last case is no errors, and returns nil.

Oh and I prefixed all the code to handle the edge case under flycheck-elm-plain.

lookyhooky commented 8 years ago

I just noticed a few typos in the comments. Is there some way to update those in this pull request or do I need to close it and open another one?

Okay answered my own question... just push to the same branch.

bsermons commented 8 years ago

Just commit to your branch and the PR will update. Looks like you already figured it out though.

bsermons commented 8 years ago

Sorry for taking so long to get back to this. Trying to test it now but I get the error about "symbol's function definition is void: flycheck-elm-plain-message". I see it being used here but I don't see where it is defined.

Was this renamed to another function?

bsermons commented 8 years ago

Ah I see, looks like it got renamed in this commit but not it's references.

lookyhooky commented 8 years ago

Okay, I've fixed the problem. I was trying to come up with better names for the functions and must not have completed the replacement.

I could also inline the regex defvars and defuns into flycheck-elm-plain-parse-data. It was just easiest to write it with them separate so I could test each piece.

It looks like elm-make is going to fix the inconsistent json output, though it might be a little while.

purcell commented 5 years ago

As of Elm 0.19 this error case works differently, and since this PR doesn't apply cleanly any more, the best choice is probably to skip these changes. The new code for Elm 0.19 support (in #17) doesn't handle non-compilation errors (e.g. missing elm.json files), and that's a separate case that might need to be addressed in due course.