From d1395a6d6164ce4119a4da2cd7dde1242c2f1b8d Mon Sep 17 00:00:00 2001 From: hamflx Date: Sat, 17 Jun 2023 17:53:50 +0800 Subject: [PATCH 1/3] feat: support 'n'/'p' key to move to the next/prev hunk. fix make check errors --- CHANGELOG.md | 1 + src/components/diff.rs | 63 ++++++++++++++++++++++++++++++++++++++++-- src/keys/key_list.rs | 4 +++ src/strings.rs | 24 ++++++++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 373b82216a..c2b2583b14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * allow `copy` file path on revision files and status tree [[@yanganto]](https://github.com/yanganto) ([#1516](https://github.com/extrawurst/gitui/pull/1516)) * print message of where log will be written if `-l` is set ([#1472](https://github.com/extrawurst/gitui/pull/1472)) * show remote branches in log [[@cruessler](https://github.com/cruessler)] ([#1501](https://github.com/extrawurst/gitui/issues/1501)) +* support 'n'/'p' key to move to the next/prev hunk in diff component [[@hamflx](https://github.com/hamflx)] ([#1523](https://github.com/extrawurst/gitui/issues/1523)) ### Fixes * fixed side effect of crossterm 0.26 on windows that caused double input of all keys [[@pm100]](https://github/pm100) ([#1686](https://github.com/extrawurst/gitui/pull/1686)) diff --git a/src/components/diff.rs b/src/components/diff.rs index 76b174f3c5..47695fa2a3 100644 --- a/src/components/diff.rs +++ b/src/components/diff.rs @@ -629,6 +629,44 @@ impl DiffComponent { Ok(()) } + fn calc_hunk_move_target( + &self, + direction: isize, + ) -> Option { + let diff = self.diff.as_ref()?; + if diff.hunks.is_empty() { + return None; + } + let max = diff.hunks.len() - 1; + let target_index = self.selected_hunk.map_or(0, |i| { + let target = if direction >= 0 { + i.saturating_add(direction.unsigned_abs()) + } else { + i.saturating_sub(direction.unsigned_abs()) + }; + std::cmp::min(max, target) + }); + Some(target_index) + } + + fn diff_hunk_move_up_down(&mut self, direction: isize) { + let Some(diff) = &self.diff else { return }; + let target_index = self.calc_hunk_move_target(direction); + // return if selected_hunk not change + if self.selected_hunk == target_index { + return; + } + if let Some(target_index) = target_index { + let lines = diff + .hunks + .iter() + .take(target_index) + .fold(0, |sum, hunk| sum + hunk.lines.len()); + self.selection = Selection::Single(lines); + self.selected_hunk = Some(target_index); + } + } + const fn is_stage(&self) -> bool { self.current.is_stage } @@ -710,7 +748,16 @@ impl Component for DiffComponent { self.can_scroll(), self.focused(), )); - + out.push(CommandInfo::new( + strings::commands::diff_hunk_next(&self.key_config), + self.calc_hunk_move_target(1) != self.selected_hunk, + self.focused(), + )); + out.push(CommandInfo::new( + strings::commands::diff_hunk_prev(&self.key_config), + self.calc_hunk_move_target(-1) != self.selected_hunk, + self.focused(), + )); out.push( CommandInfo::new( strings::commands::diff_home_end(&self.key_config), @@ -769,7 +816,7 @@ impl Component for DiffComponent { CommandBlocking::PassingOn } - #[allow(clippy::cognitive_complexity)] + #[allow(clippy::cognitive_complexity, clippy::too_many_lines)] fn event(&mut self, ev: &Event) -> Result { if self.focused() { if let Event::Key(e) = ev { @@ -815,6 +862,18 @@ impl Component for DiffComponent { self.horizontal_scroll .move_right(HorizontalScrollType::Left); Ok(EventState::Consumed) + } else if key_match( + e, + self.key_config.keys.diff_hunk_next, + ) { + self.diff_hunk_move_up_down(1); + Ok(EventState::Consumed) + } else if key_match( + e, + self.key_config.keys.diff_hunk_prev, + ) { + self.diff_hunk_move_up_down(-1); + Ok(EventState::Consumed) } else if key_match( e, self.key_config.keys.stage_unstage_item, diff --git a/src/keys/key_list.rs b/src/keys/key_list.rs index a3c79c934a..134d992d87 100644 --- a/src/keys/key_list.rs +++ b/src/keys/key_list.rs @@ -109,6 +109,8 @@ pub struct KeysList { pub pull: GituiKeyEvent, pub abort_merge: GituiKeyEvent, pub undo_commit: GituiKeyEvent, + pub diff_hunk_next: GituiKeyEvent, + pub diff_hunk_prev: GituiKeyEvent, pub stage_unstage_item: GituiKeyEvent, pub tag_annotate: GituiKeyEvent, pub view_submodules: GituiKeyEvent, @@ -193,6 +195,8 @@ impl Default for KeysList { open_file_tree: GituiKeyEvent::new(KeyCode::Char('F'), KeyModifiers::SHIFT), file_find: GituiKeyEvent::new(KeyCode::Char('f'), KeyModifiers::empty()), branch_find: GituiKeyEvent::new(KeyCode::Char('f'), KeyModifiers::empty()), + diff_hunk_next: GituiKeyEvent::new(KeyCode::Char('n'), KeyModifiers::empty()), + diff_hunk_prev: GituiKeyEvent::new(KeyCode::Char('p'), KeyModifiers::empty()), stage_unstage_item: GituiKeyEvent::new(KeyCode::Enter, KeyModifiers::empty()), tag_annotate: GituiKeyEvent::new(KeyCode::Char('a'), KeyModifiers::CONTROL), view_submodules: GituiKeyEvent::new(KeyCode::Char('S'), KeyModifiers::SHIFT), diff --git a/src/strings.rs b/src/strings.rs index fc7ef10760..3e1ff80633 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -603,6 +603,30 @@ pub mod commands { CMD_GROUP_LOG, ) } + pub fn diff_hunk_next( + key_config: &SharedKeyConfig, + ) -> CommandText { + CommandText::new( + format!( + "Next hunk [{}]", + key_config.get_hint(key_config.keys.diff_hunk_next), + ), + "move cursor to next hunk", + CMD_GROUP_DIFF, + ) + } + pub fn diff_hunk_prev( + key_config: &SharedKeyConfig, + ) -> CommandText { + CommandText::new( + format!( + "Prev hunk [{}]", + key_config.get_hint(key_config.keys.diff_hunk_prev), + ), + "move cursor to prev hunk", + CMD_GROUP_DIFF, + ) + } pub fn diff_home_end( key_config: &SharedKeyConfig, ) -> CommandText { From b4fddfdfd832f7158ecef4ebd67029f3968526e5 Mon Sep 17 00:00:00 2001 From: hamflx Date: Sat, 17 Jun 2023 20:27:16 +0800 Subject: [PATCH 2/3] feat: auto scroll next/prev hunk into visible area. --- src/components/diff.rs | 20 ++++++++++++------- src/components/utils/scroll_vertical.rs | 26 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/components/diff.rs b/src/components/diff.rs index 47695fa2a3..f894add2ba 100644 --- a/src/components/diff.rs +++ b/src/components/diff.rs @@ -651,19 +651,25 @@ impl DiffComponent { fn diff_hunk_move_up_down(&mut self, direction: isize) { let Some(diff) = &self.diff else { return }; - let target_index = self.calc_hunk_move_target(direction); + let hunk_index = self.calc_hunk_move_target(direction); // return if selected_hunk not change - if self.selected_hunk == target_index { + if self.selected_hunk == hunk_index { return; } - if let Some(target_index) = target_index { - let lines = diff + if let Some(hunk_index) = hunk_index { + let line_index = diff .hunks .iter() - .take(target_index) + .take(hunk_index) .fold(0, |sum, hunk| sum + hunk.lines.len()); - self.selection = Selection::Single(lines); - self.selected_hunk = Some(target_index); + let hunk = &diff.hunks[hunk_index]; + self.selection = Selection::Single(line_index); + self.selected_hunk = Some(hunk_index); + self.vertical_scroll.move_area_to_visible( + self.current_size.get().1 as usize, + line_index, + line_index.saturating_add(hunk.lines.len()), + ); } } diff --git a/src/components/utils/scroll_vertical.rs b/src/components/utils/scroll_vertical.rs index 02b46ac9b5..e784187f8e 100644 --- a/src/components/utils/scroll_vertical.rs +++ b/src/components/utils/scroll_vertical.rs @@ -51,6 +51,32 @@ impl VerticalScroll { true } + pub fn move_area_to_visible( + &self, + height: usize, + start: usize, + end: usize, + ) { + let top = self.top.get(); + let bottom = top + height; + let max_top = self.max_top.get(); + // the top of some content is hidden + if start < top { + self.top.set(start); + return; + } + // the bottom of some content is hidden and there is visible space available + if end > bottom && start > top { + let avail_space = start.saturating_sub(top); + let diff = std::cmp::min( + avail_space, + end.saturating_sub(bottom), + ); + let top = top.saturating_add(diff); + self.top.set(std::cmp::min(max_top, top)); + } + } + pub fn update( &self, selection: usize, From 0a1abd213ab78f548e3ca45db719c57c0256008b Mon Sep 17 00:00:00 2001 From: hamflx Date: Mon, 19 Jun 2023 23:40:16 +0800 Subject: [PATCH 3/3] add unittest for VerticalScroll::move_area_to_visible. --- src/components/utils/scroll_vertical.rs | 37 +++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/components/utils/scroll_vertical.rs b/src/components/utils/scroll_vertical.rs index e784187f8e..70d68eef3d 100644 --- a/src/components/utils/scroll_vertical.rs +++ b/src/components/utils/scroll_vertical.rs @@ -162,4 +162,41 @@ mod tests { fn test_scroll_zero_height() { assert_eq!(calc_scroll_top(4, 0, 4, 3), 0); } + + #[test] + fn test_scroll_bottom_into_view() { + let visual_height = 10; + let line_count = 20; + let scroll = VerticalScroll::new(); + scroll.max_top.set(line_count - visual_height); + + // intersecting with the bottom of the visible area + scroll.move_area_to_visible(visual_height, 9, 11); + assert_eq!(scroll.get_top(), 1); + + // completely below the visible area + scroll.move_area_to_visible(visual_height, 15, 17); + assert_eq!(scroll.get_top(), 7); + + // scrolling to the bottom overflow + scroll.move_area_to_visible(visual_height, 30, 40); + assert_eq!(scroll.get_top(), 10); + } + + #[test] + fn test_scroll_top_into_view() { + let visual_height = 10; + let line_count = 20; + let scroll = VerticalScroll::new(); + scroll.max_top.set(line_count - visual_height); + scroll.top.set(4); + + // intersecting with the top of the visible area + scroll.move_area_to_visible(visual_height, 2, 8); + assert_eq!(scroll.get_top(), 2); + + // completely above the visible area + scroll.move_area_to_visible(visual_height, 0, 2); + assert_eq!(scroll.get_top(), 0); + } }