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

hadolint: multiple fixes and adapt expected format #3678

Merged
merged 1 commit into from
Jul 5, 2021

Conversation

m-ildefons
Copy link
Contributor

  • Disable color output, so severity detection works again
  • Match on the correct filename
  • Show hadolint rule number in vim gutter in addition to ALEDetails
  • Capture and show error in case of syntax errors
  • Add tests for error capture
  • Adapt existing tests

39d22ad changed the file name, when reading from stdin from /dev/stdin
to -. This broke integration with ALE.
c00fc5c colorized the output to tty by default and added the flag
--no-color to disable it again. This broke detecting issue severity.

fixes: #2333
fixes: #958

- Show hadolint rule number in vim gutter in addition to `ALEDetails`
- Capture and show error in case of syntax errors
- Add tests for error capture
- Adapt existing tests

fixes: dense-analysis#2333
fixes: dense-analysis#958
@m-ildefons
Copy link
Contributor Author

@infokiller beat me to fixing the color output and input file name in #3680. But this commit still fixes the other issues like showing the code in the vim gutter and test coverage.

@infokiller
Copy link
Contributor

@m-ildefons sorry I missed your PR. As I mentioned in the other one, there is now also json output which will make parsing much easier, but I'm not sure if it should be used since it can break ALE with older versions of hadolint that don't support it.

@m-ildefons
Copy link
Contributor Author

No problem 😄 . Considering that switching to the json output basically involves writing the plugin and its tests from scratch, I don't think its worth the effort right now. This may change if there ever needs to be a fix done to this again.

Copy link

@ddnomad ddnomad left a comment

Choose a reason for hiding this comment

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

Just testing this one locally. It works fine.

Any idea when this will be merged? Really helpful :)

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Looks good to me and seems to be working for some.

@hsanson hsanson merged commit 958f30c into dense-analysis:master Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hadolint: Show rule ID Parse and report syntax errors for hadolint
4 participants