Skip to content

Improve in-line diff highlight and add anchor markers #11862

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 14, 2024
Merged

Improve in-line diff highlight and add anchor markers #11862

merged 2 commits into from
Oct 14, 2024

Conversation

mstv
Copy link
Member

@mstv mstv commented Aug 28, 2024 •
edited
Loading

Proposed changes

  • Try to find multiple identical sections in removed/added line (instead of just at begin and end of line)
  • Draw anchor markers in order to indicate where a pure addition / removal is located

Screenshots

of ScriptOptionsProvider.cs of 25d4c13

Before

image

After

image

Test methodology

  • adapted tests
  • add tests
  • manual

Please do not squash merge

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

@mstv mstv self-assigned this Aug 28, 2024
@mstv mstv marked this pull request as draft August 28, 2024 17:28
gerhardol
gerhardol reviewed Aug 28, 2024
View reviewed changes
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

The commit for this PR is fine for me.
The marker is not very visible, but good enough.

@RussKie
Copy link
Member

RussKie commented Aug 29, 2024

When user copies the text or a diff, would the markers be copied too? Would that have any effect on IDE or compiler?

@gerhardol
Copy link
Member

When user copies the text or a diff, would the markers be copied too? Would that have any effect on IDE or compiler?

No, GE only copies pure text (no formatting), this is not included

@mstv mstv force-pushed the feat/inline_diff branch from c40c403 to b3b7a1e Compare August 29, 2024 16:34
@mstv
Copy link
Member Author

mstv commented Aug 29, 2024

The matching algoritm is much better, but not perfect.
The diff looks strange for 226/227

image

TortoiseDiff handles that better

image

master is better in this situation (below is .17886, so the extra marker is there)

image

👍 I have restored that part of the previous algorithm.
(The algorithm will never be perfect because it does not consider the syntax of the language. But it should be good enough now.)

image

gerhardol
gerhardol reviewed Aug 29, 2024
View reviewed changes
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

+2 to be approved when the dependency is approved.

mstv added a commit to mstv/gitextensions that referenced this pull request Aug 30, 2024
@mstv
feat(DiffHighlightService): Multiple inline matches
2d7029b 
and add anchor marker(s)

Refs: gitextensions#11862
@mstv mstv force-pushed the feat/inline_diff branch from b3b7a1e to 2a6f3db Compare August 30, 2024 19:20
gerhardol
gerhardol approved these changes Aug 30, 2024
View reviewed changes
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

A PR that reviews itself

@gerhardol gerhardol mentioned this pull request Sep 4, 2024
Closed
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 15, 2024
@mstv
feat(DiffHighlightService): Multiple inline matches
5d877ce 
and add anchor marker(s)

Refs: gitextensions#11862
This was referenced Sep 16, 2024
Merged
Merged
@mstv mstv requested review from pmiossec and RussKie September 21, 2024 21:01
@gerhardol gerhardol mentioned this pull request Sep 22, 2024
Merged
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 22, 2024
@mstv
feat(DiffHighlightService): Multiple inline matches
98732fd 
and add anchor marker(s)

Refs: gitextensions#11862
gerhardol
gerhardol reviewed Sep 23, 2024
View reviewed changes
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 26, 2024
@mstv
feat(DiffHighlightService): Multiple inline matches
3bef4a5 
and add anchor marker(s)

Refs: gitextensions#11862
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 26, 2024
@mstv
feat(LinesMatcher): Then match by subwords in line
adf5360 
Refs: gitextensions#11862
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat(DiffHighlightService): Multiple inline matches
5f50426 
and add anchor marker(s)

Refs: gitextensions#11862
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat(LinesMatcher): Then match by subwords in line
4f9e57b 
Refs: gitextensions#11862
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat(DiffHighlightService): Multiple inline matches
f350bdf 
and add anchor marker(s)

Refs: gitextensions#11862
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat(LinesMatcher): Then match by subwords in line
c9d2656 
Refs: gitextensions#11862
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat(DiffHighlightService): Multiple inline matches
b5e6f64 
and add anchor marker(s)

Refs: gitextensions#11862
mstv added a commit to mstv/gitextensions that referenced this pull request Sep 27, 2024
@mstv
feat(LinesMatcher): Then match by subwords in line
2866705 
Refs: gitextensions#11862
@mstv mstv marked this pull request as ready for review September 27, 2024 21:26
@gerhardol
Copy link
Member

+2 Has only reviewed briefly after latest changes, but have appreciated using this PR for a month now

@gerhardol gerhardol mentioned this pull request Sep 27, 2024
Merged
gerhardol
gerhardol approved these changes Sep 28, 2024
View reviewed changes
@gerhardol
Copy link
Member

@gitextensions/git-extensions-team I suggest this PR is merged

@gerhardol
Copy link
Member

@mstv merge?

@mstv mstv force-pushed the feat/inline_diff branch from 44342c3 to 95a30af Compare October 14, 2024 18:22
@mstv mstv merged commit 95a30af into gitextensions:master Oct 14, 2024
4 checks passed
@mstv mstv deleted the feat/inline_diff branch October 14, 2024 19:06
@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
Reviewers

@gerhardol gerhardol gerhardol approved these changes

@pmiossec pmiossec Awaiting requested review from pmiossec

@RussKie RussKie Awaiting requested review from RussKie

Assignees

@mstv mstv

Labels
status: ready
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
3 participants
@mstv @RussKie @gerhardol