FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-134953: Expand theming for `True`/`False`/`None` by StanFromIreland · Pull Request #135000 · python/cpython · GitHub

/ cpython Public

gh-134953: Expand theming for True/False/None - #135000

Merged
ambv merged 2 commits into
python:mainfrom
StanFromIreland:constants
Sep 15, 2025
ConversationCommits (2)ChecksFiles changed
Merged

gh-134953: Expand theming for True/False/None#135000
ambv merged 2 commits into
python:mainfrom
StanFromIreland:constants

Conversation

StanFromIreland commented Jun 1, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

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.

bedevere-app Bot mentioned this pull request Jun 1, 2025
Closed
StanFromIreland commented Jun 1, 2025
View reviewed changes
Comment thread Lib/_colorize.py
class Syntax(ThemeSection):
prompt: str = ANSIColors.BOLD_MAGENTA
keyword: str = ANSIColors.BOLD_BLUE
keyword_constant: str = ANSIColors.BOLD_BLUE

StanFromIreland Jun 1, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

And so the question is, do we want to change this for pyrepl?

danielhollas Jun 1, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I agree with the issue OP that the is None in particular being in the same color looks strange.

treyhunner Jun 1, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It could default to ANSIColors.CYAN, since True, False, and None would then be the same color as all the built-ins.

ambv Sep 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

treyhunner reviewed Jun 1, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2025-06-01-11-14-00.gh-issue-134953.ashdfs.rst Outdated
@@ -0,0 +1,2 @@
Expand ``_colorize`` theme with ``keyword_colorize`` and implement in

treyhunner Jun 1, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think keyword_colorize was meant to be keyword_constant here.

ambv added the needs backport to 3.14 bugs and security fixes label Sep 15, 2025
ambv merged commit a5b9d0b into python:main Sep 15, 2025
55 checks passed

miss-islington-app Bot commented Sep 15, 2025

Copy link
Copy Markdown

Thanks @StanFromIreland for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 15, 2025
pythongh-134953: Expand theming for True/False/None (pythonGH-1…
a007a2e 
…35000)

(cherry picked from commit a5b9d0b)

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>

bedevere-app Bot commented Sep 15, 2025

Copy link
Copy Markdown

GH-138928 is a backport of this pull request to the 3.14 branch.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Sep 15, 2025
StanFromIreland deleted the constants branch September 15, 2025 15:06
hugovk pushed a commit that referenced this pull request Sep 15, 2025
[3.14] gh-134953: Expand theming for True/False/None (GH-135000) (
dc4d016 
#138928)

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

hugovk Awaiting requested review from hugovk hugovk is a code owner
pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
ambv Awaiting requested review from ambv ambv is a code owner
2 more reviewers
treyhunner treyhunner left review comments
danielhollas danielhollas left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL