-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
gh-134953: Expand theming for True/False/None
#135000
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
| class Syntax(ThemeSection): | ||
| prompt: str = ANSIColors.BOLD_MAGENTA | ||
| keyword: str = ANSIColors.BOLD_BLUE | ||
| keyword_constant: str = ANSIColors.BOLD_BLUE |
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.
And so the question is, do we want to change this for pyrepl?
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 agree with the issue OP that the is None in particular being in the same color looks strange.
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.
It could default to ANSIColors.CYAN, since True, False, and None would then be the same color as all the built-ins.
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'm happy letting people customize it but I don't think None/True/False should be highlighted as builtins by default. They can't be accessed by builtins.None (causes a SyntaxError) and can't be overridden (again, a SyntaxError). I would keep the default as is.
The reason you want it highlighted like builtins is because historically they were just default values in the builtin scope, therefore highlighters treated them as such. That's no longer the case, they are actual keywords now.
| @@ -0,0 +1,2 @@ | |||
| Expand ``_colorize`` theme with ``keyword_colorize`` and implement in | |||
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 keyword_colorize was meant to be keyword_constant here.
|
Thanks @StanFromIreland for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…35000) (cherry picked from commit a5b9d0b) Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Łukasz Langa <[email protected]>
|
GH-138928 is a backport of this pull request to the 3.14 branch. |
#138928) Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Łukasz Langa <[email protected]>
I am not a fan of
constant, but it seems to be what Pypygments and Magicpython uses so I guess it is best?As for the other parts of the proposal in the issue I am not as big a fan, but I think this one is fine, and something I myself am used to in editors.