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

Conversation

krassowski
Copy link

Closes #454.

  • Added offset when handling positions in docstrings (same as for the start coordinates)
  • Added test for the offset handling, conditional on Python >= 3.8\
  • Checked that tests pass locally and no flake8 warnings are introduced

@@ -11,6 +11,8 @@ def __init__(self, filename, loc):
self.filename = filename
self.lineno = loc.lineno
self.col = getattr(loc, 'col_offset', 0)
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.

@asottile
Copy link
Member

I don't want to land this as is, when 3.8 is our minimum targetted version we can revisit

@jayvdb
Copy link
Member

jayvdb commented Feb 14, 2024

@krassowski 3.8 is our minimum targetted version now. Could you rebase this PR?

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.

End column/lineno
4 participants