From 2d61a51ded18f0a0f928df2dbd8f5b9c34036c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Sat, 1 Mar 2025 00:20:26 -0300 Subject: [PATCH] Diff View: Scroll to center of hunks when reviewing (#25846) When reviewing hunks, scroll to put them at the center of the screen so you can better see the context around that hunk. The field `center_cursor` was added to the actions `editor::GoToHunk` and `editor::GoToPrevHunk`, this was set to `false` by default in keymaps, as it wouldn't help with in-editor navigation. The field is set to `true` for when you trigger `git::StageAndNext` and `git::UnstageAndNext`, this is also `true` for the buttons in the Diff View toolbar. Release Notes: - N/A --- assets/keymaps/default-linux.json | 8 +-- assets/keymaps/default-macos.json | 4 +- assets/keymaps/linux/jetbrains.json | 4 +- assets/keymaps/linux/sublime_text.json | 4 +- assets/keymaps/macos/jetbrains.json | 4 +- assets/keymaps/macos/sublime_text.json | 4 +- assets/keymaps/vim.json | 4 +- crates/editor/src/actions.rs | 18 +++++- crates/editor/src/editor.rs | 87 +++++++++++++++++--------- crates/editor/src/editor_tests.rs | 18 +++--- crates/editor/src/element.rs | 13 ++-- crates/git_ui/src/project_diff.rs | 24 +++++-- crates/zed/src/zed/quick_action_bar.rs | 14 ++++- 13 files changed, 140 insertions(+), 66 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index f32e2f921a..d2ec5d2384 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -372,8 +372,8 @@ "ctrl-alt-y": "git::ToggleStaged", "alt-y": ["git::StageAndNext", { "whole_excerpt": false }], "alt-shift-y": ["git::UnstageAndNext", { "whole_excerpt": false }], - "alt-.": "editor::GoToHunk", - "alt-,": "editor::GoToPrevHunk" + "alt-.": ["editor::GoToHunk", { "center_cursor": true }], + "alt-,": ["editor::GoToPrevHunk", { "center_cursor": true }] } }, { @@ -564,8 +564,8 @@ "shift-enter": "editor::ExpandExcerpts", "ctrl-alt-enter": "editor::OpenExcerptsSplit", "ctrl-shift-e": "pane::RevealInProjectPanel", - "ctrl-f8": "editor::GoToHunk", - "ctrl-shift-f8": "editor::GoToPrevHunk", + "ctrl-f8": ["editor::GoToHunk", { "center_cursor": true }], + "ctrl-shift-f8": ["editor::GoToPrevHunk", { "center_cursor": true }], "ctrl-enter": "assistant::InlineAssist", "ctrl-:": "editor::ToggleInlayHints" } diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 46e66269cb..b69cc66c32 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -640,8 +640,8 @@ "shift-enter": "editor::ExpandExcerpts", "cmd-alt-enter": "editor::OpenExcerptsSplit", "cmd-shift-e": "pane::RevealInProjectPanel", - "cmd-f8": "editor::GoToHunk", - "cmd-shift-f8": "editor::GoToPrevHunk", + "cmd-f8": ["editor::GoToHunk", { "center_cursor": true }], + "cmd-shift-f8": ["editor::GoToPrevHunk", { "center_cursor": true }], "ctrl-enter": "assistant::InlineAssist", "ctrl-:": "editor::ToggleInlayHints" } diff --git a/assets/keymaps/linux/jetbrains.json b/assets/keymaps/linux/jetbrains.json index 39f9c98d9d..fe4f8d5e42 100644 --- a/assets/keymaps/linux/jetbrains.json +++ b/assets/keymaps/linux/jetbrains.json @@ -42,8 +42,8 @@ "ctrl-alt-shift-b": "editor::GoToTypeDefinitionSplit", "f2": "editor::GoToDiagnostic", "shift-f2": "editor::GoToPrevDiagnostic", - "ctrl-alt-shift-down": "editor::GoToHunk", - "ctrl-alt-shift-up": "editor::GoToPrevHunk", + "ctrl-alt-shift-down": ["editor::GoToHunk", { "center_cursor": true }], + "ctrl-alt-shift-up": ["editor::GoToPrevHunk", { "center_cursor": true }], "ctrl-alt-z": "git::Restore", "ctrl-home": "editor::MoveToBeginning", "ctrl-end": "editor::MoveToEnd", diff --git a/assets/keymaps/linux/sublime_text.json b/assets/keymaps/linux/sublime_text.json index 3e61cc7c9d..37959b2f64 100644 --- a/assets/keymaps/linux/sublime_text.json +++ b/assets/keymaps/linux/sublime_text.json @@ -43,8 +43,8 @@ "ctrl-f12": "editor::GoToDefinitionSplit", "shift-f12": "editor::FindAllReferences", "ctrl-shift-f12": "editor::FindAllReferences", - "ctrl-.": "editor::GoToHunk", - "ctrl-,": "editor::GoToPrevHunk", + "ctrl-.": ["editor::GoToHunk", { "center_cursor": true }], + "ctrl-,": ["editor::GoToPrevHunk", { "center_cursor": true }], "ctrl-k ctrl-u": "editor::ConvertToUpperCase", "ctrl-k ctrl-l": "editor::ConvertToLowerCase", "shift-alt-m": "markdown::OpenPreviewToTheSide", diff --git a/assets/keymaps/macos/jetbrains.json b/assets/keymaps/macos/jetbrains.json index 66c2262b85..7871e7e6d8 100644 --- a/assets/keymaps/macos/jetbrains.json +++ b/assets/keymaps/macos/jetbrains.json @@ -40,8 +40,8 @@ "cmd-alt-shift-b": "editor::GoToTypeDefinitionSplit", "f2": "editor::GoToDiagnostic", "shift-f2": "editor::GoToPrevDiagnostic", - "ctrl-alt-shift-down": "editor::GoToHunk", - "ctrl-alt-shift-up": "editor::GoToPrevHunk", + "ctrl-alt-shift-down": ["editor::GoToHunk", { "center_cursor": true }], + "ctrl-alt-shift-up": ["editor::GoToPrevHunk", { "center_cursor": true }], "cmd-home": "editor::MoveToBeginning", "cmd-end": "editor::MoveToEnd", "cmd-shift-home": "editor::SelectToBeginning", diff --git a/assets/keymaps/macos/sublime_text.json b/assets/keymaps/macos/sublime_text.json index 445de88990..656e6d634c 100644 --- a/assets/keymaps/macos/sublime_text.json +++ b/assets/keymaps/macos/sublime_text.json @@ -44,8 +44,8 @@ "alt-cmd-down": "editor::GoToDefinition", "ctrl-alt-cmd-down": "editor::GoToDefinitionSplit", "alt-shift-cmd-down": "editor::FindAllReferences", - "ctrl-.": "editor::GoToHunk", - "ctrl-,": "editor::GoToPrevHunk", + "ctrl-.": ["editor::GoToHunk", { "center_cursor": true }], + "ctrl-,": ["editor::GoToPrevHunk", { "center_cursor": true }], "cmd-k cmd-u": "editor::ConvertToUpperCase", "cmd-k cmd-l": "editor::ConvertToLowerCase", "cmd-shift-j": "editor::JoinLines", diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index b2107c7954..62367044a6 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -238,8 +238,8 @@ "] x": "vim::SelectSmallerSyntaxNode", "] d": "editor::GoToDiagnostic", "[ d": "editor::GoToPrevDiagnostic", - "] c": "editor::GoToHunk", - "[ c": "editor::GoToPrevHunk", + "] c": ["editor::GoToHunk", { "center_cursor": true }], + "[ c": ["editor::GoToPrevHunk", { "center_cursor": true }], "g c": "vim::PushToggleComments" } }, diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index f4906ebe7f..26d1b21362 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -189,6 +189,20 @@ pub struct DeleteToPreviousWordStart { pub ignore_newlines: bool, } +#[derive(PartialEq, Clone, Deserialize, Default, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct GoToHunk { + #[serde(default)] + pub center_cursor: bool, +} + +#[derive(PartialEq, Clone, Deserialize, Default, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct GoToPrevHunk { + #[serde(default)] + pub center_cursor: bool, +} + #[derive(PartialEq, Clone, Deserialize, Default, JsonSchema)] pub struct FoldAtLevel(pub u32); @@ -218,6 +232,8 @@ impl_actions!( ExpandExcerptsDown, ExpandExcerptsUp, FoldAt, + GoToHunk, + GoToPrevHunk, HandleInput, MoveDownByLines, MovePageDown, @@ -300,11 +316,9 @@ gpui::actions!( GoToDefinition, GoToDefinitionSplit, GoToDiagnostic, - GoToHunk, GoToImplementation, GoToImplementationSplit, GoToPrevDiagnostic, - GoToPrevHunk, GoToTypeDefinition, GoToTypeDefinitionSplit, HalfPageDown, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 4c25549f94..811e1d5f0b 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -11411,27 +11411,45 @@ impl Editor { } } - fn go_to_next_hunk(&mut self, _: &GoToHunk, window: &mut Window, cx: &mut Context) { + fn go_to_next_hunk(&mut self, action: &GoToHunk, window: &mut Window, cx: &mut Context) { let snapshot = self.snapshot(window, cx); let selection = self.selections.newest::(cx); - self.go_to_hunk_after_position(&snapshot, selection.head(), window, cx); + self.go_to_hunk_after_or_before_position( + &snapshot, + selection.head(), + true, + action.center_cursor, + window, + cx, + ); } - fn go_to_hunk_after_position( + fn go_to_hunk_after_or_before_position( &mut self, snapshot: &EditorSnapshot, position: Point, + after: bool, + scroll_center: bool, window: &mut Window, cx: &mut Context, ) -> Option { - let hunk = self.hunk_after_position(snapshot, position); + let hunk = if after { + self.hunk_after_position(snapshot, position) + } else { + self.hunk_before_position(snapshot, position) + }; if let Some(hunk) = &hunk { - let point = Point::new(hunk.row_range.start.0, 0); + let destination = Point::new(hunk.row_range.start.0, 0); + let autoscroll = if scroll_center { + Autoscroll::center() + } else { + Autoscroll::fit() + }; - self.unfold_ranges(&[point..point], false, false, cx); - self.change_selections(Some(Autoscroll::fit()), window, cx, |s| { - s.select_ranges([point..point]); + self.unfold_ranges(&[destination..destination], false, false, cx); + self.change_selections(Some(autoscroll), window, cx, |s| { + s.select_ranges([destination..destination]); }); } @@ -11455,32 +11473,33 @@ impl Editor { }) } - fn go_to_prev_hunk(&mut self, _: &GoToPrevHunk, window: &mut Window, cx: &mut Context) { + fn go_to_prev_hunk( + &mut self, + action: &GoToPrevHunk, + window: &mut Window, + cx: &mut Context, + ) { let snapshot = self.snapshot(window, cx); let selection = self.selections.newest::(cx); - self.go_to_hunk_before_position(&snapshot, selection.head(), window, cx); + self.go_to_hunk_after_or_before_position( + &snapshot, + selection.head(), + false, + action.center_cursor, + window, + cx, + ); } - fn go_to_hunk_before_position( + fn hunk_before_position( &mut self, snapshot: &EditorSnapshot, position: Point, - window: &mut Window, - cx: &mut Context, ) -> Option { - let mut hunk = snapshot.buffer_snapshot.diff_hunk_before(position); - if hunk.is_none() { - hunk = snapshot.buffer_snapshot.diff_hunk_before(Point::MAX); - } - if let Some(hunk) = &hunk { - let destination = Point::new(hunk.row_range.start.0, 0); - self.unfold_ranges(&[destination..destination], false, false, cx); - self.change_selections(Some(Autoscroll::fit()), window, cx, |s| { - s.select_ranges(vec![destination..destination]); - }); - } - - hunk + snapshot + .buffer_snapshot + .diff_hunk_before(position) + .or_else(|| snapshot.buffer_snapshot.diff_hunk_before(Point::MAX)) } pub fn go_to_definition( @@ -13598,7 +13617,13 @@ impl Editor { }); if run_twice { - self.go_to_next_hunk(&Default::default(), window, cx); + self.go_to_next_hunk( + &GoToHunk { + center_cursor: true, + }, + window, + cx, + ); } } else if !self.buffer().read(cx).is_singleton() { self.stage_or_unstage_diff_hunks(stage, &ranges[..], window, cx); @@ -13656,7 +13681,13 @@ impl Editor { } } self.stage_or_unstage_diff_hunks(stage, &ranges[..], window, cx); - self.go_to_next_hunk(&Default::default(), window, cx); + self.go_to_next_hunk( + &GoToHunk { + center_cursor: true, + }, + window, + cx, + ); } fn do_stage_or_unstage( diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index cda4b9ef30..26ad45cfab 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -11302,7 +11302,7 @@ async fn test_go_to_hunk(executor: BackgroundExecutor, cx: &mut TestAppContext) cx.update_editor(|editor, window, cx| { //Wrap around the bottom of the buffer for _ in 0..3 { - editor.go_to_next_hunk(&GoToHunk, window, cx); + editor.go_to_next_hunk(&GoToHunk::default(), window, cx); } }); @@ -11324,7 +11324,7 @@ async fn test_go_to_hunk(executor: BackgroundExecutor, cx: &mut TestAppContext) cx.update_editor(|editor, window, cx| { //Wrap around the top of the buffer for _ in 0..2 { - editor.go_to_prev_hunk(&GoToPrevHunk, window, cx); + editor.go_to_prev_hunk(&GoToPrevHunk::default(), window, cx); } }); @@ -11344,7 +11344,7 @@ async fn test_go_to_hunk(executor: BackgroundExecutor, cx: &mut TestAppContext) ); cx.update_editor(|editor, window, cx| { - editor.go_to_prev_hunk(&GoToPrevHunk, window, cx); + editor.go_to_prev_hunk(&GoToPrevHunk::default(), window, cx); }); cx.assert_editor_state( @@ -11363,7 +11363,7 @@ async fn test_go_to_hunk(executor: BackgroundExecutor, cx: &mut TestAppContext) ); cx.update_editor(|editor, window, cx| { - editor.go_to_prev_hunk(&GoToPrevHunk, window, cx); + editor.go_to_prev_hunk(&GoToPrevHunk::default(), window, cx); }); cx.assert_editor_state( @@ -11383,7 +11383,7 @@ async fn test_go_to_hunk(executor: BackgroundExecutor, cx: &mut TestAppContext) cx.update_editor(|editor, window, cx| { for _ in 0..2 { - editor.go_to_prev_hunk(&GoToPrevHunk, window, cx); + editor.go_to_prev_hunk(&GoToPrevHunk::default(), window, cx); } }); @@ -11407,7 +11407,7 @@ async fn test_go_to_hunk(executor: BackgroundExecutor, cx: &mut TestAppContext) }); cx.update_editor(|editor, window, cx| { - editor.go_to_next_hunk(&GoToHunk, window, cx); + editor.go_to_next_hunk(&GoToHunk::default(), window, cx); }); cx.assert_editor_state( @@ -13414,7 +13414,7 @@ async fn test_toggle_selected_diff_hunks(executor: BackgroundExecutor, cx: &mut executor.run_until_parked(); cx.update_editor(|editor, window, cx| { - editor.go_to_next_hunk(&GoToHunk, window, cx); + editor.go_to_next_hunk(&GoToHunk::default(), window, cx); editor.toggle_selected_diff_hunks(&ToggleSelectedDiffHunks, window, cx); }); executor.run_until_parked(); @@ -13436,7 +13436,7 @@ async fn test_toggle_selected_diff_hunks(executor: BackgroundExecutor, cx: &mut cx.update_editor(|editor, window, cx| { for _ in 0..2 { - editor.go_to_next_hunk(&GoToHunk, window, cx); + editor.go_to_next_hunk(&GoToHunk::default(), window, cx); editor.toggle_selected_diff_hunks(&ToggleSelectedDiffHunks, window, cx); } }); @@ -13459,7 +13459,7 @@ async fn test_toggle_selected_diff_hunks(executor: BackgroundExecutor, cx: &mut ); cx.update_editor(|editor, window, cx| { - editor.go_to_next_hunk(&GoToHunk, window, cx); + editor.go_to_next_hunk(&GoToHunk::default(), window, cx); editor.toggle_selected_diff_hunks(&ToggleSelectedDiffHunks, window, cx); }); executor.run_until_parked(); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index ccf0205ee0..0e256d5ace 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -8873,7 +8873,7 @@ fn diff_hunk_controls( move |window, cx| { Tooltip::for_action_in( "Next Hunk", - &GoToHunk, + &GoToHunk::default(), &focus_handle, window, cx, @@ -8887,8 +8887,9 @@ fn diff_hunk_controls( let snapshot = editor.snapshot(window, cx); let position = hunk_range.end.to_point(&snapshot.buffer_snapshot); - editor - .go_to_hunk_after_position(&snapshot, position, window, cx); + editor.go_to_hunk_after_or_before_position( + &snapshot, position, true, true, window, cx, + ); editor.expand_selected_diff_hunks(cx); }); } @@ -8904,7 +8905,7 @@ fn diff_hunk_controls( move |window, cx| { Tooltip::for_action_in( "Previous Hunk", - &GoToPrevHunk, + &GoToPrevHunk::default(), &focus_handle, window, cx, @@ -8918,7 +8919,9 @@ fn diff_hunk_controls( let snapshot = editor.snapshot(window, cx); let point = hunk_range.start.to_point(&snapshot.buffer_snapshot); - editor.go_to_hunk_before_position(&snapshot, point, window, cx); + editor.go_to_hunk_after_or_before_position( + &snapshot, point, false, true, window, cx, + ); editor.expand_selected_diff_hunks(cx); }); } diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index c4e49f58d8..e1a10a9168 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -869,12 +869,20 @@ impl Render for ProjectDiffToolbar { .shape(ui::IconButtonShape::Square) .tooltip(Tooltip::for_action_title_in( "Go to previous hunk", - &GoToPrevHunk, + &GoToPrevHunk { + center_cursor: false, + }, &focus_handle, )) .disabled(!button_states.prev_next) .on_click(cx.listener(|this, _, window, cx| { - this.dispatch_action(&GoToPrevHunk, window, cx) + this.dispatch_action( + &GoToPrevHunk { + center_cursor: true, + }, + window, + cx, + ) })), ) .child( @@ -882,12 +890,20 @@ impl Render for ProjectDiffToolbar { .shape(ui::IconButtonShape::Square) .tooltip(Tooltip::for_action_title_in( "Go to next hunk", - &GoToHunk, + &GoToHunk { + center_cursor: false, + }, &focus_handle, )) .disabled(!button_states.prev_next) .on_click(cx.listener(|this, _, window, cx| { - this.dispatch_action(&GoToHunk, window, cx) + this.dispatch_action( + &GoToHunk { + center_cursor: true, + }, + window, + cx, + ) })), ), ) diff --git a/crates/zed/src/zed/quick_action_bar.rs b/crates/zed/src/zed/quick_action_bar.rs index 21ef7db7cc..6f5ed44918 100644 --- a/crates/zed/src/zed/quick_action_bar.rs +++ b/crates/zed/src/zed/quick_action_bar.rs @@ -182,8 +182,18 @@ impl Render for QuickActionBar { .action("Next Problem", Box::new(GoToDiagnostic)) .action("Previous Problem", Box::new(GoToPrevDiagnostic)) .separator() - .action("Next Hunk", Box::new(GoToHunk)) - .action("Previous Hunk", Box::new(GoToPrevHunk)) + .action( + "Next Hunk", + Box::new(GoToHunk { + center_cursor: true, + }), + ) + .action( + "Previous Hunk", + Box::new(GoToPrevHunk { + center_cursor: true, + }), + ) .separator() .action("Move Line Up", Box::new(MoveLineUp)) .action("Move Line Down", Box::new(MoveLineDown))