fix: LineNumber presentation for Git word-diff #11888
Conversation
Tune the heuristics to get word-diff line numbers. If the diff only had added or removed chars, only added/removed line numbers were presented. There are still many situations where the Git output is incomplete, the line numbers will be incorrect still.
There was a problem hiding this comment.
My first chosen example "23 22 SetText(ref text);" seems to be one of "the line numbers will be incorrect still." (not requesting a change)
Off topic:
The base commit also crashes when trying to display word diff of tests/app/UnitTests/GitUI.Tests/Editor/Diff/SampleGitWord.diff.
| return lineType is (DiffLineType.Minus or DiffLineType.MinusLeft) | ||
| ? (AppSettings.ReverseGitColoring.Value | ||
| ? textMarker.Color == AppColor.AnsiTerminalRedBackNormal.GetThemeColor() | ||
| : textMarker.ForeColor == AppColor.AnsiTerminalRedForeNormal.GetThemeColor()) | ||
| : (AppSettings.ReverseGitColoring.Value | ||
| ? textMarker.Color == AppColor.AnsiTerminalGreenBackNormal.GetThemeColor() | ||
| : textMarker.ForeColor == AppColor.AnsiTerminalGreenForeNormal.GetThemeColor()); |
There was a problem hiding this comment.
Consider reading AppSettings.ReverseGitColoring.Value at most once per document.
There was a problem hiding this comment.
What is the cost? This is used in other parts too.
There was a problem hiding this comment.
Similar could be done in the Difftastic highlighter, other seem OK
There was a problem hiding this comment.
~1000-4000 settings evaluations take 1 ms. This can add up.
There was a problem hiding this comment.
Cache at begining of process. Store colors in something like a dictionary of some key and the color value. Just make sure dictionary is reloaded if settings change or even if new analyzer created. Maybe even provide a function in AppColor that generates this dictionary.
That is the Git output, not much can be done.
Noticed that too, have not investigated |
8edae43 to
6b749dc
Compare
Proposed changes
Tune the heuristics to get word-diff line numbers. If the diff only had added or removed chars, only added/removed line numbers were presented.
There are still many situations where the Git output is incomplete, the line numbers will be incorrect still.
Comments added to the code, trying to explain the issues.
Screenshots
File to compare is in a private branch, did not find a good example in recent commits
with #11868
Before
see line 20
After
Test methodology
tests updated
Merge strategy
I agree that the maintainer squash merge this PR (if the commit message is clear).
✒️ I contribute this code under The Developer Certificate of Origin.