Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

StanFromIreland
Copy link
Contributor

@StanFromIreland StanFromIreland commented Jun 1, 2025

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.

@@ -176,6 +176,7 @@ class Argparse(ThemeSection):
class Syntax(ThemeSection):
prompt: str = ANSIColors.BOLD_MAGENTA
keyword: str = ANSIColors.BOLD_BLUE
keyword_constant: str = ANSIColors.BOLD_BLUE
Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Member

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.

@@ -0,0 +1,2 @@
Expand ``_colorize`` theme with ``keyword_colorize`` and implement in
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants