Git diff coloring #11590

gerhardol · 2024-02-15T23:12:04Z

Fixes #11464
Fixes #11517

Proposed changes

TBD, opened to get feedback.
Description will be improved too, as well as screenshots.

--
FileViewer: Restructure HighlightService

Move handling of "normal" patches from the shared
DiffHighlightService to PatchHighlightService.

Various code cleanups.

This commit is a preparation for Git coloring and git-grep.

--
Git diff coloring

Use the coloring by git-diff instead of GE internal algorithm,
by using Git ANSI terminal escape sequences.
This enables for instance view moved code.

Configuration: Git coloring is set by default, the existing engine is still available.
The GE theme colors for added/removed code (reverse green/red) is applied by default (default Git to just set foreground text is can be configured).
Git by default warns about white space, this is disabled by default (diff.colorMovedWS).
A user can override the Git colors for each "slot" (see below), but this is not available in GE UI.

--
Git word diff coloring

Enable Git word diff coloring view.
This is an alternative view for diff, not in patch format.
The differences are coded on the same line.

Configuration: "GE word highlighting" is the default, "Git diff highlighting" can be enabled.
By default Git highlights changed words. If the user has not set diff.wordRegex, change by character is used ('.').

--

What is not in this PR:

Documentation for the changes, how colors etc can be changed by Git config
Tests are added for the "decoding escape sequence handling". The status for other parts is the same as before (there are no major benefits for adding tests)
Tuned colors fully matching theming
A more generic way to display diff in other ways, like using diff-so-fancy or delta. That is probably a different view, similar to Blame in diff (but using the same FileViewer).

Screenshots

TBD, see #11464 for now

moved

The third is "Git default", no GE theme override, whitespace errors

range diff - the GE coloring is very limited

A missed merge (color should have been yellow, I have modified it). Dual color makes these diffs easier to read.

Git word diff

The word diff is best used to toggle the view back and forward to see details, only worddiff is confusing

ANSI Color theme

Git uses "slots" like old, new, oldMoved etc to set situations.
https://git-scm.com/docs/git-config#Documentation/git-config.txt-colordiffltslotgt
The slots are translated to named colors (8) that can be normal/bright/dim (and more, not really used in Git) as well as background/foreground. A terminal uses a "theme" to translate these colors.
The "theme" for these colors in this PR is displayed below as well as the GE theme colors overriding the default. (Note that the GE colors DiffAddedExtra and DiffRemovedExtra has no direct fit in the ANSI colors, maybe bright is the closest).

Test methodology

Manual, tests to be added, maybe in a followup PR

Merge strategy

Rebase merge (PR submitter must change the commit message for the last commit).

✒️ I contribute this code under The Developer Certificate of Origin.

RussKie · 2024-02-18T22:58:07Z

Plugins/GitUIPluginInterfaces/IGitModule.cs

@@ -242,7 +242,7 @@ public interface IGitModule
        /// <param name="cancellationToken">A cancellation token.</param>
        /// <returns>the Git output.</returns>
        string GetCustomDiffMergeTools(bool isDiff, CancellationToken cancellationToken);
-        Task<(Patch? patch, string? errorMessage)> GetSingleDiffAsync(ObjectId? firstId, ObjectId? secondId, string? fileName, string? oldFileName, string extraDiffArguments, Encoding encoding, bool cacheResult, bool isTracked, CancellationToken cancellationToken);
+        Task<(Patch? patch, string? errorMessage)> GetSingleDiffAsync(ObjectId? firstId, ObjectId? secondId, string? fileName, string? oldFileName, string extraDiffArguments, Encoding encoding, bool cacheResult, bool isTracked, bool useGitColoring, GitCommandConfiguration commandConfiguration, CancellationToken cancellationToken);
        int? GetCommitCount(string parent, string child, bool cache, bool throwOnErrorExit);


These API were already long and now they are bordering on "gross" long... We'll need to revisit these.

GitUI/Editor/FileViewer.Designer.cs

GitUI/Editor/Diff/HighlightInfo.cs

GitUI/Editor/Diff/AnsiEscapeUtilities.cs

mstv

LGTM, few minor comments and nits

GitUI/Editor/Diff/AnsiEscapeUtilities.cs

UnitTests/GitCommands.Tests/Settings/AppSettingsTests.cs

GitCommands/Patches/PatchProcessor.cs

GitUI/CommandsDialogs/SettingsDialog/Pages/ColorsSettings/ColorsSettingsPage.cs

GitUI/Editor/Diff/AnsiEscapeUtilities.cs

GitUI/Editor/ViewModeExtension.cs

GitUI/Hotkey/HotkeySettingsManager.cs

GitUI/Translation/English.xlf

mstv · 2024-02-22T19:39:31Z

GitUI/Translation/English.xlf

@@ -1598,6 +1614,10 @@ The primary difftool can still be selected by clicking the main menu entry.</sou
        <source>Show &amp;entire file</source>
        <target />
      </trans-unit>
+      <trans-unit id="showGitWordColoringToolStripMenuItem.Text">
+        <source>Show Git word coloring</source>


The assigned hotkey should be displayed in the menu item.

It is shown
Setting accelerator to D

It is shown

👍 (In other files, the regarding lines are not that spread over the files as here. So, I missed the already existing line.)

Setting accelerator to D

Then double use of D: for "&Decrease the number of lines of context"

gerhardol · 2024-02-22T23:54:00Z

@mstv Can I squash and rebase (as in tmp/git-coloring)?

mstv

have not tested in detail

gerhardol · 2024-02-25T11:13:27Z

I have added some tests to def59df, will maybe add some sequence tests before adding to this PR.
This could be merged together with #11350 if you want to use some more

mstv

2 tests fail (due to difference in linebreak?)

GitUI/Editor/Diff/AnsiEscapeUtilities.cs

gerhardol · 2024-02-27T22:49:13Z

@RussKie any comments on the tests or anything else?

mstv · 2024-02-28T20:10:11Z

It would be good to have "(go to) next / previous change"
also when "showing Git word coloring".

Copied text contains both the old and the new "words". This may be hard to circumvent, i.e. I am not requesting a change.

        textMarkers[2].Offset.Should().Be(6259);

gerhardol · 2024-02-29T20:46:19Z

It would be good to have "(go to) next / previous change"

With Git word diff, there is no notion about changed lines in the Git output.
The simplest workaround would maybe be a separate diff type and go to start of diff "@@". But that is useless when you need it the most, showing the complete file.
You could maybe guess that if a line has markers it is a change. A little to much guessing for me. But could be considered.

I probably miss the markers on the line numbers more, not sure how to guess. First or last color on line? most dominant color? rainbow?

I have not thought about this in detail, would like to get this and git-diff "ready" before I divert again. Maybe using Delta viewer as an alternative is a step forward (with even more limitations in navigating and copying).

For git-word-diff, it is probably easier to modify Git directly than to patch the output. I almost got it working guessing how to convert back to word format.

Copied text contains both the old and the new "words". This may be hard to circumvent, i.e. I am not requesting a change.

Even harder, then you need to know what the change is and set unique color codes for old/new that are weird or check all Git colors so they are not used for other purposes too.
So I do not see this as likely ever.
The workflow could be to toggle back to patch view.

mstv · 2024-02-29T22:55:54Z

The simplest workaround would maybe be a separate diff type and go to start of diff "@@". But that is useless when you need it the most, showing the complete file.

exactly

You could maybe guess that if a line has markers it is a change. A little to much guessing for me. But could be considered.

Would be good in a follow-up.

I probably miss the markers on the line numbers more, not sure how to guess. First or last color on line? most dominant color? rainbow?

' ', '+', '-', '~'

The workflow could be to toggle back to patch view.

acceptable
I use to mark the text using the mouse. So, a toolbar button would be helpful because the hotkey and the context menu are the second and the third choice only.
But when switching, the wrong line numbers (old/new) are used for restoring the position.

gerhardol · 2024-03-01T09:58:38Z

You could maybe guess that if a line has markers it is a change. A little to much guessing for me. But could be considered.

Would be good in a follow-up.

I probably miss the markers on the line numbers more, not sure how to guess. First or last color on line? most dominant color? rainbow?

' ', '+', '-', '~'

Maybe just "context" color. could be used for next/previous too. But it may require a new "Highlight strategy", something that I have avoided so far.

Edit: there is no + etc at the start of the line.

The workflow could be to toggle back to patch view.

acceptable I use to mark the text using the mouse. So, a toolbar button would be helpful because the hotkey and the context menu are the second and the third choice only. But when switching, the wrong line numbers (old/new) are used for restoring the position.

Line numbers are not accurate, only the line numbers before the differences are correct.

Assuming that code is added rather than removed, the new line numbers is a better choice than old line numbers.

But this raises the question if only new line numbers should be shown or really any numbers at all?

This is one reason why I did not want to promote the function with a toolbar button yet. (I use 'z' as hotkey. this is useful to interpret diffs when reviewing.)
But a button could be added. The obstacle is to decide on an icon.

mstv

Missing features which require heuristics could be added in follow-ups.

gerhardol · 2024-03-09T23:43:53Z

Missing features which require heuristics could be added in follow-ups.

@mstv See tmp/git-coloring 5cae497
Separate or add to this PR?
There are quite some changes (but simplifications too)
I believe this is working correctly, will add one more test. If adding to this PR, I need to squash the current commits, then split this PR
(range-diff navigation is aligned with other diffs and get line numbers)

@RussKie Any comments on the tests, I would like to proceed with this and the dependent git-grep PR

RussKie

🚀

GitCommands/Settings/AppSettings.cs

Enable Git word diff coloring view. This is an alternative view for diff, not in patch format. The differences are coded on the same line.

gerhardol · 2024-03-10T13:50:06Z

rebased, squashed and added xmldoc

@mstv to decide if e7b1fd3 is to be added or be a separate PR

mstv · 2024-03-10T14:19:57Z

Let's merge this now and separately review e7b1fd3.

Move handling of "normal" patches from the shared DiffHighlightService to PatchHighlightService. File-structure namespaces. Various code cleanups. This commit is a preparation for Git coloring and git-grep.

Use the coloring by git-diff instead of GE internal algorithm, by using Git ANSI terminal escape sequences. This enables for instance view moved code.

Enable Git word diff coloring view. This is an alternative view for diff, not in patch format. The differences are coded on the same line.

as well as range-diff and git-grep. git coloring of command output was added in gitextensions#11590. The patch view (default) is configurable, keeping the current GE coloring if a user prefers. This removes the GE coloring for some commands ans the GE coloring is very limited and adding to the complexity. * git-word-diff (gitextensions#11590) * git-grep (gitextensions#11359) * git-range-diff (very limited GE coloring).

as well as range-diff and git-grep. Use of Git coloring of command output was added in gitextensions#11590. The patch view (default) is configurable, setting to keep the current GE coloring. This removes the GE coloring for some commands ans the GE coloring is very limited and is adding complexity. * git-word-diff (gitextensions#11590) * git-grep (gitextensions#11359) * git-range-diff (very limited GE coloring).

as well as range-diff and git-grep. Use of Git coloring of command output was added in #11590. The patch view (default) is configurable, setting to keep the current GE coloring. This removes the GE coloring for some commands ans the GE coloring is very limited and is adding complexity. * git-word-diff (#11590) * git-grep (#11359) * git-range-diff (very limited GE coloring).

ghost assigned gerhardol Feb 15, 2024

gerhardol marked this pull request as draft February 15, 2024 23:12

gerhardol mentioned this pull request Feb 15, 2024

git-grep UI #11350

Merged

gerhardol force-pushed the feature/git-coloring branch from ada71da to e19be0c Compare February 16, 2024 23:29

gerhardol changed the title ~~WIP Git coloring~~ Git diff coloring Feb 16, 2024

gerhardol marked this pull request as ready for review February 16, 2024 23:30

This comment was marked as resolved.

Sign in to view

RussKie reviewed Feb 18, 2024

View reviewed changes

GitUI/Editor/FileViewer.Designer.cs Outdated Show resolved Hide resolved

RussKie reviewed Feb 18, 2024

View reviewed changes

GitUI/Editor/Diff/HighlightInfo.cs Outdated Show resolved Hide resolved

RussKie reviewed Feb 18, 2024

View reviewed changes

GitUI/Editor/Diff/AnsiEscapeUtilities.cs Show resolved Hide resolved

mstv reviewed Feb 22, 2024

View reviewed changes

gerhardol force-pushed the feature/git-coloring branch from ed1ad2d to 511b00f Compare February 24, 2024 13:50

gerhardol added the release-notes-worthy label Feb 24, 2024

gerhardol force-pushed the feature/git-coloring branch from 511b00f to 8f0cf1e Compare February 24, 2024 17:49

mstv approved these changes Feb 24, 2024

View reviewed changes

gerhardol force-pushed the feature/git-coloring branch from bb68c11 to 578837e Compare February 25, 2024 22:36

mstv reviewed Feb 26, 2024

View reviewed changes

GitUI/Editor/Diff/AnsiEscapeUtilities.cs Outdated Show resolved Hide resolved

GitUI/Editor/Diff/AnsiEscapeUtilities.cs Outdated Show resolved Hide resolved

mstv self-requested a review February 28, 2024 20:00

mstv approved these changes Mar 3, 2024

View reviewed changes

This was referenced Mar 6, 2024

Is it possible to integrate Difftastic #11618

Closed

Word Wrap for Diff and File Tree views #448

Closed

RussKie approved these changes Mar 10, 2024

View reviewed changes

GitCommands/Settings/AppSettings.cs Show resolved Hide resolved

gerhardol mentioned this pull request Mar 10, 2024

Combined diff error presentation #11626

Merged

gerhardol added a commit to gerhardol/gitextensions that referenced this pull request Mar 10, 2024

Git word diff coloring (gitextensions#11590)

7e2c953

Enable Git word diff coloring view. This is an alternative view for diff, not in patch format. The differences are coded on the same line.

gerhardol force-pushed the feature/git-coloring branch from f8fbb2a to 7e2c953 Compare March 10, 2024 13:43

gerhardol added 3 commits March 10, 2024 21:56

FileViewer: Restructure HighlightService

04a4431

Move handling of "normal" patches from the shared DiffHighlightService to PatchHighlightService. File-structure namespaces. Various code cleanups. This commit is a preparation for Git coloring and git-grep.

Git diff coloring

455ab21

Use the coloring by git-diff instead of GE internal algorithm, by using Git ANSI terminal escape sequences. This enables for instance view moved code.

Git word diff coloring (gitextensions#11590)

4c0d640

Enable Git word diff coloring view. This is an alternative view for diff, not in patch format. The differences are coded on the same line.

gerhardol force-pushed the feature/git-coloring branch from 7e2c953 to 4c0d640 Compare March 10, 2024 21:31

gerhardol merged commit 7296021 into gitextensions:master Mar 10, 2024

gerhardol deleted the feature/git-coloring branch March 10, 2024 22:01

ghost added this to the vNext milestone Mar 10, 2024

gerhardol mentioned this pull request Mar 10, 2024

git-word-diff navigation and line index marking #11628

Closed

mstv mentioned this pull request Mar 12, 2024

Git *word* coloring #11634

Closed

gerhardol mentioned this pull request Apr 7, 2024

Always use Git coloring for word-diff #11674

Merged

gerhardol mentioned this pull request Apr 28, 2024

Document diff appearance and Git coloring gitextensions/GitExtensionsDoc#150

Merged

gerhardol mentioned this pull request Aug 2, 2024

[vNext] Inconsistent diff colouring #11820

Closed

RussKie mentioned this pull request Aug 3, 2024

[vNext] Diff colouring not enough contrast #11821

Closed

mstv mentioned this pull request Aug 27, 2024

Diff View: Match similar lines in changed blocks #11851

Merged

Uh oh!

Git diff coloring #11590

Git diff coloring #11590

Uh oh!

Conversation

gerhardol commented Feb 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Screenshots

moved

range diff - the GE coloring is very limited

Git word diff

ANSI Color theme

Test methodology

Merge strategy

Uh oh!

This comment was marked as resolved.

Uh oh!

RussKie Feb 18, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mstv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mstv Feb 22, 2024

Choose a reason for hiding this comment

Uh oh!

gerhardol Feb 22, 2024

Choose a reason for hiding this comment

Uh oh!

mstv Feb 24, 2024

Choose a reason for hiding this comment

Uh oh!

gerhardol commented Feb 22, 2024

Uh oh!

mstv left a comment

Choose a reason for hiding this comment

Uh oh!

gerhardol commented Feb 25, 2024

Uh oh!

mstv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gerhardol commented Feb 27, 2024

Uh oh!

mstv commented Feb 28, 2024

Uh oh!

gerhardol commented Feb 29, 2024

Uh oh!

mstv commented Feb 29, 2024

Uh oh!

gerhardol commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstv left a comment

Choose a reason for hiding this comment

Uh oh!

gerhardol commented Mar 9, 2024

Uh oh!

RussKie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gerhardol commented Mar 10, 2024

Uh oh!

mstv commented Mar 10, 2024

gerhardol commented Feb 15, 2024 •

edited

Loading

gerhardol commented Mar 1, 2024 •

edited

Loading