Skip to content
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

Add navigation to "Prev/Next Change" for built-in Diff viewer #616

Closed
goran-w opened this issue Oct 28, 2024 · 8 comments
Closed

Add navigation to "Prev/Next Change" for built-in Diff viewer #616

goran-w opened this issue Oct 28, 2024 · 8 comments
Assignees
@love-linger
Labels
suggestion We are still considering whether it is necessary to add this feature

Comments

@goran-w
Copy link
Contributor

goran-w commented Oct 28, 2024

The built-in Diff viewer has no way to navigate quickly to the previous or next change (or hunk). This would be very helpful, especially if also implementing #615.

I suggest adding icon-toggles (up-down arrows) for "Prev(ious)/Next Change" (with corresponding hotkeys) in the top (mini-)bar of the built-in Diff viewer.

For reference, SmartGit and GitKraken has this feature, and it's also very common in standalone diff/merge vievers like P4Merge, Meld etc.
@love-linger
Copy link
Collaborator

SourceGit provides support for external comparison tools, which means that in this project, the built-in comparison view will only implement some basic functions.

As you mentioned above, this feature is also unnecessary until the entire file display feature is added, because SourceGit only displays a limited context.

@love-linger love-linger self-assigned this Oct 29, 2024
@love-linger love-linger added the suggestion We are still considering whether it is necessary to add this feature label Oct 29, 2024
@goran-w goran-w mentioned this issue Oct 29, 2024
Closed
@goran-w
Copy link
Contributor Author

goran-w commented Oct 29, 2024 •
edited
Loading

I find it unfortunate to have to resort to external tools for this, when the built-in Diff is much more convenient for quick (re)viewing of changes. Having to popup an external tool takes extra time and disrupts the workflow - I would prefer to only use that for resolving merge conflicts. Viewing the whole file (#615) and being able to navigate quickly between changes would improve this tool a lot, IMHO. (Would you be willing to accept a PR for this, eventually?)

@dougcunha
Copy link
Contributor

I don't see why the navigation buttons shouldn't be available regardless of whether the entire file is displayed or not. Navigation could happen within the lines that are loaded. This feature, combined with the addition of keyboard shortcuts to accept or reject a block of changes, would make keyboard operation easier.

@love-linger
Copy link
Collaborator

Because the context line number is not large enough, go to next/prev change just like page down/up

@dougcunha
Copy link
Contributor

Instead of simply scrolling through changes, I believe the optimal approach would be to automatically select the next difference block upon navigation. This way, I can utilize keyboard shortcuts to discard or accept changes efficiently, ensuring smooth commit review even when the context is limited. This keyboard-centric approach streamlines the entire process and significantly enhances productivity.

@goran-w
Copy link
Contributor Author

goran-w commented Oct 30, 2024

I agree - keyboard shortcuts for navigating up/dn to each change (instead of just PgUp/Dn) and for stage/discard would be a great workflow.

@goran-w goran-w mentioned this issue Nov 15, 2024
Merged
love-linger added a commit that referenced this issue Nov 16, 2024
@love-linger
feature: add buttons to go to prev/next change in text diff view (#616)
9093bff 
Signed-off-by: leo <longshuang@msn.cn>
love-linger added a commit that referenced this issue Nov 16, 2024
@love-linger
feature: add buttons to go to prev/next change in text diff view (#616)
134c710 
Signed-off-by: leo <longshuang@msn.cn>
@love-linger
Copy link
Collaborator

Done. You can download latest CI build from Github Action

goran-w added a commit to goran-w/sourcegit that referenced this issue Nov 16, 2024
@goran-w
Cherrypick - feature: add buttons to go to prev/next change in text d…
e634df7 
…iff view (sourcegit-scm#616)

Signed-off-by: leo <longshuang@msn.cn>
(cherry picked from commit 134c710)

# Conflicts:
#	src/Views/DiffView.axaml
#	src/Views/TextDiffView.axaml.cs
goran-w added a commit to goran-w/sourcegit that referenced this issue Nov 16, 2024
@goran-w
Cherrypick - feature: add buttons to go to prev/next change in text d…
1a99ce5 
…iff view (sourcegit-scm#616)

Signed-off-by: leo <longshuang@msn.cn>
(cherry picked from commit 134c710)

# Conflicts:
#	src/Views/DiffView.axaml
#	src/Views/TextDiffView.axaml.cs
@goran-w goran-w mentioned this issue Nov 18, 2024
Closed
@goran-w
Copy link
Contributor Author

goran-w commented Nov 18, 2024

@dougcunha Thanks for your comments earlier. See suggestion #717 for continued discussion - my alternative implementation is currently in PR #703.

goran-w added a commit to goran-w/sourcegit that referenced this issue Nov 28, 2024
@goran-w
Cherrypick - feature: add buttons to go to prev/next change in text d…
cae8c6e 
…iff view (sourcegit-scm#616)

Signed-off-by: leo <longshuang@msn.cn>
(cherry picked from commit 134c710)

# Conflicts:
#	src/Views/DiffView.axaml
#	src/Views/TextDiffView.axaml.cs
goran-w added a commit to goran-w/sourcegit that referenced this issue Nov 28, 2024
@goran-w
Cherrypick - feature: add buttons to go to prev/next change in text d…
692474a 
…iff view (sourcegit-scm#616)

Signed-off-by: leo <longshuang@msn.cn>
(cherry picked from commit 134c710)

# Conflicts:
#	src/Views/DiffView.axaml
#	src/Views/TextDiffView.axaml.cs
love-linger pushed a commit that referenced this issue Dec 8, 2024
@goran-w
Add navigation and highlighting of change-blocks in Text Diff (#616, #…
655d71b 
…717) (#703)

* Corrected misspelled local variable nextHigh(t)light

* Implemented change-block navigation

* Modified behavior of the Prev/Next Change buttons in DiffView toolbar.
* Well-defined change-blocks are pre-calculated and can be navigated between.
* Current change-block is highlighted in the Diff panel(s).
* Prev/next at start/end of range (re-)scrolls to first/last change-block
(I.e when unset, or already at first/last change-block, or at the only one.)
* Current change-block is unset in RefreshContent().

* Added safeguards for edge cases

* Added indicator of current/total change-blocks in DiffView toolbar

* Added new Icon and String (en-US) for Highlighted Diff Navigation

* Added Preference and ToggleButton for diff navigation style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@love-linger love-linger

Labels
suggestion We are still considering whether it is necessary to add this feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dougcunha @love-linger @goran-w