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-133346: Make theming support in _colorize extensible by ambv · Pull Request #133347 · python/cpython

Conversation

@ambv

@ambv ambv commented May 3, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

See issue for design details.

  • Issue: Theming support for _colorize #133346

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ambv
pythongh-133346: Make theming support in _colorize extensible
3344008
@ambv
ambv requested review from hugovk, lysnikolaou and pablogsal as code owners May 3, 2025 15:15
@bedevere-app bedevere-app Bot mentioned this pull request May 3, 2025
Theming support for _colorize #133346
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label May 3, 2025
ambv
ambv commented May 3, 2025
View reviewed changes
Comment thread Lib/_pyrepl/utils.py
class ColorSpan(NamedTuple):
span: Span
tag: _colorize.ColorTag
tag: str

@ambv ambv May 3, 2025

Copy link
Copy Markdown
Contributor 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.

I'm a little sad about this but currently Python typing cannot construct a type that is "a set of string literals from this type's attributes" and I didn't feel like repeating myself in _colorize.

Sorry, something went wrong.

Uh oh!

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

@pablogsal pablogsal May 5, 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.

:(

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
ambv added 2 commits May 3, 2025 17:17
@ambv
Fix lint
68e2385
@ambv
Add theming to traceback.py
252dfa0
@ambv
ambv requested review from 1st1, asvetlov, kumaraditya303 and willingc as code owners May 3, 2025 16:42
ambv
ambv commented May 3, 2025
View reviewed changes
Comment thread Lib/_pyrepl/reader.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
hugovk
hugovk reviewed May 3, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
ambv and others added 2 commits May 3, 2025 19:26
@ambv
Fix WASI test failures
b29c9b9
@ambv @hugovk
Apply suggestions from code review
ca34939 
Co-authored-by: Hugo van Kemenade <[email protected]>
hugovk
hugovk reviewed May 4, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
hugovk and others added 7 commits May 4, 2025 18:49
@hugovk @ambv
Add theming to unittest
5c5c3e1
@ambv
Add missing plumbing for _colorize.Unittest
f449e7b
@ambv
Adapt tests to theming when executed with -j
8da314f
@ambv
Fix. lint.
65d3a78
@ambv
Add blurb and some docstrings
0866fd8
@ambv
FIX. LINT.
965c7cf
@hugovk @ambv
Add theming to argparse
230d658
@ambv
ambv requested a review from savannahostrowski as a code owner May 4, 2025 18:30
hugovk
hugovk approved these changes May 4, 2025
View reviewed changes

@hugovk hugovk left a comment

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.

Thanks, it's much nicer to specify semantic meanings rather than colours in the code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels May 4, 2025
@ambv @hugovk
Apply suggestions from code review
c4808c8 
Co-authored-by: Hugo van Kemenade <[email protected]>
@ambv
Rename REPL to Syntax now that pdb is using it, too; adapt pdb tests
e67fda9
@ambv
ambv requested a review from gaogaotiantian as a code owner May 5, 2025 09:24
@ambv
Bring json.tool into the theming fold
70ca161
@hugovk hugovk mentioned this pull request May 5, 2025
Colorize sqlite3 CLI #133447
Closed
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_pyrepl/utils.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/json/tool.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/test/test_pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/test/test_traceback.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal approved these changes May 5, 2025
View reviewed changes

@pablogsal pablogsal left a comment

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.

This LGTM I left some comments and questions but nothing blocking.

I have a request though: I think we should explore offering some semi-final non-experimental way to support this in 3.14 with proper documentation. If @hugovk is fine with this, I would like to propose a discussion in an issue and maybe with the SC. I don't particularly thing this needs a PEP by any means (my personal opinion non SC hat on) but I think that releasing 3.14 with colorize.set_theme() with the docs being a comment is not great and maybe it is worth an exception.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_colorize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Lib/test/test_traceback.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed May 5, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-05-04-19-46-14.gh-issue-133346.nRXi4f.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ambv
Merge branch 'main' into colorize-theme-support
3166d63
@hugovk

hugovk commented May 5, 2025

Copy link
Copy Markdown
Member

I have a request though: I think we should explore offering some semi-final non-experimental way to support this in 3.14 with proper documentation. If @hugovk is fine with this, I would like to propose a discussion in an issue and maybe with the SC. I don't particularly thing this needs a PEP by any means (my personal opinion non SC hat on) but I think that releasing 3.14 with colorize.set_theme() with the docs being a comment is not great and maybe it is worth an exception.

Fine by me.
ambv reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ambv added 3 commits May 5, 2025 22:51
@ambv
Make no_colors() into classmethods
5c45ddb
@ambv
Address review
68a5c44
@ambv
Group effort
9f51769
tomasr8
tomasr8 approved these changes May 5, 2025
View reviewed changes

@tomasr8 tomasr8 left a comment

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.

Exciting stuff! Json changes look good 🙂

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_json/test_tool.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ambv
ambv merged commit f610bbd into python:main May 5, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label May 5, 2025
danielhollas
danielhollas reviewed May 5, 2025
View reviewed changes
Comment thread Lib/_colorize.py
import sys

from collections.abc import Callable, Iterator, Mapping
from dataclasses import dataclass, field, Field

@danielhollas danielhollas May 5, 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.

It's a little unfortunate for import times that this will bring a dependency on dataclasses module to all modules that import this. :-(

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
GalaxySnail reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
This was referenced May 6, 2025
gh-133447: Add basic color to sqlite3 CLI #133461
Merged
gh-133722: Add Difflib theme to _colorize and 'color' option to difflib.unified_diff #133725
Merged
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@ambv @hugovk @Pranjal095
pythongh-133346: Make theming support in _colorize extensible (python…
6048ede 
…GH-133347)

Co-authored-by: Hugo van Kemenade <[email protected]>
@hugovk hugovk mentioned this pull request Apr 27, 2026
gh-149026: Add colour to pickletools CLI output #149027
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk approved these changes
@pablogsal pablogsal pablogsal approved these changes
@tomasr8 tomasr8 tomasr8 approved these changes
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner
@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski savannahostrowski is a code owner
@gaogaotiantian gaogaotiantian Awaiting requested review from gaogaotiantian gaogaotiantian is a code owner
+1 more reviewer
@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.

6 participants

@ambv @bedevere-bot @hugovk @tomasr8 @danielhollas @pablogsal
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.