-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Compatibility: pycode parser: adjust dedent-token handling to maintain py3.12+ compatibility #11440
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
Compatibility: pycode parser: adjust dedent-token handling to maintain py3.12+ compatibility #11440
Conversation
sphinx/pycode/parser.py
Outdated
if end_pos == len(self.buffers) + 1 and end_col == 0: | ||
return True | ||
if end_col < len(self.get_line(end_pos)): | ||
return True |
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 first of these two conditional if
blocks handles implicit dedent tokens that occur when the end-of-file is reached (because any open code blocks should be closed at that point).
I'm less confident in explaining what the second conditional is intended to handle - and it may be possible to simplify the logic used for it.
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 second condition catches dedent tokens that are contained on the same line as some code. For example, similar to the 'compact' test code coverage added:
class A:
def b():
pass
def c():
pass
... we want the definition of b
to omit the line that c
is on. It's a non-empty line, and it emits a dedent token, and it contains a name token.
I think the issue is now solved with python/cpython#105030. At least my latest PR does not fail on 3.12. |
Thanks @picnixz, glad to hear that the tests are passing again. I had a read of that thread, was slightly puzzled because it affected a different platform to our test failures, and then through a linked Either way; closing this as unnecessary. |
Do you want to leave the issue open? (maybe for confirmation?) |
I think we can close it, unless it begins occurring again. Asking for confirmation seems like a good sanity-check though 👍 |
Feature or Bugfix
Purpose
Detail
Relates
Thank you, @mgmacias95 for guidance on the fix.