Skip to content

Commit ab14103

Browse files
lusinganderlusingander
authored
Improve numeric prefix (#90)
* Rename * Remove unnecessary processes * Remove navigation left/right from countable user events * Extract functions * Add GoToParent to countable user events * Refactor numeric prefix processing into a standalone function * Use rstest * Display active numeric prefix * Fix numeric prefix processes * Cancel numeric prefix on UserEvent::Cancel * Fix numeric prefix handling in input mode
1 parent d56f338 commit ab14103

File tree

7 files changed

+94
-197
lines changed

7 files changed

+94
-197
lines changed

‎src/app.rs‎

Lines changed: 65 additions & 183 deletions
Original file line numberDiff line numberDiff line change
@@ -133,33 +133,45 @@ impl App<'_> {
133133
}
134134
}
135135

136-
match self.keybind.get(&key) {
137-
Some(UserEvent::ForceQuit) => {
136+
let user_event = self.keybind.get(&key);
137+
138+
if let Some(UserEvent::Cancel) = user_event {
139+
if !self.numeric_prefix.is_empty() {
140+
// Clear numeric prefix and cancel the event
138141
self.numeric_prefix.clear();
142+
continue;
143+
}
144+
}
145+
146+
match user_event {
147+
Some(UserEvent::ForceQuit) => {
139148
self.tx.send(AppEvent::Quit);
140149
}
141150
Some(ue) => {
142-
let event_with_count = self.process_numeric_prefix(*ue, key);
143-
if let Some(event_with_count) = event_with_count {
144-
self.view.handle_event_with_count(event_with_count, key);
145-
self.numeric_prefix.clear();
146-
}
151+
let event_with_count =
152+
process_numeric_prefix(&self.numeric_prefix, *ue, key);
153+
self.view.handle_event(event_with_count, key);
154+
self.numeric_prefix.clear();
147155
}
148156
None => {
149-
if let KeyCode::Char(c) = key.code {
157+
if let StatusLine::Input(_, _, _) = self.status_line {
158+
// In input mode, pass all key events to the view
159+
// fixme: currently, the only thing that processes key_event is searching the list,
160+
// so this probably works, but it's not the right process...
161+
self.numeric_prefix.clear();
162+
self.view.handle_event(
163+
UserEventWithCount::from_event(UserEvent::Unknown),
164+
key,
165+
);
166+
} else if let KeyCode::Char(c) = key.code {
167+
// Accumulate numeric prefix
150168
if c.is_ascii_digit()
151169
&& (c != '0' || !self.numeric_prefix.is_empty())
152170
{
153171
self.numeric_prefix.push(c);
154172
continue;
155173
}
156174
}
157-
158-
self.numeric_prefix.clear();
159-
self.view.handle_event_with_count(
160-
UserEventWithCount::from_event(UserEvent::Unknown),
161-
key,
162-
);
163175
}
164176
}
165177
}
@@ -235,7 +247,13 @@ impl App<'_> {
235247
impl App<'_> {
236248
fn render_status_line(&self, f: &mut Frame, area: Rect) {
237249
let text: Line = match &self.status_line {
238-
StatusLine::None => "".into(),
250+
StatusLine::None => {
251+
if self.numeric_prefix.is_empty() {
252+
Line::raw("")
253+
} else {
254+
Line::raw(self.numeric_prefix.as_str()).fg(self.color_theme.status_input_fg)
255+
}
256+
}
239257
StatusLine::Input(msg, _, transient_msg) => {
240258
let msg_w = console::measure_text_width(msg.as_str());
241259
if let Some(t_msg) = transient_msg {
@@ -287,38 +305,6 @@ impl App<'_> {
287305
}
288306

289307
impl App<'_> {
290-
fn process_numeric_prefix(
291-
&self,
292-
user_event: UserEvent,
293-
_key: KeyEvent,
294-
) -> Option<UserEventWithCount> {
295-
let count = if self.numeric_prefix.is_empty() {
296-
1
297-
} else {
298-
self.numeric_prefix.parse::<usize>().unwrap_or(1)
299-
};
300-
301-
match user_event {
302-
UserEvent::NavigateUp
303-
| UserEvent::NavigateDown
304-
| UserEvent::NavigateLeft
305-
| UserEvent::NavigateRight
306-
| UserEvent::ScrollUp
307-
| UserEvent::ScrollDown
308-
| UserEvent::PageUp
309-
| UserEvent::PageDown
310-
| UserEvent::HalfPageUp
311-
| UserEvent::HalfPageDown => Some(UserEventWithCount::new(user_event, count)),
312-
_ => {
313-
if self.numeric_prefix.is_empty() {
314-
Some(UserEventWithCount::new(user_event, 1))
315-
} else {
316-
None
317-
}
318-
}
319-
}
320-
}
321-
322308
fn open_detail(&mut self) {
323309
if let View::List(ref mut view) = self.view {
324310
let commit_list_state = view.take_list_state();
@@ -452,150 +438,46 @@ impl App<'_> {
452438
}
453439
}
454440

455-
#[cfg(test)]
456-
mod tests {
457-
use super::*;
458-
459-
// Helper function to test numeric prefix parsing logic
460-
fn test_process_numeric_prefix_logic(
461-
numeric_prefix: &str,
462-
user_event: UserEvent,
463-
) -> Option<UserEventWithCount> {
441+
fn process_numeric_prefix(
442+
numeric_prefix: &str,
443+
user_event: UserEvent,
444+
_key_event: KeyEvent,
445+
) -> UserEventWithCount {
446+
if user_event.is_countable() {
464447
let count = if numeric_prefix.is_empty() {
465448
1
466449
} else {
467450
numeric_prefix.parse::<usize>().unwrap_or(1)
468451
};
469-
470-
match user_event {
471-
UserEvent::NavigateUp
472-
| UserEvent::NavigateDown
473-
| UserEvent::NavigateLeft
474-
| UserEvent::NavigateRight
475-
| UserEvent::ScrollUp
476-
| UserEvent::ScrollDown
477-
| UserEvent::PageUp
478-
| UserEvent::PageDown
479-
| UserEvent::HalfPageUp
480-
| UserEvent::HalfPageDown => Some(UserEventWithCount::new(user_event, count)),
481-
_ => {
482-
if numeric_prefix.is_empty() {
483-
Some(UserEventWithCount::new(user_event, 1))
484-
} else {
485-
None
486-
}
487-
}
488-
}
489-
}
490-
491-
#[test]
492-
fn test_process_numeric_prefix_no_prefix() {
493-
let result = test_process_numeric_prefix_logic("", UserEvent::NavigateDown);
494-
495-
assert!(result.is_some());
496-
let event_with_count = result.unwrap();
497-
assert_eq!(event_with_count.event, UserEvent::NavigateDown);
498-
assert_eq!(event_with_count.count, 1);
499-
}
500-
501-
#[test]
502-
fn test_process_numeric_prefix_with_prefix() {
503-
let result = test_process_numeric_prefix_logic("5", UserEvent::NavigateDown);
504-
505-
assert!(result.is_some());
506-
let event_with_count = result.unwrap();
507-
assert_eq!(event_with_count.event, UserEvent::NavigateDown);
508-
assert_eq!(event_with_count.count, 5);
509-
}
510-
511-
#[test]
512-
fn test_process_numeric_prefix_invalid_number() {
513-
let result = test_process_numeric_prefix_logic("abc", UserEvent::NavigateDown);
514-
515-
assert!(result.is_some());
516-
let event_with_count = result.unwrap();
517-
assert_eq!(event_with_count.event, UserEvent::NavigateDown);
518-
assert_eq!(event_with_count.count, 1); // Should fallback to 1
519-
}
520-
521-
#[test]
522-
fn test_process_numeric_prefix_countable_events() {
523-
let countable_events = [
524-
UserEvent::NavigateUp,
525-
UserEvent::NavigateDown,
526-
UserEvent::NavigateLeft,
527-
UserEvent::NavigateRight,
528-
UserEvent::ScrollUp,
529-
UserEvent::ScrollDown,
530-
UserEvent::PageUp,
531-
UserEvent::PageDown,
532-
UserEvent::HalfPageUp,
533-
UserEvent::HalfPageDown,
534-
];
535-
536-
for event in countable_events {
537-
let result = test_process_numeric_prefix_logic("3", event);
538-
assert!(result.is_some());
539-
let event_with_count = result.unwrap();
540-
assert_eq!(event_with_count.event, event);
541-
assert_eq!(event_with_count.count, 3);
542-
}
543-
}
544-
545-
#[test]
546-
fn test_process_numeric_prefix_non_countable_events() {
547-
let non_countable_events = [
548-
UserEvent::Quit,
549-
UserEvent::Confirm,
550-
UserEvent::Cancel,
551-
UserEvent::HelpToggle,
552-
UserEvent::Search,
553-
UserEvent::ShortCopy,
554-
UserEvent::FullCopy,
555-
];
556-
557-
for event in non_countable_events {
558-
let result = test_process_numeric_prefix_logic("5", event);
559-
assert!(result.is_none()); // Should return None when prefix exists but event isn't countable
560-
}
561-
}
562-
563-
#[test]
564-
fn test_process_numeric_prefix_non_countable_events_no_prefix() {
565-
let result = test_process_numeric_prefix_logic("", UserEvent::Confirm);
566-
assert!(result.is_some());
567-
let event_with_count = result.unwrap();
568-
assert_eq!(event_with_count.event, UserEvent::Confirm);
569-
assert_eq!(event_with_count.count, 1);
570-
}
571-
572-
#[test]
573-
fn test_process_numeric_prefix_large_numbers() {
574-
let result = test_process_numeric_prefix_logic("999", UserEvent::NavigateDown);
575-
576-
assert!(result.is_some());
577-
let event_with_count = result.unwrap();
578-
assert_eq!(event_with_count.event, UserEvent::NavigateDown);
579-
assert_eq!(event_with_count.count, 999);
452+
UserEventWithCount::new(user_event, count)
453+
} else {
454+
UserEventWithCount::from_event(user_event)
580455
}
456+
}
581457

582-
#[test]
583-
fn test_process_numeric_prefix_zero() {
584-
let result = test_process_numeric_prefix_logic("0", UserEvent::NavigateUp);
585-
586-
assert!(result.is_some());
587-
let event_with_count = result.unwrap();
588-
assert_eq!(event_with_count.event, UserEvent::NavigateUp);
589-
assert_eq!(event_with_count.count, 1); // UserEventWithCount::new converts 0 to 1
590-
}
458+
#[cfg(test)]
459+
mod tests {
460+
use rstest::rstest;
591461

592-
#[test]
593-
fn test_process_numeric_prefix_multi_digit() {
594-
let result = test_process_numeric_prefix_logic("42", UserEvent::ScrollDown);
462+
use super::*;
595463

596-
assert!(result.is_some());
597-
let event_with_count = result.unwrap();
598-
assert_eq!(event_with_count.event, UserEvent::ScrollDown);
599-
assert_eq!(event_with_count.count, 42);
464+
#[rustfmt::skip]
465+
#[rstest]
466+
#[case("", UserEvent::NavigateDown, UserEventWithCount::new(UserEvent::NavigateDown, 1))] // no prefix
467+
#[case("5", UserEvent::NavigateUp, UserEventWithCount::new(UserEvent::NavigateUp, 5))] // with prefix
468+
#[case("0", UserEvent::PageDown, UserEventWithCount::new(UserEvent::PageDown, 1))] // zero should be converted to 1
469+
#[case("42", UserEvent::ScrollDown, UserEventWithCount::new(UserEvent::ScrollDown, 42))] // multi-digit number
470+
#[case("999", UserEvent::PageDown, UserEventWithCount::new(UserEvent::PageDown, 999))] // large number
471+
#[case("abc", UserEvent::ScrollUp, UserEventWithCount::new(UserEvent::ScrollUp, 1))] // should fallback to 1
472+
#[case("5", UserEvent::Quit, UserEventWithCount::new(UserEvent::Quit, 1))] // non-countable event with prefix
473+
#[case("", UserEvent::Confirm, UserEventWithCount::new(UserEvent::Confirm, 1))] // non-countable event without prefix
474+
fn test_process_numeric_prefix(
475+
#[case] numeric_prefix: &str,
476+
#[case] user_event: UserEvent,
477+
#[case] expected: UserEventWithCount,
478+
) {
479+
let dummy_key_event = KeyEvent::from(KeyCode::Enter); // KeyEvent is not used in the logic
480+
let actual = process_numeric_prefix(numeric_prefix, user_event, dummy_key_event);
481+
assert_eq!(actual, expected);
600482
}
601483
}

‎src/event.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,23 @@ pub enum UserEvent {
118118
Unknown,
119119
}
120120

121+
impl UserEvent {
122+
pub fn is_countable(&self) -> bool {
123+
matches!(
124+
self,
125+
UserEvent::NavigateUp
126+
| UserEvent::NavigateDown
127+
| UserEvent::ScrollUp
128+
| UserEvent::ScrollDown
129+
| UserEvent::GoToParent
130+
| UserEvent::PageUp
131+
| UserEvent::PageDown
132+
| UserEvent::HalfPageUp
133+
| UserEvent::HalfPageDown
134+
)
135+
}
136+
}
137+
121138
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
122139
pub struct UserEventWithCount {
123140
pub event: UserEvent,

‎src/view/detail.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl<'a> DetailView<'a> {
5858
}
5959
}
6060

61-
pub fn handle_event_with_count(&mut self, event_with_count: UserEventWithCount, _: KeyEvent) {
61+
pub fn handle_event(&mut self, event_with_count: UserEventWithCount, _: KeyEvent) {
6262
let event = event_with_count.event;
6363
let count = event_with_count.count;
6464

‎src/view/help.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ impl HelpView<'_> {
5656
}
5757
}
5858

59-
pub fn handle_event_with_count(&mut self, event_with_count: UserEventWithCount, _: KeyEvent) {
59+
pub fn handle_event(&mut self, event_with_count: UserEventWithCount, _: KeyEvent) {
6060
let event = event_with_count.event;
6161
let count = event_with_count.count;
6262

‎src/view/list.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ impl<'a> ListView<'a> {
3131
}
3232
}
3333

34-
pub fn handle_event_with_count(&mut self, event_with_count: UserEventWithCount, key: KeyEvent) {
34+
pub fn handle_event(&mut self, event_with_count: UserEventWithCount, key: KeyEvent) {
3535
let event = event_with_count.event;
3636
let count = event_with_count.count;
3737
if let SearchState::Searching { .. } = self.as_list_state().search_state() {
@@ -74,7 +74,9 @@ impl<'a> ListView<'a> {
7474
}
7575
}
7676
UserEvent::GoToParent => {
77-
self.as_mut_list_state().select_parent();
77+
for _ in 0..count {
78+
self.as_mut_list_state().select_parent();
79+
}
7880
}
7981
UserEvent::GoToTop => {
8082
self.as_mut_list_state().select_first();

‎src/view/refs.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl<'a> RefsView<'a> {
4545
}
4646
}
4747

48-
pub fn handle_event_with_count(&mut self, event_with_count: UserEventWithCount, _: KeyEvent) {
48+
pub fn handle_event(&mut self, event_with_count: UserEventWithCount, _: KeyEvent) {
4949
let event = event_with_count.event;
5050
let count = event_with_count.count;
5151

‎src/view/views.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,13 @@ pub enum View<'a> {
2222
}
2323

2424
impl<'a> View<'a> {
25-
pub fn handle_event_with_count(
26-
&mut self,
27-
event_with_count: UserEventWithCount,
28-
key_event: KeyEvent,
29-
) {
25+
pub fn handle_event(&mut self, event_with_count: UserEventWithCount, key_event: KeyEvent) {
3026
match self {
3127
View::Default => {}
32-
View::List(view) => view.handle_event_with_count(event_with_count, key_event),
33-
View::Detail(view) => view.handle_event_with_count(event_with_count, key_event),
34-
View::Refs(view) => view.handle_event_with_count(event_with_count, key_event),
35-
View::Help(view) => view.handle_event_with_count(event_with_count, key_event),
28+
View::List(view) => view.handle_event(event_with_count, key_event),
29+
View::Detail(view) => view.handle_event(event_with_count, key_event),
30+
View::Refs(view) => view.handle_event(event_with_count, key_event),
31+
View::Help(view) => view.handle_event(event_with_count, key_event),
3632
}
3733
}
3834

0 commit comments

Comments
 (0)