GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-134953: Expand theming for `True`/`False`/`None` by StanFromIreland · Pull Request #135000 · python/cpython

Conversation

@StanFromIreland

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

Uh oh!

There was an error while loading. Please reload this page.

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.

  • Issue: Add more granularity to _pyrepl syntax theming for "keywords" #134953

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
danielhollas and treyhunner reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@StanFromIreland
Expand theme
0602b69
@StanFromIreland
StanFromIreland requested review from ambv, hugovk, lysnikolaou and pablogsal as code owners June 1, 2025 10:19
@bedevere-app bedevere-app Bot added the awaiting review label Jun 1, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jun 1, 2025
Add more granularity to _pyrepl syntax theming for "keywords" #134953
Closed
StanFromIreland
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 StanFromIreland Jun 1, 2025

Copy link
Copy Markdown
Member Author

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@danielhollas danielhollas Jun 1, 2025

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@treyhunner treyhunner Jun 1, 2025

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
danielhollas reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@ambv ambv Sep 15, 2025

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
treyhunner
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 treyhunner Jun 1, 2025

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ambv reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@ambv
Fix blurb
c7468cf
@ambv ambv added the needs backport to 3.14 bugs and security fixes label Sep 15, 2025
@ambv
ambv merged commit a5b9d0b into python:main Sep 15, 2025
@miss-islington-app

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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting review label Sep 15, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 15, 2025
@StanFromIreland @ambv @miss-islington
pythongh-134953: Expand theming for True/False/None (pythonGH-1…
a007a2e 
…35000)

(cherry picked from commit a5b9d0b)

Co-authored-by: Stan Ulbrych <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
@bedevere-app

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Sep 15, 2025
@StanFromIreland
StanFromIreland deleted the constants branch September 15, 2025 15:06
hugovk pushed a commit that referenced this pull request Sep 15, 2025
@miss-islington @StanFromIreland @ambv
[3.14] gh-134953: Expand theming for True/False/None (GH-135000) (
dc4d016 
#138928)

Co-authored-by: Stan Ulbrych <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@StanFromIreland @ambv @treyhunner @danielhollas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.