-
Notifications
You must be signed in to change notification settings - Fork 179
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
base: main
Are you sure you want to change the base?
Conversation
@@ -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) |
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.
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.
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 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).
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 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.
I don't want to land this as is, when 3.8 is our minimum targetted version we can revisit |
ce2d3df
to
3583849
Compare
@krassowski 3.8 is our minimum targetted version now. Could you rebase this PR? |
Closes #454.