Skip to content

Conversation

gerhardol
Copy link
Member

@gerhardol gerhardol commented Oct 14, 2024

Proposed changes

First commit reviewed in #11982, this commit is incomplete in the handling of text markers . Actually, #11921 that merged adjacent textmarkers, slightly broke the Difftastic color processing.
The colors bleed slightly.
This PR refactors the processing and simplifies the handling too.

Note: It was maybe not a good move to try to process the Difftastic output to be similar to other GE diffs, but now its done.

Screenshots

This is with #11982 only, not in master
image

image

image

image

Test methodology

Manual

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.

instead of inserting '|' to the text
The processing that parses the line numbers printed by
Difftastic did not always remove the textmarkers for
the removed line numbers.
@gerhardol gerhardol force-pushed the feature/difftastic-markers branch from c6ab31b to 3bf0668 Compare October 19, 2024 19:46
@gerhardol gerhardol marked this pull request as ready for review October 20, 2024 14:57
Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

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

Looks sensible, have not run

@mstv
Copy link
Member

mstv commented Oct 26, 2024

The vertical ruler position gets overridden after closing the settings dialog. Just to be aware of - not requesting a change.

@gerhardol gerhardol merged commit 3a1f17a into gitextensions:master Oct 27, 2024
3 of 4 checks passed
@gerhardol gerhardol deleted the feature/difftastic-markers branch October 27, 2024 13:03
@gerhardol
Copy link
Member Author

The vertical ruler position gets overridden after closing the settings dialog. Just to be aware of - not requesting a change.

separate.
Can you explain more?
The splitter "jumps" when refreshing etc, is that what you mean?
Not much to do about that.

@mstv
Copy link
Member

mstv commented Oct 27, 2024

The vertical ruler used as separator for side-by-side diff jumps away to the setting value instead of the difftastic result when refreshing after canceling the settings dialog.

@RussKie RussKie added this to the 5.1 milestone Oct 29, 2024
Sign up for free to 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.

3 participants