-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update reporters to (allow) use of end_line
and end_column
#5372
Changes from 5 commits
f395226
09bd841
837b65f
9792d04
45e7bd0
d980fb8
1ed7e17
35fd523
45e0033
0a6e6d6
3c3f141
b24dd1c
6ea66c6
a1bdf7c
6e91225
e280f8c
da4932d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,3 +198,10 @@ Other Changes | |
* Fix crash on ``open()`` calls when the ``mode`` argument is not a simple string. | ||
|
||
Partially closes #5321 | ||
|
||
* Add ability to add ``end_line`` and ``end_column`` to the ``--msg-template`` option. | ||
With the standard ``TextReporter`` this will add the line and column number of the | ||
end of a node to the output of Pylint. If these numbers are unknown, they are represented | ||
by an empty string. | ||
|
||
* Add ``end_line`` and ``end_column`` fields to the output of the ``JSONReporter``. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It look like a breaking change that should wait for #4741. We have a 3.0 alpha branch, might be the time to dust it off ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted! See #5380 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,6 +89,9 @@ class MessageStyle(NamedTuple): | |
"cyan": "36", | ||
"white": "37", | ||
} | ||
PRINT_AS_EMPTY_STRING = {"end_line", "end_column"} | ||
cdce8p marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Set of Message attributes that should be printed as an | ||
empty string when they are None""" | ||
|
||
|
||
def _get_ansi_code(msg_style: MessageStyle) -> str: | ||
|
@@ -189,7 +192,10 @@ def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: | |
|
||
def write_message(self, msg: Message) -> None: | ||
"""Convenience method to write a formatted message with class default template""" | ||
self.writeln(msg.format(self._template)) | ||
self_dict = msg._asdict() | ||
for key in PRINT_AS_EMPTY_STRING: | ||
cdce8p marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self_dict[key] = self_dict[key] or "" | ||
self.writeln(self._template.format(**self_dict)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Traceback (most recent call last):
File "/.../venv-310/bin/pylint", line 33, in <module>
sys.exit(load_entry_point('pylint', 'console_scripts', 'pylint')())
File "/.../pylint/__init__.py", line 24, in run_pylint
PylintRun(sys.argv[1:])
File "/.../pylint/lint/run.py", line 398, in __init__
linter.check(args)
File "/.../pylint/lint/pylinter.py", line 992, in check
self._check_files(
File "/.../pylint/lint/pylinter.py", line 1041, in _check_files
self.add_message(symbol, args=msg)
File "/.../pylint/lint/pylinter.py", line 1513, in add_message
self._add_one_message(
File "/.../pylint/lint/pylinter.py", line 1471, in _add_one_message
self.reporter.handle_message(
File "/.../pylint/reporters/text.py", line 202, in handle_message
self.write_message(msg)
File "/.../pylint/reporters/text.py", line 192, in write_message
self.writeln(msg.format(self._template))
File "/.../pylint/message/message.py", line 94, in format
return template.format(**self._asdict())
KeyError: 'msg1' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't that just raise a new warning? Something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That doesn't really solve the issue. At the core of it its backwards incompatibility. We should be able to use a new template, i.e. with If at some point we add another key, this will only repeat. Every time incompatible with old versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Came up with something. A little hacky, but let me know what you think. I added |
||
|
||
def handle_message(self, msg: Message) -> None: | ||
"""manage message of different type and in the context of path""" | ||
|
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 think we could also change the default for msg-template in 3.0. Do we want the default to have end line and end column ? This seems like something that is useful in an IDE but not a lot for a text output read by humans.
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.
@cdce8p Argued that we might leave the default output as is because of the potential to break many tools. I think this is a discussion which (if we want to have it anyway) we could leave until we are closer to the release of
3.0
. With this merge everybody that wants to can use this new feature!