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

Add end coordinates for column and line number #649

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1270,9 +1270,13 @@ def getDocstring(self, node):
def handleNode(self, node, parent):
if node is None:
return
if self.offset and getattr(node, 'lineno', None) is not None:
node.lineno += self.offset[0]
node.col_offset += self.offset[1]
if self.offset:
if getattr(node, 'lineno', None) is not None:
node.lineno += self.offset[0]
node.col_offset += self.offset[1]
if getattr(node, 'end_lineno', None) is not None:
node.end_lineno += self.offset[0]
node.end_col_offset += self.offset[1]
if self.futuresAllowed and not (isinstance(node, ast.ImportFrom) or
self.isDocstring(node)):
self.futuresAllowed = False
Expand Down
2 changes: 2 additions & 0 deletions pyflakes/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ def __init__(self, filename, loc):
self.filename = filename
self.lineno = loc.lineno
self.col = loc.col_offset
self.end_col = getattr(loc, 'end_col_offset', None)
self.end_lineno = getattr(loc, 'end_lineno', None)
Copy link
Contributor

Choose a reason for hiding this comment

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

As I stated in the issue, I think it would much better to make the default to be equal to the start. That way the type is always an integer, and start = end will typically just do the right thing for most code that would deal with these things.

Copy link
Author

Choose a reason for hiding this comment

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

I disagree. In the use-case of editors when we want to highlight a range with wavy underline, providing start = end will make the underline invisible. start = end might be handled as a special case e.g. when a character is missing (and then denoted with another type of marker like an arrow pointing to that location in the editor). Coercing all missing data to that representation prevents the editors from distinguishing when they need to show the arrow marker and when they need to infer the underline range themselves (e.g default it to the closest token or the entire line).

Copy link
Contributor

Choose a reason for hiding this comment

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

If editors want to show a different marker, can't they just check if start == end?

We could also consider this an off by one type issue where we could make the default to be to select a single character.


def __str__(self):
return '{}:{}:{}: {}'.format(self.filename, self.lineno, self.col+1,
Expand Down
5 changes: 5 additions & 0 deletions pyflakes/test/test_doctests.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@ def doctest_stuff():
''', m.UndefinedName).messages[0]
self.assertEqual(exc.lineno, 5)
self.assertEqual(exc.col, 20)
# end_col_offset and end_lineno are new in Python 3.8
if sys.version_info >= (3, 8):
# check that offsets are also added to end_col and end_lineno
self.assertEqual(exc.end_lineno, 5)
self.assertEqual(exc.end_col, 21)

def test_offsetAfterDoctests(self):
exc = self.flakes('''
Expand Down