-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add support for reading from stdin #2746
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
@Thisch This is very cool, thanks for working on it! Can you add some tests as well? |
d414dc4
to
e96d49d
Compare
pylint/lint.py
Outdated
def _read_stdin(): | ||
# https://mail.python.org/pipermail/python-list/2012-November/634424.html | ||
# FIXME should this try to check the file's declared encoding? | ||
sys.stdin = TextIOWrapper(sys.stdin.detach(), newline=None) |
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.
I don't yet understand why we need this line.
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.
580a862
to
7a967be
Compare
I need a bit your help with fixing the unit tests: Here are the errors reported by travis ci:
Where does this "M...0611" output come from? When I run pylint on a file with the contents "import os" locally, pylint reports
I don't understand why the travis-ci output contains "M...0611" whereas locally I get "W0611". |
@@ -269,3 +269,5 @@ contributors: | |||
* Svetoslav Neykov: contributor | |||
|
|||
* Federico Bond: contributor | |||
|
|||
* Thomas Hisch: contributor |
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 patch is based on the one in #1189, please acknowledge that here too :)
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.
Sure. Thx for pointing that out.
Thanks for working on this! I don't have time to review the patch, but I'm excited to see that this feature will be coming soon ^^ |
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.
Hey @Thisch Took a first look and this is going in the right direction, thanks for the work! I'm not sure yet why it fails on Travis, as I didn't get a chance to see the failure itself. Once the new job fails again, I'll check again.
4890940
to
564105d
Compare
The "M...0611" output is generated by the pytest, so it is not related to an issue in pylint. |
ae1a11e
to
718843f
Compare
@PCManticore when you have time, please take again a look at this PR. I think that it is ready now. |
In the course of adding the testcase a related bug in the --from-stdin code was fixed.
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.
Hey @Thisch Great stuff! Left a final round of comments, once these are addresses we can merge the PR.
if ast_node is not None: | ||
self.file_state = utils.FileState(filepath) | ||
self.check_astroid_module(ast_node, walker, rawcheckers, tokencheckers) | ||
# warn about spurious inline messages handling |
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.
This block seems duplicated with the normal flow, maybe move it after the whole if
branch?
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.
Moving it after the whole if self.config.from_stdin
branch does not work IMO. I think it is not worth the effort to avoid this code duplication, especially since only a couple of lines of code are duplicated.
the failure on travis-ci is unrelated to my change. |
Thank you @Thisch for your contribution! 🎉 This will be part of pylint 2.4. when it gets released. |
@PCManticore Any estimates when we can expect a 2.4 release? What are the open issues that need to be fixed before 2.4 can be relased? |
First of all thx @cpitclaudel for starting with the first draft (#1189) of the Read-from-stdin feature.
This PR is based on the inital PR by @cpitclaudel and tries to incorporate the comment by @PCManticore from #1189. I would like to know from @PCManticore if this PR now goes into the right direction.
I tested this PR by using the following command:
Closes: #1187
Steps
doc/whatsnew/<current release.rst>
.Description
Type of Changes
Related Issue