From a8e3340777e64fa0aa9290a73e4175afa6274d08 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Mon, 22 Jul 2024 15:23:40 -0400 Subject: [PATCH] Allow "left arrow" and "h" to collapse text sections After this change the "left arrow" and "h" keypresses will collapse a text section if it is expanded and currently selected. This allows users to navigate around a diff more easily since they don't need to scroll through all of the lines in a section to get to the next section. Instead, they can just collapse the section quickly and move to the next one. This fixes one of my only gripes about scm-record compared to the builtin editor in Mercurial. --- scm-record/src/ui.rs | 77 +++++++++++----- scm-record/tests/test_scm_record.rs | 131 ++++++++++++++++++++++++++-- 2 files changed, 180 insertions(+), 28 deletions(-) diff --git a/scm-record/src/ui.rs b/scm-record/src/ui.rs index b0afa67..8bdb84e 100644 --- a/scm-record/src/ui.rs +++ b/scm-record/src/ui.rs @@ -124,20 +124,30 @@ pub enum Event { PageUp, PageDown, FocusPrev, - FocusPrevSameKind, // focus on the previous item of the same kind (i.e. file, section, line) + /// Move focus to the previous item of the same kind (i.e. file, section, line). + FocusPrevSameKind, FocusPrevPage, FocusNext, - FocusNextSameKind, // focus on the next item of the same kind + /// Move focus to the next item of the same kind. + FocusNextSameKind, FocusNextPage, FocusInner, - FocusOuter, + /// If `fold_section` is true, and the current section is expanded, the + /// section should be collapsed without moving focus. Otherwise, move the + /// focus outwards. + FocusOuter { + fold_section: bool, + }, ToggleItem, ToggleItemAndAdvance, ToggleAll, ToggleAllUniform, ExpandItem, ExpandAll, - Click { row: usize, column: usize }, + Click { + row: usize, + column: usize, + }, ToggleCommitViewMode, // no key binding currently EditCommitMessage, Help, @@ -248,13 +258,22 @@ impl From for Event { Event::Key(KeyEvent { code: KeyCode::Left | KeyCode::Char('h'), - modifiers: KeyModifiers::NONE | KeyModifiers::CONTROL, + modifiers: KeyModifiers::SHIFT, + kind: KeyEventKind::Press, + state: _, + }) => Self::FocusOuter { + fold_section: false, + }, + Event::Key(KeyEvent { + code: KeyCode::Left | KeyCode::Char('h'), + modifiers: KeyModifiers::NONE, kind: KeyEventKind::Press, state: _, - }) => Self::FocusOuter, + }) => Self::FocusOuter { fold_section: true }, Event::Key(KeyEvent { code: KeyCode::Right | KeyCode::Char('l'), - modifiers: KeyModifiers::NONE | KeyModifiers::CONTROL, + // The shift modifier is accepted for continuity with FocusOuter. + modifiers: KeyModifiers::NONE | KeyModifiers::SHIFT, kind: KeyEventKind::Press, state: _, }) => Self::FocusInner, @@ -774,11 +793,19 @@ impl<'state, 'input> Recorder<'state, 'input> { event: Event::FocusNextSameKind, }, MenuItem { - label: Cow::Borrowed("Outer item (left, h)"), - event: Event::FocusOuter, + label: Cow::Borrowed( + "Outer item without folding (shift-left, shift-h)", + ), + event: Event::FocusOuter { + fold_section: false, + }, + }, + MenuItem { + label: Cow::Borrowed("Outer item with folding (left, h)"), + event: Event::FocusOuter { fold_section: true }, }, MenuItem { - label: Cow::Borrowed("Inner item (right, l)"), + label: Cow::Borrowed("Inner item with unfolding (right, l)"), event: Event::FocusInner, }, MenuItem { @@ -1070,7 +1097,7 @@ impl<'state, 'input> Recorder<'state, 'input> { // If pressing ctrl-c again wile the dialog is open, force quit. (Some(_), Event::QuitInterrupt) => StateUpdate::QuitCancel, // Select left quit dialog button. - (Some(quit_dialog), Event::FocusOuter) => { + (Some(quit_dialog), Event::FocusOuter { .. }) => { StateUpdate::SetQuitDialog(Some(QuitDialog { focused_button: QuitDialogButtonId::GoBack, ..quit_dialog.clone() @@ -1179,7 +1206,7 @@ impl<'state, 'input> Recorder<'state, 'input> { ensure_in_viewport: true, } } - (None, Event::FocusOuter) => self.select_outer(), + (None, Event::FocusOuter { fold_section }) => self.select_outer(fold_section), (None, Event::FocusInner) => { let selection_key = self.select_inner(); StateUpdate::SelectItem { @@ -1490,23 +1517,31 @@ impl<'state, 'input> Recorder<'state, 'input> { .unwrap_or(self.selection_key) } - fn select_outer(&self) -> StateUpdate { + fn select_outer(&self, fold_section: bool) -> StateUpdate { match self.selection_key { SelectionKey::None => StateUpdate::None, selection_key @ SelectionKey::File(_) => { StateUpdate::SetExpandItem(selection_key, false) } - SelectionKey::Section(SectionKey { + selection_key @ SelectionKey::Section(SectionKey { commit_idx, file_idx, section_idx: _, - }) => StateUpdate::SelectItem { - selection_key: SelectionKey::File(FileKey { - commit_idx, - file_idx, - }), - ensure_in_viewport: true, - }, + }) => { + // If folding is requested and the selection is expanded, + // collapse it. Otherwise, move the selection to the file. + if fold_section && self.expanded_items.contains(&selection_key) { + StateUpdate::SetExpandItem(selection_key, false) + } else { + StateUpdate::SelectItem { + selection_key: SelectionKey::File(FileKey { + commit_idx, + file_idx, + }), + ensure_in_viewport: true, + } + } + } SelectionKey::Line(LineKey { commit_idx, file_idx, diff --git a/scm-record/tests/test_scm_record.rs b/scm-record/tests/test_scm_record.rs index e8a425c..31495e1 100644 --- a/scm-record/tests/test_scm_record.rs +++ b/scm-record/tests/test_scm_record.rs @@ -436,6 +436,7 @@ fn test_quit_dialog_buttons() -> TestResult { let expect_back_button_closes_quit_dialog = TestingScreenshot::default(); let expect_right_focuses_quit_button = TestingScreenshot::default(); let expect_right_again_does_not_wrap = TestingScreenshot::default(); + let expect_ctrl_left_focuses_go_back_button = TestingScreenshot::default(); let expect_exited = TestingScreenshot::default(); let mut input = TestingInput::new( 80, @@ -445,23 +446,30 @@ fn test_quit_dialog_buttons() -> TestResult { Event::QuitCancel, expect_quit_button_focused_initially.event(), // Pressing left should select the back button. - Event::FocusOuter, + Event::FocusOuter { fold_section: true }, expect_left_focuses_go_back_button.event(), // Pressing left again should do nothing. - Event::FocusOuter, + Event::FocusOuter { fold_section: true }, expect_left_again_does_not_wrap.event(), // Selecting the back button should close the dialog. Event::ToggleItem, expect_back_button_closes_quit_dialog.event(), Event::QuitCancel, // Pressing right should select the quit button. - Event::FocusOuter, + Event::FocusOuter { fold_section: true }, Event::FocusInner, expect_right_focuses_quit_button.event(), // Pressing right again should do nothing. Event::FocusInner, expect_right_again_does_not_wrap.event(), + // We have two ways to focus outer, with and without folding. + // Both should navigate properly in this menu. + Event::FocusOuter { + fold_section: false, + }, + expect_ctrl_left_focuses_go_back_button.event(), // Selecting the quit button should quit. + Event::FocusInner, Event::ToggleItem, expect_exited.event(), ], @@ -517,6 +525,14 @@ fn test_quit_dialog_buttons() -> TestResult { " 19 this is some text⏎ " " 20 this is some text⏎ " "###); + insta::assert_snapshot!(expect_ctrl_left_focuses_go_back_button, @r###" + "[File] [Edit] [Select] [View] " + "(◐) foo/b┌Quit───────────────────────────────────────────────────────┐ (-)" + " ⋮│You have changes to 2 files. Are you sure you want to quit?│ " + " 18└───────────────────────────────────────────(Go Back)─[Quit]┘ " + " 19 this is some text⏎ " + " 20 this is some text⏎ " + "###); insta::assert_snapshot!(expect_exited, @""); Ok(()) } @@ -1772,13 +1788,21 @@ fn test_focus_outer() -> TestResult { Event::FocusNext, Event::FocusNext, initial.event(), - Event::FocusOuter, + Event::FocusOuter { + fold_section: false, + }, outer1.event(), - Event::FocusOuter, + Event::FocusOuter { + fold_section: false, + }, outer2.event(), - Event::FocusOuter, + Event::FocusOuter { + fold_section: false, + }, outer3.event(), - Event::FocusOuter, + Event::FocusOuter { + fold_section: false, + }, outer4.event(), Event::QuitAccept, ], @@ -1835,6 +1859,99 @@ fn test_focus_outer() -> TestResult { Ok(()) } +#[test] +fn test_focus_outer_fold_section() -> TestResult { + let state = example_contents(); + let initial = TestingScreenshot::default(); + let outer1 = TestingScreenshot::default(); + let outer2 = TestingScreenshot::default(); + let outer3 = TestingScreenshot::default(); + let outer4 = TestingScreenshot::default(); + let outer5 = TestingScreenshot::default(); + let mut input = TestingInput::new( + 80, + 7, + [ + Event::FocusNext, + Event::ExpandItem, + Event::FocusNext, + Event::FocusNext, + Event::FocusNext, + initial.event(), + Event::FocusOuter { fold_section: true }, + outer1.event(), + Event::FocusOuter { fold_section: true }, + outer2.event(), + Event::FocusOuter { fold_section: true }, + outer3.event(), + Event::FocusOuter { fold_section: true }, + outer4.event(), + Event::FocusOuter { fold_section: true }, + outer5.event(), + Event::QuitAccept, + ], + ); + let recorder = Recorder::new(state, &mut input); + recorder.run()?; + + insta::assert_snapshot!(initial, @r###" + "[File] [Edit] [Select] [View] " + "[●] baz [-]" + " [●] Section 1/1 [-]" + " [●] - before text 1⏎ " + " (●) - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + "###); + insta::assert_snapshot!(outer1, @r###" + "[File] [Edit] [Select] [View] " + "[●] baz [-]" + " (●) Section 1/1 (-)" + " [●] - before text 1⏎ " + " [●] - before text 2⏎ " + " [●] + after text 1⏎ " + " [●] + after text 2⏎ " + "###); + insta::assert_snapshot!(outer2, @r###" + "[File] [Edit] [Select] [View] " + "[●] baz [±]" + " (●) Section 1/1 (+)" + " " + " " + " " + " " + "###); + insta::assert_snapshot!(outer3, @r###" + "[File] [Edit] [Select] [View] " + "(●) baz (±)" + " [●] Section 1/1 [+]" + " " + " " + " " + " " + "###); + insta::assert_snapshot!(outer4, @r###" + "[File] [Edit] [Select] [View] " + "(●) baz (+)" + " " + " " + " " + " " + " " + "###); + insta::assert_snapshot!(outer5, @r###" + "[File] [Edit] [Select] [View] " + "(●) baz (+)" + " " + " " + " " + " " + " " + "###); + + Ok(()) +} + #[test] fn test_sticky_header_scroll() -> TestResult { let state = example_contents();