-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-113547 cases generator lexer: dedent > 0 + escape sequence #113546
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
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
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 changes for escape sequences look fine, but it could do with a few tests.
The changes to to_text could be simplified and it also needs a test or two.
Tools/cases_generator/lexer.py
Outdated
| temp: list[str] = [] | ||
| for line in text.split("\n"): | ||
| leading_space = len(line) - len(line.lstrip()) | ||
| if leading_space > dedent: |
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.
Is the regex necessary? Would line = line[min(leading_space, dedent):] work?
|
Added tests in Lib/test/test_tools + simplified the de-indentation as you mentioned |
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 a better name and more general name is: test_lexer.py and then the two short files can be merged.
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.
done!
Addresses TODOs
char now describe escape sequences and positive dedent for multi lines comments is supported in lexer