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-133722: Add Difflib theme to_colorize and 'color' option todifflib.unified_diff#133725

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

Open
dougthor42 wants to merge21 commits intopython:main
base:main
Choose a base branch
Loading
fromdougthor42:difflib-color-gh133722

Conversation

dougthor42
Copy link
Contributor

@dougthor42dougthor42 commentedMay 9, 2025•
edited by github-actionsbot
Loading

Add acolor arg (defaulting toFalse) todifflib.unified_diff. WhenTrue, ANSI color codes are injected to the diff lines so that the printed result looks likegit diff --color:

>>>importdifflib>>>args= [['one','three'], ['two','three'],'Original','Current']>>>forlineindifflib.unified_diff(*args,lineterm='',color=True):...print(line)...---Original+++Current@@-1,2+1,2 @@-one+twothree

image

Fixes#133722.

It's a bummer, Ijust missed the feature freeze window for 3.14 (it was yesterday! 😭), but c'est la vie!

📚 Documentation preview 📚:https://cpython-previews--133725.org.readthedocs.build/

@dougthor42dougthor42 marked this pull request as ready for reviewMay 9, 2025 03:40
@tomasr8
Copy link
Member

Thanks for the PR!

With the basic theming support in#133347 merged, perhaps we could define a separate difflib theme and use that here?

@hugovk
Copy link
Member

Thanks for the PR! I had a play with something similar but didn't get very far before the 3.14 freeze.

With the basic theming support in#133347 merged, perhaps we could define a separate difflib theme and use that here?

Yes, let's add theming support.

Seethis docstring for instructions.

For a couple of standalone examples of adding theme support to a module, see230d658 and5c5c3e1 from that PR.

@dougthor42
Copy link
ContributorAuthor

Ah, TIL! Thanks, I'll take a look at those docs and update accordingly. It'll be a few days a least before I can get to it, so for now I'll revert this back to draft.

tomasr8 and hugovk reacted with thumbs up emoji

@dougthor42dougthor42 marked this pull request as draftMay 9, 2025 17:14
@hugovk
Copy link
Member

Absolutely no rush for this, we have 12 months until the 3.15 feature freeze. Don't hesitate if you have questions :)

Copy link
ContributorAuthor

@dougthor42dougthor42 left a comment

Choose a reason for hiding this comment

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

Wow that was really easy! The examples helped a lot too, thanks! PTAL at your leisure.

@dougthor42dougthor42 marked this pull request as ready for reviewMay 11, 2025 04:38
@dougthor42dougthor42 changed the titlegh-133722: Add 'color' option to 'difflib.unified_diff'gh-133722: Add Difflib theme to_colorize and 'color' option todifflib.unified_diffMay 11, 2025
Comment on lines 215 to 217
equal: str = ANSIColors.RESET # context lines
insert: str = ANSIColors.GREEN
delete: str = ANSIColors.RED
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I opted to use thedifflib.context_diff internal terminology here. Should I use the more git-like termscontext,added, andremoved instead?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, maybe the Git-like terms if they're going to be more familiar with people and if thedifflib ones are all internal.

Where does Git mention them?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Overall they are defined in theGNU unified diff detailed description.

Thegit diff man page mentions them in a variety of places (emphasis mine):

-U
--unified=

Generate diffs with lines ofcontext instead of the usual three. Implies --patch.

Generating patch text with -p

  1. Hunk headers mention the name of the function to which thehunk applies.

plain

Any line that isadded in one location and wasremoved in another location will be colored withcolor.diff.newMoved.

Though sometimes it uses "deleted" instead of "removed":

--numstat

Similar to --stat, but shows number ofadded anddeleted lines

I've gone ahead and switched to the GNU unified diff terms prior to the requested sorting. Also, to keep consistent with the other dataclass attributes (which are not currently sorted), thereset value was left at the end.

Copy link
Member

@ZeroIntensityZeroIntensity left a comment

Choose a reason for hiding this comment

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

Hm, I would expect to see this in the difflib CLI rather than a library function. But, as it turns out, difflib doesn't have a working CLI? That's different than what'sdocumented.

I personally think it's worth adding one if we're going through the effort to add colorization.

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.

Let's also add a What's New entry, perhaps something similar tohttps://docs.python.org/3.14/whatsnew/3.14.html#whatsnew314-color-argparse

Comment on lines 215 to 217
equal: str = ANSIColors.RESET # context lines
insert: str = ANSIColors.GREEN
delete: str = ANSIColors.RED
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, maybe the Git-like terms if they're going to be more familiar with people and if thedifflib ones are all internal.

Where does Git mention them?

Comment on lines 229 to +232
syntax: Syntax = field(default_factory=Syntax)
traceback: Traceback = field(default_factory=Traceback)
unittest: Unittest = field(default_factory=Unittest)
difflib: Difflib = field(default_factory=Difflib)
Copy link
Member

Choose a reason for hiding this comment

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

And here:

Suggested change
syntax:Syntax=field(default_factory=Syntax)
traceback:Traceback=field(default_factory=Traceback)
unittest:Unittest=field(default_factory=Unittest)
difflib:Difflib=field(default_factory=Difflib)
difflib:Difflib=field(default_factory=Difflib)
syntax:Syntax=field(default_factory=Syntax)
traceback:Traceback=field(default_factory=Traceback)
unittest:Unittest=field(default_factory=Unittest)

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 very slightly concerned about sorting when the dataclasses aren'tkw_only.

Should I update the dataclass decorator to includekw_only=True and then sort? Or is that out of scope of this PR?

Copy link
Member

Choose a reason for hiding this comment

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

Let's ask@ambv about this.

@@ -0,0 +1,2 @@
Added a ``color`` option to :func:`difflib.unified_diff` that injects ANSI color
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Added a``color`` option to:func:`difflib.unified_diff` that injects ANSI color
Added a*color* option to:func:`difflib.unified_diff` that injects ANSI color

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

Still todo :)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Oops!

Copy link
ContributorAuthor

@dougthor42dougthor42 left a comment•
edited
Loading

Choose a reason for hiding this comment

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

What's New entry

Added, assuming a 3.15 release.

I would expect to see this in the difflib CLI rather than a library function.

My underlying use case and request is specific to the library function. That said, I agree that any CLI should also include coloring.

difflib doesn't have a working CLI? That's different than what's documented.

Hmm... I'm not able to find the difflib CLI docs? The closest thing I found washttps://docs.python.org/3/library/difflib.html#difflib-interface but that just looks like an example of how to make a CLI for difflib. Was that what you were thinking of?

If Iactually read the comment I would have seen that the docs were indeed linked... And yes, the linked item is just an example of how one might make a CLI fordifflib.

I personally think it's worth adding one if we're going through the effort to add colorization.

I'd be happy to modify any existing CLI code to include colors. I can't say I'd have the time toadd a CLI though.

Comment on lines 215 to 217
equal: str = ANSIColors.RESET # context lines
insert: str = ANSIColors.GREEN
delete: str = ANSIColors.RED
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Overall they are defined in theGNU unified diff detailed description.

Thegit diff man page mentions them in a variety of places (emphasis mine):

-U
--unified=

Generate diffs with lines ofcontext instead of the usual three. Implies --patch.

Generating patch text with -p

  1. Hunk headers mention the name of the function to which thehunk applies.

plain

Any line that isadded in one location and wasremoved in another location will be colored withcolor.diff.newMoved.

Though sometimes it uses "deleted" instead of "removed":

--numstat

Similar to --stat, but shows number ofadded anddeleted lines

I've gone ahead and switched to the GNU unified diff terms prior to the requested sorting. Also, to keep consistent with the other dataclass attributes (which are not currently sorted), thereset value was left at the end.

Comment on lines 229 to +232
syntax: Syntax = field(default_factory=Syntax)
traceback: Traceback = field(default_factory=Traceback)
unittest: Unittest = field(default_factory=Unittest)
difflib: Difflib = field(default_factory=Difflib)
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 very slightly concerned about sorting when the dataclasses aren'tkw_only.

Should I update the dataclass decorator to includekw_only=True and then sort? Or is that out of scope of this PR?

@@ -0,0 +1,2 @@
Added a ``color`` option to :func:`difflib.unified_diff` that injects ANSI color
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Done

@dougthor42
Copy link
ContributorAuthor

Hmm... I'm not able to find the difflib CLI docs? The closest thing I found

@ZeroIntensity Doh! I just re-read your comment and realized you linked to it haha.

Anyway yeah that's just example code. To my knowledge there is no official CLI fordifflib.

Comment on lines 229 to +232
syntax: Syntax = field(default_factory=Syntax)
traceback: Traceback = field(default_factory=Traceback)
unittest: Unittest = field(default_factory=Unittest)
difflib: Difflib = field(default_factory=Difflib)
Copy link
Member

Choose a reason for hiding this comment

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

Let's ask@ambv about this.

@@ -0,0 +1,2 @@
Added a ``color`` option to :func:`difflib.unified_diff` that injects ANSI color
Copy link
Member

Choose a reason for hiding this comment

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

Still todo :)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@hugovkhugovkhugovk left review comments

@ZeroIntensityZeroIntensityZeroIntensity left review comments

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add acolor: bool arg todifflib.unified_diff
4 participants
@dougthor42@tomasr8@hugovk@ZeroIntensity
[8]ページ先頭
©2009-2025 Movatter.jp