Skip to content

Conversation

@ambv
Copy link
Contributor

@ambvambv commented May 3, 2025

See issue for design details.

classColorSpan(NamedTuple):
span: Span
tag: _colorize.ColorTag
tag: str
Copy link
ContributorAuthor

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.

Copy link
Member

Choose a reason for hiding this comment

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

:(

ambvand others added 2 commits May 3, 2025 19:26
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Copy link
Member

@hugovkhugovk left a 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.

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@ambvambv requested a review from gaogaotiantian as a code ownerMay 5, 2025 09:24
@hugovkhugovk mentioned this pull request May 5, 2025
Copy link
Member

@pablogsalpablogsal left a 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.

@hugovk
Copy link
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.

Copy link
Member

@tomasr8tomasr8 left a 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 🙂

@ambvambv merged commit f610bbd into python:mainMay 5, 2025
48 checks passed
importsys

fromcollections.abcimportCallable, Iterator, Mapping
fromdataclassesimportdataclass, field, Field
Copy link
Contributor

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. :-(

Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
…GH-133347) Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@ambv@bedevere-bot@hugovk@tomasr8@danielhollas@pablogsal