-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-140347: Use pyrepl's colorizer in IDLE #140337
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
base: main
Are you sure you want to change the base?
Conversation
|
@terryjreedy What do you think about this approach in general? It is a cut/paste job with some minor fixup's to get tests passing, so it needs some cleaning up. We could also consider importing duplicated logic. This will also fix the reported issue, and probably a few more. |
|
cc @ambv you may find this use of your code interesting :-) |
|
I need to work on this a bit more, I ran the tests without gui. From manual testing it looks promising. |
|
Down to four errors, three are because the old colorizer would continue on errornous syntax, whereas tokenizing is much stricter. I added a line-by-line fallback to cover some test cases, but I think in general more complex fallbacks (maybe trying a few lines at a time?) are worth it, and so we would need to accept some differences with the old implementation. I will leave this till I hear more from Terry. |
|
|
||
|
|
||
| def gen_colors(buffer: str) -> Iterator[ColorSpan]: | ||
| """Returns a list of index spans to color using the given color tag. |
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 is a generator function, which returns a generator-iterator that yields ColorSpans. The class name need not be repeated.
| """Returns a list of index spans to color using the given color tag. | |
| """Yield spans within buffer to be colored. |
|
General comments belong on the new issue. See questions there. |
|
Manually testing some long files ( |
| recovered = True | ||
| maxpos = max(maxpos, recovered_color.span.end) | ||
|
|
||
| # fall back to trying each line seperetly |
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.
Apart from this section and a few minor modifications above, it is the same logic as in Lib/_pyrepl/utils.py.
| case T.COMMENT: | ||
| span = Span.from_token(token, line_lengths) | ||
| yield ColorSpan(span, "COMMENT") | ||
| # XXX the old colorizer added SYNC on newlines, do we still need this? |
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.
The old colorizer uses SYNC when the user updates a line. The colorizer doesn't re-parse the whole file, only the code up to the closest SYNC tags. Eg. deleting the and from
pass
if x and \
y:
pass
passThe 2 closest SYNC tags are at the end of the first pass and right before the last pass. So IDLE will only try to re-parse from the if token to the newline token after the : (inclusive on both sides).
For that reason, there shouldn't be any SYNC tags inside multi-line strings or line-continuations. How would the tokenizer work if it's passed in a line from the middle of a function body (in terms of indentation errors).
Uh oh!
There was an error while loading. Please reload this page.