Skip to content

Commit

Permalink
Allow "left arrow" and "h" to collapse text sections
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
emesterhazy committed Jan 9, 2025
1 parent 85463e1 commit a8e3340
Show file tree
Hide file tree
Showing 2 changed files with 180 additions and 28 deletions.
77 changes: 56 additions & 21 deletions scm-record/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -248,13 +258,22 @@ impl From<crossterm::event::Event> 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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
131 changes: 124 additions & 7 deletions scm-record/tests/test_scm_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
],
Expand Down Expand Up @@ -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, @"<this screenshot was never assigned>");
Ok(())
}
Expand Down Expand Up @@ -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,
],
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit a8e3340

Please sign in to comment.