-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Include comment text in token #4426
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
Conversation
You use the comment text in some way in Ruff? IIRC CPython doesn't keep the text around but I could be remembering wrong. |
I'm assuming @charliermarsh wants something like the |
Looks like it does:
|
@DimitrisJim I found this.
It looks like cpython only produces tokens for type comments. |
Right now, we use the locations on the token and extract the comments from the source code. It works fine. It'd be convenient to have the comment text directly, and likely more performant for Ruff, but if it's a CPython incompatibility and the RustPython team would prefer not to include it, I'd understand that too. |
@charliermarsh Thanks for the comment! |
https://github.com/python/cpython/blob/72263f2a20002ceff443e3a231c713f2e14fe3fe/Lib/tokenize.py#L17
It's unclear why they made this decision. |
@harupy - Always grateful for all the work you're doing to improve Ruff and RustPython :) |
Oh, that's rough, pun intended. I think its fine if the comment contents are caught since it doesn't change the semantics in any way, its a relatively small change that's easily maintainable and it helps the Ruff team simplify things. |
@DimitrisJim Awesome! I love the pun. |
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.
lgtm, let's let @youknowone also give the ok and merge it.
Thanks @DimitrisJim, grateful for all the collaboration that's happening between these two projects! |
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 incompatibility is not a big deal. Let's go.
RustPython/RustPython#4426 has been merged. We can simplify code using text in comment tokens.
Include comment text in the token for a linter like Ruff which checks comments to ignore errors.