Merge 6e68424ab1 into 8619c07f3f

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Added
+* vim-like navigation: `j`/`k` for up/down, `l` to open preview
+
 ### Changed
 * use [tombi](https://github.com/tombi-toml/tombi) for all toml file formatting
 * open the external editor from the status diff view [[@WaterWhisperer](https://github.com/WaterWhisperer)] ([#2805](https://github.com/gitui-org/gitui/issues/2805))

src/components/commit_details/details.rs

@@ -361,40 +361,26 @@ impl Component for DetailsComponent {
 	fn event(&mut self, event: &Event) -> Result<EventState> {
 		if self.focused {
 			if let Event::Key(e) = event {
-				return Ok(
-					if key_match(e, self.key_config.keys.move_up) {
-						self.move_scroll_top(ScrollType::Up).into()
-					} else if key_match(
-						e,
-						self.key_config.keys.move_down,
-					) {
-						self.move_scroll_top(ScrollType::Down).into()
-					} else if key_match(
-						e,
-						self.key_config.keys.page_up,
-					) {
-						self.move_scroll_top(ScrollType::PageUp)
-							.into()
-					} else if key_match(
-						e,
-						self.key_config.keys.page_down,
-					) {
-						self.move_scroll_top(ScrollType::PageDown)
-							.into()
-					} else if key_match(e, self.key_config.keys.home)
-						|| key_match(e, self.key_config.keys.shift_up)
-					{
-						self.move_scroll_top(ScrollType::Home).into()
-					} else if key_match(e, self.key_config.keys.end)
-						|| key_match(
-							e,
-							self.key_config.keys.shift_down,
-						) {
-						self.move_scroll_top(ScrollType::End).into()
-					} else {
-						EventState::NotConsumed
-					},
-				);
+				return Ok(if self.key_config.is_nav_up(e) {
+					self.move_scroll_top(ScrollType::Up).into()
+				} else if self.key_config.is_nav_down(e) {
+					self.move_scroll_top(ScrollType::Down).into()
+				} else if key_match(e, self.key_config.keys.page_up) {
+					self.move_scroll_top(ScrollType::PageUp).into()
+				} else if key_match(e, self.key_config.keys.page_down)
+				{
+					self.move_scroll_top(ScrollType::PageDown).into()
+				} else if key_match(e, self.key_config.keys.home)
+					|| key_match(e, self.key_config.keys.shift_up)
+				{
+					self.move_scroll_top(ScrollType::Home).into()
+				} else if key_match(e, self.key_config.keys.end)
+					|| key_match(e, self.key_config.keys.shift_down)
+				{
+					self.move_scroll_top(ScrollType::End).into()
+				} else {
+					EventState::NotConsumed
+				});
 			}
 		}
 

src/components/commit_details/mod.rs

@@ -7,10 +7,7 @@ use super::{
 	Component, DrawableComponent, EventState, StatusTreeComponent,
 };
 use crate::{
-	accessors,
-	app::Environment,
-	keys::{key_match, SharedKeyConfig},
-	strings,
+	accessors, app::Environment, keys::SharedKeyConfig, strings,
 };
 use anyhow::Result;
 use asyncgit::{
@@ -208,13 +205,13 @@ impl Component for CommitDetailsComponent {
 
 		if self.focused() {
 			if let Event::Key(e) = ev {
-				return if key_match(e, self.key_config.keys.move_down)
+				return if self.key_config.is_nav_down(e)
 					&& self.details_focused()
 				{
 					self.set_details_focus(false);
 					self.file_tree.focus(true);
 					Ok(EventState::Consumed)
-				} else if key_match(e, self.key_config.keys.move_up)
+				} else if self.key_config.is_nav_up(e)
 					&& self.file_tree.focused()
 					&& !self.is_compare()
 				{

src/components/commitlist.rs

@@ -830,42 +830,37 @@ impl DrawableComponent for CommitList {
 impl Component for CommitList {
 	fn event(&mut self, ev: &Event) -> Result<EventState> {
 		if let Event::Key(k) = ev {
-			let selection_changed =
-				if key_match(k, self.key_config.keys.move_up) {
-					self.move_selection(ScrollType::Up)?
-				} else if key_match(k, self.key_config.keys.move_down)
-				{
-					self.move_selection(ScrollType::Down)?
-				} else if key_match(k, self.key_config.keys.shift_up)
-					|| key_match(k, self.key_config.keys.home)
-				{
-					self.move_selection(ScrollType::Home)?
-				} else if key_match(
-					k,
-					self.key_config.keys.shift_down,
-				) || key_match(k, self.key_config.keys.end)
-				{
-					self.move_selection(ScrollType::End)?
-				} else if key_match(k, self.key_config.keys.page_up) {
-					self.move_selection(ScrollType::PageUp)?
-				} else if key_match(k, self.key_config.keys.page_down)
-				{
-					self.move_selection(ScrollType::PageDown)?
-				} else if key_match(
-					k,
-					self.key_config.keys.log_mark_commit,
-				) {
-					self.mark();
-					true
-				} else if key_match(
-					k,
-					self.key_config.keys.log_checkout_commit,
-				) {
-					self.checkout();
-					true
-				} else {
-					false
-				};
+			let selection_changed = if self.key_config.is_nav_up(k) {
+				self.move_selection(ScrollType::Up)?
+			} else if self.key_config.is_nav_down(k) {
+				self.move_selection(ScrollType::Down)?
+			} else if key_match(k, self.key_config.keys.shift_up)
+				|| key_match(k, self.key_config.keys.home)
+			{
+				self.move_selection(ScrollType::Home)?
+			} else if key_match(k, self.key_config.keys.shift_down)
+				|| key_match(k, self.key_config.keys.end)
+			{
+				self.move_selection(ScrollType::End)?
+			} else if key_match(k, self.key_config.keys.page_up) {
+				self.move_selection(ScrollType::PageUp)?
+			} else if key_match(k, self.key_config.keys.page_down) {
+				self.move_selection(ScrollType::PageDown)?
+			} else if key_match(
+				k,
+				self.key_config.keys.log_mark_commit,
+			) {
+				self.mark();
+				true
+			} else if key_match(
+				k,
+				self.key_config.keys.log_checkout_commit,
+			) {
+				self.checkout();
+				true
+			} else {
+				false
+			};
 			return Ok(selection_changed.into());
 		}
 

src/components/diff.rs

@@ -831,8 +831,7 @@ impl Component for DiffComponent {
 	fn event(&mut self, ev: &Event) -> Result<EventState> {
 		if self.focused() {
 			if let Event::Key(e) = ev {
-				return if key_match(e, self.key_config.keys.move_down)
-				{
+				return if self.key_config.is_nav_down(e) {
 					self.move_selection(ScrollType::Down);
 					Ok(EventState::Consumed)
 				} else if key_match(
@@ -851,7 +850,7 @@ impl Component for DiffComponent {
 				} else if key_match(e, self.key_config.keys.home) {
 					self.move_selection(ScrollType::Home);
 					Ok(EventState::Consumed)
-				} else if key_match(e, self.key_config.keys.move_up) {
+				} else if self.key_config.is_nav_up(e) {
 					self.move_selection(ScrollType::Up);
 					Ok(EventState::Consumed)
 				} else if key_match(e, self.key_config.keys.page_up) {
@@ -861,10 +860,7 @@ impl Component for DiffComponent {
 				{
 					self.move_selection(ScrollType::PageDown);
 					Ok(EventState::Consumed)
-				} else if key_match(
-					e,
-					self.key_config.keys.move_right,
-				) {
+				} else if self.key_config.is_nav_right(e) {
 					self.horizontal_scroll
 						.move_right(HorizontalScrollType::Right);
 					Ok(EventState::Consumed)

src/components/revision_files.rs

@@ -496,8 +496,7 @@ impl Component for RevisionFilesComponent {
 					self.hide();
 					return Ok(EventState::Consumed);
 				}
-			} else if key_match(key, self.key_config.keys.move_right)
-			{
+			} else if self.key_config.is_nav_right(key) {
 				if is_tree_focused {
 					self.focus = Focus::File;
 					self.current_file.focus(true);

src/components/status_tree.rs

@@ -518,12 +518,11 @@ impl Component for StatusTreeComponent {
 				} else if key_match(e, self.key_config.keys.copy) {
 					self.copy_file_path();
 					Ok(EventState::Consumed)
-				} else if key_match(e, self.key_config.keys.move_down)
-				{
+				} else if self.key_config.is_nav_down(e) {
 					Ok(self
 						.move_selection(MoveSelection::Down)
 						.into())
-				} else if key_match(e, self.key_config.keys.move_up) {
+				} else if self.key_config.is_nav_up(e) {
 					Ok(self.move_selection(MoveSelection::Up).into())
 				} else if key_match(e, self.key_config.keys.home)
 					|| key_match(e, self.key_config.keys.shift_up)
@@ -549,10 +548,7 @@ impl Component for StatusTreeComponent {
 					Ok(self
 						.move_selection(MoveSelection::Left)
 						.into())
-				} else if key_match(
-					e,
-					self.key_config.keys.move_right,
-				) {
+				} else if self.key_config.is_nav_right(e) {
 					Ok(self
 						.move_selection(MoveSelection::Right)
 						.into())

src/keys/key_config.rs

@@ -1,11 +1,11 @@
 use anyhow::Result;
-use crossterm::event::{KeyCode, KeyModifiers};
+use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
 use std::{fs::canonicalize, path::PathBuf, rc::Rc};
 
 use crate::{args::get_app_config_path, strings::symbol};
 
 use super::{
-	key_list::{GituiKeyEvent, KeysList},
+	key_list::{key_match, GituiKeyEvent, KeysList},
 	symbols::KeySymbols,
 };
 
@@ -52,6 +52,24 @@ impl KeyConfig {
 		Ok(Self { keys, symbols })
 	}
 
+	pub fn is_nav_up(&self, key: &KeyEvent) -> bool {
+		key_match(key, self.keys.move_up)
+			|| (key.code == KeyCode::Char('k')
+				&& key.modifiers == KeyModifiers::NONE)
+	}
+
+	pub fn is_nav_down(&self, key: &KeyEvent) -> bool {
+		key_match(key, self.keys.move_down)
+			|| (key.code == KeyCode::Char('j')
+				&& key.modifiers == KeyModifiers::NONE)
+	}
+
+	pub fn is_nav_right(&self, key: &KeyEvent) -> bool {
+		key_match(key, self.keys.move_right)
+			|| (key.code == KeyCode::Char('l')
+				&& key.modifiers == KeyModifiers::NONE)
+	}
+
 	fn get_key_symbol(&self, k: KeyCode) -> &str {
 		match k {
 			KeyCode::Enter => &self.symbols.enter,
@@ -135,6 +153,71 @@ mod tests {
 	use std::io::Write;
 	use tempfile::NamedTempFile;
 
+	fn key_event(code: KeyCode, modifiers: KeyModifiers) -> KeyEvent {
+		KeyEvent::new(code, modifiers)
+	}
+
+	#[test]
+	fn test_vim_nav_up() {
+		let config = KeyConfig::default();
+		assert!(config
+			.is_nav_up(&key_event(KeyCode::Up, KeyModifiers::NONE)));
+		assert!(config.is_nav_up(&key_event(
+			KeyCode::Char('k'),
+			KeyModifiers::NONE
+		)));
+		assert!(!config.is_nav_up(&key_event(
+			KeyCode::Char('k'),
+			KeyModifiers::CONTROL
+		)));
+		assert!(!config.is_nav_up(&key_event(
+			KeyCode::Char('j'),
+			KeyModifiers::NONE
+		)));
+	}
+
+	#[test]
+	fn test_vim_nav_down() {
+		let config = KeyConfig::default();
+		assert!(config.is_nav_down(&key_event(
+			KeyCode::Down,
+			KeyModifiers::NONE
+		)));
+		assert!(config.is_nav_down(&key_event(
+			KeyCode::Char('j'),
+			KeyModifiers::NONE
+		)));
+		assert!(!config.is_nav_down(&key_event(
+			KeyCode::Char('j'),
+			KeyModifiers::CONTROL
+		)));
+		assert!(!config.is_nav_down(&key_event(
+			KeyCode::Char('k'),
+			KeyModifiers::NONE
+		)));
+	}
+
+	#[test]
+	fn test_vim_nav_right() {
+		let config = KeyConfig::default();
+		assert!(config.is_nav_right(&key_event(
+			KeyCode::Right,
+			KeyModifiers::NONE
+		)));
+		assert!(config.is_nav_right(&key_event(
+			KeyCode::Char('l'),
+			KeyModifiers::NONE
+		)));
+		assert!(!config.is_nav_right(&key_event(
+			KeyCode::Char('l'),
+			KeyModifiers::CONTROL
+		)));
+		assert!(!config.is_nav_right(&key_event(
+			KeyCode::Char('j'),
+			KeyModifiers::NONE
+		)));
+	}
+
 	#[test]
 	fn test_get_hint() {
 		let config = KeyConfig::default();

src/popups/blame_file.rs

@@ -257,13 +257,9 @@ impl Component for BlameFilePopup {
 			if let Event::Key(key) = event {
 				if key_match(key, self.key_config.keys.exit_popup) {
 					self.hide_stacked(false);
-				} else if key_match(key, self.key_config.keys.move_up)
-				{
+				} else if self.key_config.is_nav_up(key) {
 					self.move_selection(ScrollType::Up);
-				} else if key_match(
-					key,
-					self.key_config.keys.move_down,
-				) {
+				} else if self.key_config.is_nav_down(key) {
 					self.move_selection(ScrollType::Down);
 				} else if key_match(
 					key,
@@ -289,10 +285,7 @@ impl Component for BlameFilePopup {
 				} else if key_match(key, self.key_config.keys.page_up)
 				{
 					self.move_selection(ScrollType::PageUp);
-				} else if key_match(
-					key,
-					self.key_config.keys.move_right,
-				) {
+				} else if self.key_config.is_nav_right(key) {
 					if let Some(commit_id) = self.selected_commit() {
 						self.hide_stacked(true);
 						self.queue.push(InternalEvent::OpenPopup(

src/popups/branchlist.rs

@@ -170,7 +170,7 @@ impl Component for BranchListPopup {
 					"rebase error:",
 					self.rebase_branch()
 				);
-			} else if key_match(e, self.key_config.keys.move_right)
+			} else if self.key_config.is_nav_right(e)
 				&& self.valid_selection()
 			{
 				self.inspect_head_of_branch();
@@ -258,11 +258,11 @@ impl BranchListPopup {
 	fn move_event(&mut self, e: &KeyEvent) -> Result<EventState> {
 		if key_match(e, self.key_config.keys.exit_popup) {
 			self.hide();
-		} else if key_match(e, self.key_config.keys.move_down) {
+		} else if self.key_config.is_nav_down(e) {
 			return self
 				.move_selection(ScrollType::Up)
 				.map(Into::into);
-		} else if key_match(e, self.key_config.keys.move_up) {
+		} else if self.key_config.is_nav_up(e) {
 			return self
 				.move_selection(ScrollType::Down)
 				.map(Into::into);

src/popups/checkout_option.rs

@@ -200,13 +200,9 @@ impl Component for CheckoutOptionPopup {
 			if let Event::Key(key) = &event {
 				if key_match(key, self.key_config.keys.exit_popup) {
 					self.hide();
-				} else if key_match(
-					key,
-					self.key_config.keys.move_down,
-				) {
+				} else if self.key_config.is_nav_down(key) {
 					self.change_kind(true);
-				} else if key_match(key, self.key_config.keys.move_up)
-				{
+				} else if self.key_config.is_nav_up(key) {
 					self.change_kind(false);
 				} else if key_match(key, self.key_config.keys.enter) {
 					try_or_popup!(

src/popups/compare_commits.rs

@@ -124,10 +124,8 @@ impl Component for CompareCommitsPopup {
 					} else {
 						self.hide_stacked(false);
 					}
-				} else if key_match(
-					e,
-					self.key_config.keys.move_right,
-				) && self.can_focus_diff()
+				} else if self.key_config.is_nav_right(e)
+					&& self.can_focus_diff()
 				{
 					self.details.focus(false);
 					self.diff.focus(true);

src/popups/file_revlog.rs

@@ -507,10 +507,8 @@ impl Component for FileRevlogPopup {
 					} else {
 						self.hide_stacked(false);
 					}
-				} else if key_match(
-					key,
-					self.key_config.keys.move_right,
-				) && self.can_focus_diff()
+				} else if self.key_config.is_nav_right(key)
+					&& self.can_focus_diff()
 				{
 					self.diff.focus(true);
 				} else if key_match(key, self.key_config.keys.enter) {
@@ -537,13 +535,9 @@ impl Component for FileRevlogPopup {
 							),
 						));
 					}
-				} else if key_match(key, self.key_config.keys.move_up)
-				{
+				} else if self.key_config.is_nav_up(key) {
 					self.move_selection(ScrollType::Up)?;
-				} else if key_match(
-					key,
-					self.key_config.keys.move_down,
-				) {
+				} else if self.key_config.is_nav_down(key) {
 					self.move_selection(ScrollType::Down)?;
 				} else if key_match(
 					key,

src/popups/help.rs

@@ -137,10 +137,9 @@ impl Component for HelpPopup {
 			if let Event::Key(e) = ev {
 				if key_match(e, self.key_config.keys.exit_popup) {
 					self.hide();
-				} else if key_match(e, self.key_config.keys.move_down)
-				{
+				} else if self.key_config.is_nav_down(e) {
 					self.move_selection(true);
-				} else if key_match(e, self.key_config.keys.move_up) {
+				} else if self.key_config.is_nav_up(e) {
 					self.move_selection(false);
 				}
 			}

src/popups/inspect_commit.rs

@@ -161,10 +161,8 @@ impl Component for InspectCommitPopup {
 					} else {
 						self.hide_stacked(false);
 					}
-				} else if key_match(
-					e,
-					self.key_config.keys.move_right,
-				) && self.can_focus_diff()
+				} else if self.key_config.is_nav_right(e)
+					&& self.can_focus_diff()
 				{
 					self.details.focus(false);
 					self.diff.focus(true);

src/popups/options.rs

@@ -322,18 +322,11 @@ impl Component for OptionsPopup {
 			if let Event::Key(key) = &event {
 				if key_match(key, self.key_config.keys.exit_popup) {
 					self.hide();
-				} else if key_match(key, self.key_config.keys.move_up)
-				{
+				} else if self.key_config.is_nav_up(key) {
 					self.move_selection(true);
-				} else if key_match(
-					key,
-					self.key_config.keys.move_down,
-				) {
+				} else if self.key_config.is_nav_down(key) {
 					self.move_selection(false);
-				} else if key_match(
-					key,
-					self.key_config.keys.move_right,
-				) {
+				} else if self.key_config.is_nav_right(key) {
 					self.switch_option(true);
 				} else if key_match(
 					key,

src/popups/remotelist.rs

@@ -192,11 +192,11 @@ impl RemoteListPopup {
 	fn move_event(&mut self, e: &KeyEvent) -> Result<EventState> {
 		if key_match(e, self.key_config.keys.exit_popup) {
 			self.hide();
-		} else if key_match(e, self.key_config.keys.move_down) {
+		} else if self.key_config.is_nav_down(e) {
 			return self
 				.move_selection(ScrollType::Up)
 				.map(Into::into);
-		} else if key_match(e, self.key_config.keys.move_up) {
+		} else if self.key_config.is_nav_up(e) {
 			return self
 				.move_selection(ScrollType::Down)
 				.map(Into::into);

src/popups/reset.rs

@@ -230,13 +230,9 @@ impl Component for ResetPopup {
 			if let Event::Key(key) = &event {
 				if key_match(key, self.key_config.keys.exit_popup) {
 					self.hide();
-				} else if key_match(
-					key,
-					self.key_config.keys.move_down,
-				) {
+				} else if self.key_config.is_nav_down(key) {
 					self.change_kind(true);
-				} else if key_match(key, self.key_config.keys.move_up)
-				{
+				} else if self.key_config.is_nav_up(key) {
 					self.change_kind(false);
 				} else if key_match(key, self.key_config.keys.enter) {
 					self.reset();

src/popups/submodules.rs

@@ -151,11 +151,11 @@ impl Component for SubmodulesListPopup {
 		if let Event::Key(e) = ev {
 			if key_match(e, self.key_config.keys.exit_popup) {
 				self.hide();
-			} else if key_match(e, self.key_config.keys.move_down) {
+			} else if self.key_config.is_nav_down(e) {
 				return self
 					.move_selection(ScrollType::Up)
 					.map(Into::into);
-			} else if key_match(e, self.key_config.keys.move_up) {
+			} else if self.key_config.is_nav_up(e) {
 				return self
 					.move_selection(ScrollType::Down)
 					.map(Into::into);

src/popups/taglist.rs

@@ -187,13 +187,9 @@ impl Component for TagListPopup {
 			if let Event::Key(key) = event {
 				if key_match(key, self.key_config.keys.exit_popup) {
 					self.hide();
-				} else if key_match(key, self.key_config.keys.move_up)
-				{
+				} else if self.key_config.is_nav_up(key) {
 					self.move_selection(ScrollType::Up);
-				} else if key_match(
-					key,
-					self.key_config.keys.move_down,
-				) {
+				} else if self.key_config.is_nav_down(key) {
 					self.move_selection(ScrollType::Down);
 				} else if key_match(
 					key,
@@ -219,10 +215,8 @@ impl Component for TagListPopup {
 				} else if key_match(key, self.key_config.keys.page_up)
 				{
 					self.move_selection(ScrollType::PageUp);
-				} else if key_match(
-					key,
-					self.key_config.keys.move_right,
-				) && self.can_show_annotation()
+				} else if self.key_config.is_nav_right(key)
+					&& self.can_show_annotation()
 				{
 					self.show_annotation();
 				} else if key_match(

src/tabs/revlog.rs

@@ -494,10 +494,8 @@ impl Component for Revlog {
 							Ok(EventState::Consumed)
 						},
 					);
-				} else if key_match(
-					k,
-					self.key_config.keys.move_right,
-				) && self.commit_details.is_visible()
+				} else if self.key_config.is_nav_right(k)
+					&& self.commit_details.is_visible()
 				{
 					self.inspect_commit();
 					return Ok(EventState::Consumed);

src/tabs/status.rs

@@ -839,10 +839,8 @@ impl Component for Status {
 				{
 					self.switch_focus(self.focus.toggled_focus())
 						.map(Into::into)
-				} else if key_match(
-					k,
-					self.key_config.keys.move_right,
-				) && self.can_focus_diff()
+				} else if self.key_config.is_nav_right(k)
+					&& self.can_focus_diff()
 				{
 					self.switch_focus(Focus::Diff).map(Into::into)
 				} else if key_match(
@@ -854,12 +852,12 @@ impl Component for Status {
 						DiffTarget::WorkingDir => Focus::WorkDir,
 					})
 					.map(Into::into)
-				} else if key_match(k, self.key_config.keys.move_down)
+				} else if self.key_config.is_nav_down(k)
 					&& self.focus == Focus::WorkDir
 					&& !self.index.is_empty()
 				{
 					self.switch_focus(Focus::Stage).map(Into::into)
-				} else if key_match(k, self.key_config.keys.move_up)
+				} else if self.key_config.is_nav_up(k)
 					&& self.focus == Focus::Stage
 					&& !self.index_wd.is_empty()
 				{

src/ui/mod.rs

@@ -128,15 +128,15 @@ pub fn common_nav(
 	key: &crossterm::event::KeyEvent,
 	key_config: &SharedKeyConfig,
 ) -> Option<MoveSelection> {
-	if key_match(key, key_config.keys.move_down) {
+	if key_config.is_nav_down(key) {
 		Some(MoveSelection::Down)
-	} else if key_match(key, key_config.keys.move_up) {
+	} else if key_config.is_nav_up(key) {
 		Some(MoveSelection::Up)
 	} else if key_match(key, key_config.keys.page_up) {
 		Some(MoveSelection::PageUp)
 	} else if key_match(key, key_config.keys.page_down) {
 		Some(MoveSelection::PageDown)
-	} else if key_match(key, key_config.keys.move_right) {
+	} else if key_config.is_nav_right(key) {
 		Some(MoveSelection::Right)
 	} else if key_match(key, key_config.keys.move_left) {
 		Some(MoveSelection::Left)