-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fixed regex for Pylama output #18339
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
Conversation
|
I hope they will accept PR soon because it had bothered me too |
karrtikr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Please add a news entry thanking yourself! (see news/README.md for instructions)
| @@ -0,0 +1 @@ | |||
| Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella)) No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this influence mypy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you enable mypy as linters for pylama, it comes without an error code. That is why, in the regex, it is now optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #15609 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry allow to me clarify, here's my understanding:
- MyPy output parsing is handled my the mypy linter.
- mypy is a separate linter (handled by
mypy.tswhich has it's own regex) and Pylama is another linter (handled inpylama.ts), so I'm not sure why changes in pylama linter would affect functioning of another linter.
So this shouldn't influence "mypy output parsing", can you clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use pylama, you can specify multiple external linters with the option --linters=mccabe,mypy,pycodestyle,pylint,isort.
In this case, the output for those linters is picked up by pylama itself and then re-printed unified under the pylama output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, perhaps the following description suits better then:
| Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella)) | |
| Fixes Pylama output parsing with MyPy (thanks [Nicola Marella](https://github.com/nicolamarella)) |
| import { BaseLinter } from './baseLinter'; | ||
| import { ILintMessage, LintMessageSeverity } from './types'; | ||
|
|
||
| const REGEX = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add an example in the comments justifying this regex? Ideally we should be adding tests but seeing as there're currently no tests for this, just the comment would suffice.
| @@ -0,0 +1 @@ | |||
| Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella)) No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, perhaps the following description suits better then:
| Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella)) | |
| Fixes Pylama output parsing with MyPy (thanks [Nicola Marella](https://github.com/nicolamarella)) |
karrtikr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes will be good but not required, so I'll be merging this PR, thanks for contributing!
|
Hey @karrtikr, |
|
Sure, that would be great, just mention me once you are done. |
* Fixed regex for pylama output * added optional error code as it is not reported when using mypy * added news
Closes #15609
Recently I couldn't see errors and warning from pylama linter.
I believe there was a small change in the output format for
parsablein pylama, the regex for parse the output is missing a semicolon. See https://github.com/klen/pylama/blob/227ddc94eb73638d423833075b1af2ac6bb308f7/pylama/main.py#L17https://github.com/klen/pylama/blob/227ddc94eb73638d423833075b1af2ac6bb308f7/pylama/main.py#L17This should fix it.
MyPy output
I added, additionally, the case of mypy output, not having that the error code. See #15609 (comment)