From 85c1574be46b34695019b087334da4ccedf7f309 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Thu, 9 May 2024 14:27:15 -0400 Subject: [PATCH 1/3] Change Page Up and Page Down keybindings This changes the bindings as follows: For page up: - Before: page-up OR ctrl-b - After: ctrl-page-up OR ctrl-b For page down: - Before: page-down OR ctrl-f - After: ctrl-page-down OR ctrl-f The page-up and page-down keys will be repurposed in a subsequent commit to jump between editable sections. --- src/ui.rs | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/src/ui.rs b/src/ui.rs index b7549b6..bb42113 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -189,34 +189,18 @@ impl From for Event { modifiers: _, }) => Self::ScrollDown, - Event::Key( - KeyEvent { - code: KeyCode::PageUp, - modifiers: KeyModifiers::NONE, - kind: KeyEventKind::Press, - state: _, - } - | KeyEvent { - code: KeyCode::Char('b'), - modifiers: KeyModifiers::CONTROL, - kind: KeyEventKind::Press, - state: _, - }, - ) => Self::PageUp, - Event::Key( - KeyEvent { - code: KeyCode::PageDown, - modifiers: KeyModifiers::NONE, - kind: KeyEventKind::Press, - state: _, - } - | KeyEvent { - code: KeyCode::Char('f'), - modifiers: KeyModifiers::CONTROL, - kind: KeyEventKind::Press, - state: _, - }, - ) => Self::PageDown, + Event::Key(KeyEvent { + code: KeyCode::PageUp | KeyCode::Char('b'), + modifiers: KeyModifiers::CONTROL, + kind: KeyEventKind::Press, + state: _, + }) => Self::PageUp, + Event::Key(KeyEvent { + code: KeyCode::PageDown | KeyCode::Char('f'), + modifiers: KeyModifiers::CONTROL, + kind: KeyEventKind::Press, + state: _, + }) => Self::PageDown, Event::Key(KeyEvent { code: KeyCode::Up | KeyCode::Char('k'), @@ -776,11 +760,11 @@ impl<'state, 'input> Recorder<'state, 'input> { event: Event::ScrollDown, }, MenuItem { - label: Cow::Borrowed("Page up (page-up, ctrl-b)"), + label: Cow::Borrowed("Previous page (ctrl-page-up, ctrl-b)"), event: Event::PageUp, }, MenuItem { - label: Cow::Borrowed("Page down (page-down, ctrl-f)"), + label: Cow::Borrowed("Next page (ctrl-page-down, ctrl-f)"), event: Event::PageDown, }, ], From e96a31e762238971a9595d082f036dfd09b20398 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Thu, 9 May 2024 14:34:20 -0400 Subject: [PATCH 2/3] Add new keybindings for Scroll Up and Scroll Down These new keybindings are added alongside the existing keybindings. For scroll up: - Before: ctrl-y - After: ctrl-y OR ctrl-up For scroll down: - Before: ctrl-e - After: ctrl-e OR ctrl-down --- src/ui.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ui.rs b/src/ui.rs index bb42113..ddfdc3f 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -165,7 +165,7 @@ impl From for Event { }) => Self::QuitAccept, Event::Key(KeyEvent { - code: KeyCode::Char('y'), + code: KeyCode::Up | KeyCode::Char('y'), modifiers: KeyModifiers::CONTROL, kind: KeyEventKind::Press, state: _, @@ -177,7 +177,7 @@ impl From for Event { modifiers: _, }) => Self::ScrollUp, Event::Key(KeyEvent { - code: KeyCode::Char('e'), + code: KeyCode::Down | KeyCode::Char('e'), modifiers: KeyModifiers::CONTROL, kind: KeyEventKind::Press, state: _, @@ -752,11 +752,11 @@ impl<'state, 'input> Recorder<'state, 'input> { event: Event::ExpandAll, }, MenuItem { - label: Cow::Borrowed("Scroll up (ctrl-y)"), + label: Cow::Borrowed("Scroll up (ctrl-up, ctrl-y)"), event: Event::ScrollUp, }, MenuItem { - label: Cow::Borrowed("Scroll down (ctrl-e)"), + label: Cow::Borrowed("Scroll down (ctrl-down, ctrl-e)"), event: Event::ScrollDown, }, MenuItem { From 96a2db270a5939b458aecd9c1fb5bf8b907d6cca Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Thu, 9 May 2024 14:42:12 -0400 Subject: [PATCH 3/3] Add keybindings to jump to next section of the same type For example, if the currently highlighted row is a section header, the page down key will jump to the next section. If the currently highlighted row is a line change, the page down key will jump to the next selectable line. The new keybindings are page-up and page-down. --- src/ui.rs | 70 ++++++++ tests/test_scm_record.rs | 358 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 428 insertions(+) diff --git a/src/ui.rs b/src/ui.rs index ddfdc3f..01919da 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -123,8 +123,10 @@ pub enum Event { PageUp, PageDown, FocusPrev, + FocusPrevSameKind, // focus on the previous item of the same kind (i.e. file, section, line) FocusPrevPage, FocusNext, + FocusNextSameKind, // focus on the next item of the same kind FocusNextPage, FocusInner, FocusOuter, @@ -215,6 +217,19 @@ impl From for Event { state: _, }) => Self::FocusNext, + Event::Key(KeyEvent { + code: KeyCode::PageUp, + modifiers: KeyModifiers::NONE, + kind: KeyEventKind::Press, + state: _, + }) => Self::FocusPrevSameKind, + Event::Key(KeyEvent { + code: KeyCode::PageDown, + modifiers: KeyModifiers::NONE, + kind: KeyEventKind::Press, + state: _, + }) => Self::FocusNextSameKind, + Event::Key(KeyEvent { code: KeyCode::Left | KeyCode::Char('h'), modifiers: KeyModifiers::NONE, @@ -722,6 +737,14 @@ impl<'state, 'input> Recorder<'state, 'input> { label: Cow::Borrowed("Next item (down, j)"), event: Event::FocusNext, }, + MenuItem { + label: Cow::Borrowed("Previous item of the same kind (page-up)"), + event: Event::FocusPrevSameKind, + }, + MenuItem { + label: Cow::Borrowed("Next item of the same kind (page-down)"), + event: Event::FocusNextSameKind, + }, MenuItem { label: Cow::Borrowed("Outer item (left, h)"), event: Event::FocusOuter, @@ -1040,6 +1063,8 @@ impl<'state, 'input> Recorder<'state, 'input> { | Event::PageDown | Event::FocusPrev | Event::FocusNext + | Event::FocusPrevSameKind + | Event::FocusNextSameKind | Event::FocusPrevPage | Event::FocusNextPage | Event::ToggleAll @@ -1082,6 +1107,22 @@ impl<'state, 'input> Recorder<'state, 'input> { ensure_in_viewport: true, } } + (None, Event::FocusPrevSameKind) => { + let selection_key = + self.select_prev_or_next_of_same_kind(/*select_previous=*/ true); + StateUpdate::SelectItem { + selection_key, + ensure_in_viewport: true, + } + } + (None, Event::FocusNextSameKind) => { + let selection_key = + self.select_prev_or_next_of_same_kind(/*select_previous=*/ false); + StateUpdate::SelectItem { + selection_key, + ensure_in_viewport: true, + } + } (None, Event::FocusPrevPage) => { let selection_key = self.select_prev_page(term_height, drawn_rects); StateUpdate::SelectItem { @@ -1293,6 +1334,35 @@ impl<'state, 'input> Recorder<'state, 'input> { } } + // Returns the previous or next SelectionKey of the same kind as the current + // selection key. If there are no other keys of the same kind, the current + // key is returned instead. If `select_previous` is true, the previous key + // is returned. Otherwise, the next key is returned. + fn select_prev_or_next_of_same_kind(&self, select_previous: bool) -> SelectionKey { + let (keys, index) = self.find_selection(); + let iterate_keys_with_wrap_around = |i| -> Box> { + let forward_iter = keys[i + 1..] // Skip the current key + .iter() + .chain(keys[..i].iter()); + if select_previous { + Box::new(forward_iter.rev()) + } else { + Box::new(forward_iter) + } + }; + match index { + None => self.first_selection_key(), + Some(index) => { + match iterate_keys_with_wrap_around(index) + .find(|k| std::mem::discriminant(*k) == std::mem::discriminant(&keys[index])) + { + None => keys[index], + Some(key) => *key, + } + } + } + } + fn select_prev_page( &self, term_height: usize, diff --git a/tests/test_scm_record.rs b/tests/test_scm_record.rs index 0405843..510fd49 100644 --- a/tests/test_scm_record.rs +++ b/tests/test_scm_record.rs @@ -2817,6 +2817,364 @@ fn test_quit_dialog_when_commit_message_provided() -> eyre::Result<()> { Ok(()) } +#[test] +fn test_prev_same_kind() -> eyre::Result<()> { + let initial = TestingScreenshot::default(); + let to_baz = TestingScreenshot::default(); + let to_baz_section = TestingScreenshot::default(); + let to_bar_section = TestingScreenshot::default(); + let to_baz_lines = TestingScreenshot::default(); + let mut input = TestingInput::new( + 80, + 20, + [ + Event::ExpandAll, + initial.event(), + // Moves the current item from foo/bar to baz + Event::FocusPrevSameKind, + to_baz.event(), + Event::FocusInner, + to_baz_section.event(), + Event::FocusPrevSameKind, + to_bar_section.event(), + Event::FocusInner, + Event::FocusPrevSameKind, + Event::FocusPrevSameKind, + to_baz_lines.event(), + Event::QuitAccept, + ], + ); + let state = example_contents(); + let recorder = Recorder::new(state, &mut input); + recorder.run()?; + + insta::assert_snapshot!(initial, @r###" + "[File] [Edit] [Select] [View] " + "(◐) foo/bar (-)" + " ⋮ " + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + "###); + insta::assert_snapshot!(to_baz, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "(●) baz (-)" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + insta::assert_snapshot!(to_baz_section, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " (●) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + insta::assert_snapshot!(to_bar_section, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " (◐) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + insta::assert_snapshot!(to_baz_lines, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " (●) + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + Ok(()) +} + +#[test] +fn test_next_same_kind() -> eyre::Result<()> { + let initial = TestingScreenshot::default(); + let to_baz = TestingScreenshot::default(); + let to_baz_section = TestingScreenshot::default(); + let to_bar_section = TestingScreenshot::default(); + let to_bar_lines = TestingScreenshot::default(); + let mut input = TestingInput::new( + 80, + 20, + [ + Event::ExpandAll, + initial.event(), + // Moves the current item from foo/bar to baz + Event::FocusNextSameKind, + to_baz.event(), + Event::FocusInner, + to_baz_section.event(), + Event::FocusNextSameKind, + to_bar_section.event(), + Event::FocusInner, + Event::FocusNextSameKind, + Event::FocusNextSameKind, + to_bar_lines.event(), + Event::QuitAccept, + ], + ); + let state = example_contents(); + let recorder = Recorder::new(state, &mut input); + recorder.run()?; + + insta::assert_snapshot!(initial, @r###" + "[File] [Edit] [Select] [View] " + "(◐) foo/bar (-)" + " ⋮ " + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + "###); + insta::assert_snapshot!(to_baz, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "(●) baz (-)" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + insta::assert_snapshot!(to_baz_section, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " (●) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + insta::assert_snapshot!(to_bar_section, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " (◐) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + insta::assert_snapshot!(to_bar_lines, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " [◐] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " (●) + after text 1⏎ " + " [ ] + after text 2⏎ " + " 23 this is some trailing text⏎ " + "[●] baz [-]" + " 1 Some leading text 1⏎ " + " 2 Some leading text 2⏎ " + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + " 5 this is some trailing text⏎ " + "###); + Ok(()) +} + +// Test the prev/next same kind keybindings when there is only a single section +// of a given kind. +#[test] +fn test_prev_next_same_kind_single_section() -> eyre::Result<()> { + let initial = TestingScreenshot::default(); + let next = TestingScreenshot::default(); + let prev = TestingScreenshot::default(); + let mut input = TestingInput::new( + 80, + 10, + [ + Event::ExpandAll, + // Move down to the section so the current selection isn't the + // first item. + Event::FocusNext, + initial.event(), + // Moves the current item from foo/bar to baz + Event::FocusNextSameKind, + next.event(), + Event::FocusPrevSameKind, + prev.event(), + Event::QuitAccept, + ], + ); + let mut state = example_contents(); + // Change the example so that there's only a single file. + state.files = vec![state.files[0].clone()]; + let recorder = Recorder::new(state, &mut input); + recorder.run()?; + // Since we start at the foo/bar file section and there are no other + // sections, the current section never changes. + insta::assert_snapshot!(initial, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " (◐) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + "###); + insta::assert_snapshot!(next, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " (◐) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + "###); + insta::assert_snapshot!(prev, @r###" + "[File] [Edit] [Select] [View] " + "[◐] foo/bar [-]" + " 18 this is some text⏎ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + " (◐) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [ ] + after text 2⏎ " + "###); + Ok(()) +} + #[cfg(feature = "serde")] #[test] fn test_deserialize() -> eyre::Result<()> {