Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-133346: Make theming support in _colorize extensible#133347

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ambv merged 21 commits intopython:mainfromambv:colorize-theme-support
May 5, 2025

Conversation

ambv
Copy link
Contributor

@ambvambv commentedMay 3, 2025•
edited by bedevere-appbot
Loading

See issue for design details.

@@ -44,7 +49,7 @@ def from_token(cls, token: TI, line_len: list[int]) -> Self:

class ColorSpan(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 added2 commitsMay 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>
@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMay 4, 2025
@hugovkhugovk mentioned this pull requestMay 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 withcolorize.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 withcolorize.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

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 commitf610bbd intopython:mainMay 5, 2025
48 checks passed
import io
import os
import sys

from collections.abc import Callable, Iterator, Mapping
from dataclasses import dataclass, 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 ondataclasses module to all modules that import this. :-(

GalaxySnail reacted with thumbs up emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@hugovkhugovkhugovk approved these changes

@danielhollasdanielhollasdanielhollas left review comments

@pablogsalpablogsalpablogsal approved these changes

@tomasr8tomasr8tomasr8 approved these changes

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@1st11st1Awaiting requested review from 1st11st1 is a code owner

@asvetlovasvetlovAwaiting requested review from asvetlovasvetlov is a code owner

@kumaraditya303kumaraditya303Awaiting requested review from kumaraditya303kumaraditya303 is a code owner

@willingcwillingcAwaiting requested review from willingcwillingc is a code owner

@savannahostrowskisavannahostrowskiAwaiting requested review from savannahostrowskisavannahostrowski is a code owner

@gaogaotiantiangaogaotiantianAwaiting requested review from gaogaotiantiangaogaotiantian is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@ambv@bedevere-bot@hugovk@tomasr8@danielhollas@pablogsal
[8]ページ先頭
©2009-2025 Movatter.jp