From 8998aec8389eba3ee214a3beca9f434961b01fda Mon Sep 17 00:00:00 2001 From: Kaniel Kirby Date: Mon, 1 Jul 2024 13:21:30 -0500 Subject: [PATCH 1/7] Support recursive blame (fixes #2194) - Replaces `BlameFilePopup.blame: Option` with `BlameFilePopup.blame_stack: Vec`. - Adds keybinding for going back from a blame (`b`). - Makes keybinding for `blame` (`Shift+B`) work in the `popups/blame_file.rs` view, where it will take the commit and open a blame from that. --- src/keys/key_list.rs | 2 + src/popups/blame_file.rs | 123 +++++++++++++++++++++++---------------- 2 files changed, 76 insertions(+), 49 deletions(-) diff --git a/src/keys/key_list.rs b/src/keys/key_list.rs index a542ef938a..d27242d8a8 100644 --- a/src/keys/key_list.rs +++ b/src/keys/key_list.rs @@ -68,6 +68,7 @@ pub struct KeysList { pub shift_down: GituiKeyEvent, pub enter: GituiKeyEvent, pub blame: GituiKeyEvent, + pub blame_back: GituiKeyEvent, pub file_history: GituiKeyEvent, pub edit_file: GituiKeyEvent, pub status_stage_all: GituiKeyEvent, @@ -160,6 +161,7 @@ impl Default for KeysList { shift_down: GituiKeyEvent::new(KeyCode::Down, KeyModifiers::SHIFT), enter: GituiKeyEvent::new(KeyCode::Enter, KeyModifiers::empty()), blame: GituiKeyEvent::new(KeyCode::Char('B'), KeyModifiers::SHIFT), + blame_back: GituiKeyEvent::new(KeyCode::Char('b'), KeyModifiers::empty()), file_history: GituiKeyEvent::new(KeyCode::Char('H'), KeyModifiers::SHIFT), edit_file: GituiKeyEvent::new(KeyCode::Char('e'), KeyModifiers::empty()), status_stage_all: GituiKeyEvent::new(KeyCode::Char('a'), KeyModifiers::empty()), diff --git a/src/popups/blame_file.rs b/src/popups/blame_file.rs index 9413b7db78..8f1e3f7a95 100644 --- a/src/popups/blame_file.rs +++ b/src/popups/blame_file.rs @@ -93,7 +93,7 @@ pub struct BlameFilePopup { table_state: std::cell::Cell, key_config: SharedKeyConfig, current_height: std::cell::Cell, - blame: Option, + blame_stack: Vec, app_sender: Sender, git_sender: Sender, repo: RepoPathRef, @@ -123,7 +123,7 @@ impl DrawableComponent for BlameFilePopup { ]; let number_of_rows: usize = rows.len(); - let syntax_highlight_progress = match self.blame { + let syntax_highlight_progress = match self.blame_stack.last() { Some(BlameProcess::SyntaxHighlighting { ref job, .. @@ -193,10 +193,7 @@ impl Component for BlameFilePopup { out: &mut Vec, force_all: bool, ) -> CommandBlocking { - let has_result = self - .blame - .as_ref() - .is_some_and(|blame| blame.result().is_some()); + let has_result = self.blame_stack.last().is_some_and(|last| last.result().is_some()); if self.is_visible() || force_all { out.push( CommandInfo::new( @@ -307,8 +304,40 @@ impl Component for BlameFilePopup { ), )); } + } else if key_match( + key, + self.key_config.keys.blame, + ) { + if let Some(file_path) = self + .params + .as_ref() + .map(|p| p.file_path.clone()) + { + let _ = self.open(BlameFileOpen { + file_path, + commit_id: self.selected_commit(), + selection: self.get_selection(), + }); + } + } else if key_match( + key, + self.key_config.keys.blame_back, + ) { + match self.blame_stack.len() { + 0 => { + self.hide_stacked(false) + }, + 1 => { + self.blame_stack.pop(); + self.hide_stacked(false) + }, + _ => { + self.blame_stack.pop(); + }, + } } + return Ok(EventState::Consumed); } } @@ -342,7 +371,7 @@ impl BlameFilePopup { current_height: std::cell::Cell::new(0), app_sender: env.sender_app.clone(), git_sender: env.sender_git.clone(), - blame: None, + blame_stack: vec![], repo: env.repo.clone(), } } @@ -371,11 +400,12 @@ impl BlameFilePopup { file_path: open.file_path, commit_id: open.commit_id, }); - self.blame = - Some(BlameProcess::GettingBlame(AsyncBlame::new( + self.blame_stack.push( + BlameProcess::GettingBlame(AsyncBlame::new( self.repo.borrow().clone(), &self.git_sender, - ))); + )) + ); self.table_state.get_mut().select(Some(0)); self.visible = true; self.update()?; @@ -384,9 +414,8 @@ impl BlameFilePopup { } /// - pub const fn any_work_pending(&self) -> bool { - self.blame.is_some() - && !matches!(self.blame, Some(BlameProcess::Result(_))) + pub fn any_work_pending(&self) -> bool { + self.blame_stack.last().is_some_and(|last| last.result().is_some()) } pub fn update_async( @@ -416,7 +445,7 @@ impl BlameFilePopup { if self.is_visible() { if let Some(BlameProcess::GettingBlame( ref mut async_blame, - )) = self.blame + )) = self.blame_stack.last_mut() { if let Some(params) = &self.params { if let Some(( @@ -425,19 +454,17 @@ impl BlameFilePopup { )) = async_blame.last()? { if previous_blame_params == *params { - self.blame = Some( - BlameProcess::SyntaxHighlighting { - unstyled_file_blame: - SyntaxFileBlame { - file_blame: - last_file_blame, - styled_text: None, - }, - job: AsyncSingleJob::new( - self.app_sender.clone(), - ), - }, - ); + *self.blame_stack.last_mut().unwrap() = BlameProcess::SyntaxHighlighting { + unstyled_file_blame: + SyntaxFileBlame { + file_blame: + last_file_blame, + styled_text: None, + }, + job: AsyncSingleJob::new( + self.app_sender.clone(), + ), + }; self.set_open_selection(); self.highlight_blame_lines(); @@ -457,7 +484,7 @@ impl BlameFilePopup { let Some(BlameProcess::SyntaxHighlighting { ref unstyled_file_blame, ref job, - }) = self.blame + }) = self.blame_stack.last() else { return; }; @@ -474,16 +501,15 @@ impl BlameFilePopup { == Path::new( unstyled_file_blame.path(), ) { - self.blame = - Some(BlameProcess::Result( - SyntaxFileBlame { - file_blame: - unstyled_file_blame - .file_blame - .clone(), - styled_text: Some(syntax), - }, - )); + *self.blame_stack.last_mut().unwrap() = BlameProcess::Result( + SyntaxFileBlame { + file_blame: + unstyled_file_blame + .file_blame + .clone(), + styled_text: Some(syntax), + }, + ); } } } @@ -498,7 +524,7 @@ impl BlameFilePopup { match ( self.any_work_pending(), self.params.as_ref(), - self.blame.as_ref().and_then(|blame| blame.result()), + self.blame_stack.last().and_then(|blame| blame.result()), ) { (true, Some(params), _) => { format!( @@ -526,8 +552,7 @@ impl BlameFilePopup { /// fn get_rows(&self, width: usize) -> Vec { - self.blame - .as_ref() + self.blame_stack.last() .and_then(|blame| blame.result()) .map(|file_blame| { let styled_text: Option> = file_blame @@ -556,7 +581,7 @@ impl BlameFilePopup { let Some(BlameProcess::SyntaxHighlighting { ref unstyled_file_blame, ref mut job, - }) = self.blame + }) = self.blame_stack.last_mut() else { return; }; @@ -654,7 +679,7 @@ impl BlameFilePopup { }); let file_blame = - self.blame.as_ref().and_then(|blame| blame.result()); + self.blame_stack.last().and_then(|blame| blame.result()); let is_blamed_commit = file_blame .and_then(|file_blame| { blame_hunk.map(|hunk| { @@ -673,8 +698,8 @@ impl BlameFilePopup { } fn get_max_line_number(&self) -> usize { - self.blame - .as_ref() + self.blame_stack + .last() .and_then(|blame| blame.result()) .map_or(0, |file_blame| file_blame.lines().len() - 1) } @@ -727,8 +752,8 @@ impl BlameFilePopup { } fn get_selection(&self) -> Option { - self.blame - .as_ref() + self.blame_stack + .last() .and_then(|blame| blame.result()) .and_then(|_| { let table_state = self.table_state.take(); @@ -742,8 +767,8 @@ impl BlameFilePopup { } fn selected_commit(&self) -> Option { - self.blame - .as_ref() + self.blame_stack + .last() .and_then(|blame| blame.result()) .and_then(|file_blame| { let table_state = self.table_state.take(); From 3337347ecf767c2bab59e83cb6a1c6c4676b669b Mon Sep 17 00:00:00 2001 From: Kaniel Kirby Date: Mon, 1 Jul 2024 13:59:22 -0500 Subject: [PATCH 2/7] Add blame hints, remove extra reload --- src/popups/blame_file.rs | 33 ++++++++++++++++++++++++++++----- src/strings.rs | 10 ++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/popups/blame_file.rs b/src/popups/blame_file.rs index 8f1e3f7a95..fd0c73d3d3 100644 --- a/src/popups/blame_file.rs +++ b/src/popups/blame_file.rs @@ -231,6 +231,26 @@ impl Component for BlameFilePopup { ) .order(1), ); + out.push( + CommandInfo::new( + strings::commands::blame_file( + &self.key_config, + ), + true, + has_result, + ) + .order(1), + ); + out.push( + CommandInfo::new( + strings::commands::blame_file_back( + &self.key_config, + ), + true, + has_result, + ) + .order(1), + ); } visibility_blocking(self) @@ -313,11 +333,14 @@ impl Component for BlameFilePopup { .as_ref() .map(|p| p.file_path.clone()) { - let _ = self.open(BlameFileOpen { - file_path, - commit_id: self.selected_commit(), - selection: self.get_selection(), - }); + // Avoid loading the same view. + if self.selected_commit().is_some_and(|c| { self.blame_stack.last().unwrap().result().is_some_and(|r| { r.file_blame.commit_id != c }) }) { + let _ = self.open(BlameFileOpen { + file_path, + commit_id: self.selected_commit(), + selection: self.get_selection(), + }); + } } } else if key_match( key, diff --git a/src/strings.rs b/src/strings.rs index 70ca9e3e8f..223f928d7a 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -1288,6 +1288,16 @@ pub mod commands { CMD_GROUP_GENERAL, ) } + pub fn blame_file_back(key_config: &SharedKeyConfig) -> CommandText { + CommandText::new( + format!( + "Back [{}]", + key_config.get_hint(key_config.keys.blame_back), + ), + "go to previous blame", + CMD_GROUP_GENERAL, + ) + } pub fn open_file_history( key_config: &SharedKeyConfig, ) -> CommandText { From c988fcea6239736fb2fd9edcf060b7a4f0dffed5 Mon Sep 17 00:00:00 2001 From: Kaniel Kirby Date: Mon, 1 Jul 2024 14:16:26 -0500 Subject: [PATCH 3/7] Fix infinite load issue from switch to blame_stack --- src/popups/blame_file.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/popups/blame_file.rs b/src/popups/blame_file.rs index fd0c73d3d3..6fba22f093 100644 --- a/src/popups/blame_file.rs +++ b/src/popups/blame_file.rs @@ -247,7 +247,7 @@ impl Component for BlameFilePopup { &self.key_config, ), true, - has_result, + true, ) .order(1), ); @@ -438,7 +438,7 @@ impl BlameFilePopup { /// pub fn any_work_pending(&self) -> bool { - self.blame_stack.last().is_some_and(|last| last.result().is_some()) + self.blame_stack.last().is_some_and(|last| !matches!(last, &BlameProcess::Result(_))) } pub fn update_async( From 96a3ca1229a52988bdbb03338969845341c311c0 Mon Sep 17 00:00:00 2001 From: Kaniel Kirby Date: Mon, 1 Jul 2024 14:59:07 -0500 Subject: [PATCH 4/7] Fix overscrolling when doing blame --- src/popups/blame_file.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/popups/blame_file.rs b/src/popups/blame_file.rs index 6fba22f093..fb9325af17 100644 --- a/src/popups/blame_file.rs +++ b/src/popups/blame_file.rs @@ -151,6 +151,11 @@ impl DrawableComponent for BlameFilePopup { let mut table_state = self.table_state.take(); + if table_state.selected().is_some_and(|s| s > number_of_rows) { + table_state.select(Some(number_of_rows - 1)); + table_state = table_state.with_offset(0); + } + f.render_widget(Clear, area); f.render_stateful_widget(table, area, &mut table_state); From c757f5653a5b887c3b31cce53aaed929ff463e78 Mon Sep 17 00:00:00 2001 From: Kaniel Kirby Date: Mon, 1 Jul 2024 15:29:59 -0500 Subject: [PATCH 5/7] Conform to `make check` --- src/popups/blame_file.rs | 106 ++++++++++++++++++++++----------------- src/strings.rs | 4 +- 2 files changed, 63 insertions(+), 47 deletions(-) diff --git a/src/popups/blame_file.rs b/src/popups/blame_file.rs index fb9325af17..ed96ea2add 100644 --- a/src/popups/blame_file.rs +++ b/src/popups/blame_file.rs @@ -123,16 +123,17 @@ impl DrawableComponent for BlameFilePopup { ]; let number_of_rows: usize = rows.len(); - let syntax_highlight_progress = match self.blame_stack.last() { - Some(BlameProcess::SyntaxHighlighting { - ref job, - .. - }) => job - .progress() - .map(|p| format!(" ({}%)", p.progress)) - .unwrap_or_default(), - _ => String::new(), - }; + let syntax_highlight_progress = + match self.blame_stack.last() { + Some(BlameProcess::SyntaxHighlighting { + ref job, + .. + }) => job + .progress() + .map(|p| format!(" ({}%)", p.progress)) + .unwrap_or_default(), + _ => String::new(), + }; let title_with_highlight_progress = format!("{title}{syntax_highlight_progress}"); @@ -151,7 +152,10 @@ impl DrawableComponent for BlameFilePopup { let mut table_state = self.table_state.take(); - if table_state.selected().is_some_and(|s| s > number_of_rows) { + if table_state + .selected() + .is_some_and(|s| s > number_of_rows) + { table_state.select(Some(number_of_rows - 1)); table_state = table_state.with_offset(0); } @@ -198,7 +202,10 @@ impl Component for BlameFilePopup { out: &mut Vec, force_all: bool, ) -> CommandBlocking { - let has_result = self.blame_stack.last().is_some_and(|last| last.result().is_some()); + let has_result = self + .blame_stack + .last() + .is_some_and(|last| last.result().is_some()); if self.is_visible() || force_all { out.push( CommandInfo::new( @@ -238,9 +245,7 @@ impl Component for BlameFilePopup { ); out.push( CommandInfo::new( - strings::commands::blame_file( - &self.key_config, - ), + strings::commands::blame_file(&self.key_config), true, has_result, ) @@ -261,6 +266,7 @@ impl Component for BlameFilePopup { visibility_blocking(self) } + #[allow(too_many_lines)] fn event( &mut self, event: &crossterm::event::Event, @@ -329,17 +335,21 @@ impl Component for BlameFilePopup { ), )); } - } else if key_match( - key, - self.key_config.keys.blame, - ) { + } else if key_match(key, self.key_config.keys.blame) { if let Some(file_path) = self .params .as_ref() .map(|p| p.file_path.clone()) { // Avoid loading the same view. - if self.selected_commit().is_some_and(|c| { self.blame_stack.last().unwrap().result().is_some_and(|r| { r.file_blame.commit_id != c }) }) { + if self.selected_commit().is_some_and(|c| { + self.blame_stack + .last() + .and_then(|last| last.result()) + .map_or(false, |r| { + r.file_blame.commit_id != c + }) + }) { let _ = self.open(BlameFileOpen { file_path, commit_id: self.selected_commit(), @@ -352,20 +362,17 @@ impl Component for BlameFilePopup { self.key_config.keys.blame_back, ) { match self.blame_stack.len() { - 0 => { - self.hide_stacked(false) - }, + 0 => self.hide_stacked(false), 1 => { self.blame_stack.pop(); - self.hide_stacked(false) - }, + self.hide_stacked(false); + } _ => { self.blame_stack.pop(); - }, + } } } - return Ok(EventState::Consumed); } } @@ -428,12 +435,12 @@ impl BlameFilePopup { file_path: open.file_path, commit_id: open.commit_id, }); - self.blame_stack.push( - BlameProcess::GettingBlame(AsyncBlame::new( + self.blame_stack.push(BlameProcess::GettingBlame( + AsyncBlame::new( self.repo.borrow().clone(), &self.git_sender, - )) - ); + ), + )); self.table_state.get_mut().select(Some(0)); self.visible = true; self.update()?; @@ -443,7 +450,9 @@ impl BlameFilePopup { /// pub fn any_work_pending(&self) -> bool { - self.blame_stack.last().is_some_and(|last| !matches!(last, &BlameProcess::Result(_))) + self.blame_stack.last().is_some_and(|last| { + !matches!(last, &BlameProcess::Result(_)) + }) } pub fn update_async( @@ -482,17 +491,18 @@ impl BlameFilePopup { )) = async_blame.last()? { if previous_blame_params == *params { - *self.blame_stack.last_mut().unwrap() = BlameProcess::SyntaxHighlighting { - unstyled_file_blame: - SyntaxFileBlame { - file_blame: - last_file_blame, - styled_text: None, - }, - job: AsyncSingleJob::new( - self.app_sender.clone(), - ), - }; + *self.blame_stack.last_mut().expect("Last will always exist here, as we just got it.") = + BlameProcess::SyntaxHighlighting { + unstyled_file_blame: + SyntaxFileBlame { + file_blame: + last_file_blame, + styled_text: None, + }, + job: AsyncSingleJob::new( + self.app_sender.clone(), + ), + }; self.set_open_selection(); self.highlight_blame_lines(); @@ -512,7 +522,7 @@ impl BlameFilePopup { let Some(BlameProcess::SyntaxHighlighting { ref unstyled_file_blame, ref job, - }) = self.blame_stack.last() + }) = self.blame_stack.last_mut() else { return; }; @@ -529,7 +539,10 @@ impl BlameFilePopup { == Path::new( unstyled_file_blame.path(), ) { - *self.blame_stack.last_mut().unwrap() = BlameProcess::Result( + *self + .blame_stack + .last_mut() + .expect("Last will always exist here, as we just got it.") = BlameProcess::Result( SyntaxFileBlame { file_blame: unstyled_file_blame @@ -580,7 +593,8 @@ impl BlameFilePopup { /// fn get_rows(&self, width: usize) -> Vec { - self.blame_stack.last() + self.blame_stack + .last() .and_then(|blame| blame.result()) .map(|file_blame| { let styled_text: Option> = file_blame diff --git a/src/strings.rs b/src/strings.rs index 223f928d7a..1a8525ef36 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -1288,7 +1288,9 @@ pub mod commands { CMD_GROUP_GENERAL, ) } - pub fn blame_file_back(key_config: &SharedKeyConfig) -> CommandText { + pub fn blame_file_back( + key_config: &SharedKeyConfig, + ) -> CommandText { CommandText::new( format!( "Back [{}]", From a76b3b83e0b855e6566b9f0b5ff8f94445946da0 Mon Sep 17 00:00:00 2001 From: Kaniel Kirby Date: Mon, 1 Jul 2024 15:34:06 -0500 Subject: [PATCH 6/7] Add changelog item --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 630cf520d6..729b81c48b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.26.3] - 2024-06-02 +### Added +* add recursive git blame (do `SHIFT+B` in the revlog to go forward, and `b` to go back). + +## [0.26.3] - 2024-06-02 + ### Breaking Changes #### Theme file format From 67204664d203cefc0be9c8963cdc2857469e465d Mon Sep 17 00:00:00 2001 From: Kaniel Kirby Date: Mon, 1 Jul 2024 16:33:00 -0500 Subject: [PATCH 7/7] Add recursive Git blame to file_revlog popup --- src/popups/file_revlog.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/popups/file_revlog.rs b/src/popups/file_revlog.rs index 1bfc0b266e..71dc13c782 100644 --- a/src/popups/file_revlog.rs +++ b/src/popups/file_revlog.rs @@ -613,6 +613,14 @@ impl Component for FileRevlogPopup { ) .order(1), ); + out.push( + CommandInfo::new( + strings::commands::blame_file_back(&self.key_config), + true, + self.selected_commit().is_some(), + ) + .order(1), + ); out.push(CommandInfo::new( strings::commands::diff_focus_right(&self.key_config),