Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flycheck-coala.el: json -> format output #16

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

fleimgruber
Copy link
Contributor

The --format option is more robust than the --json option when
logging output is mixed with the output of :command;
json-read-from-string breaks in this case.

Fixes #15

:command ("coala" "--json" "--find-config" "--files" source)
:error-parser flycheck-coala-parse-json
:command ("coala" "--format" "{line}:{column}:{severity}:{message}" "--find-config" "--files" source)
:error-patterns ((error (or "None" line) ":" (or "None" column) ":" (any "03") ":" (message))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 03 and not 3?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if 03 for a reason this needs a comment IMO

Copy link
Contributor Author

@fleimgruber fleimgruber Jun 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not use numerical error codes any more, see below.

Copy link
Member

@userzimmermann userzimmermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @fleimgruber :) Thanks for the PR! Always great to see other Emacs enthusiasts in the coala community... I am only wondering about some little things ;)

:command ("coala" "--json" "--find-config" "--files" source)
:error-parser flycheck-coala-parse-json
:command ("coala" "--format" "{line}:{column}:{severity}:{message}" "--find-config" "--files" source)
:error-patterns ((error (or "None" line) ":" (or "None" column) ":" (any "03") ":" (message))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can there really be severity 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not use numerical error codes any more, see below.

:error-parser flycheck-coala-parse-json
:command ("coala" "--format" "{line}:{column}:{severity}:{message}" "--find-config" "--files" source)
:error-patterns ((error (or "None" line) ":" (or "None" column) ":" (any "03") ":" (message))
(warning (or "None" line) ":" (or "None" column) ":" (any "2") ":" (message))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

":2:" should be enough instead of using (any "...")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito

The ``--format`` option is more robust than the ``--json`` option when
logging output is mixed with the output of ``:command``;
``json-read-from-string`` breaks in this case.

Fixes coala#15
@fleimgruber
Copy link
Contributor Author

fleimgruber commented Jun 1, 2017

Thanks for your comments! I changed this to use severity_str and literal matches since these map nicely to flycheck error levels. I also added tests to this, see #11. WDYT?

@userzimmermann
Copy link
Member

ack a9253f1

@userzimmermann
Copy link
Member

@rultor merge

@rultor
Copy link

rultor commented Jun 7, 2017

@rultor merge

@userzimmermann OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit a9253f1 into coala:master Jun 7, 2017
@userzimmermann
Copy link
Member

@fleimgruber Now create the PR for #11 :)

@rultor
Copy link

rultor commented Jun 7, 2017

@rultor merge

@userzimmermann Done! FYI, the full log is here (took me 2min)

@fleimgruber fleimgruber deleted the json_to_format branch June 7, 2017 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants