-
-
Notifications
You must be signed in to change notification settings - Fork 605
Add support for recursive blame #2285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
8998aec
3337347
c988fce
96a3ca1
c757f56
a76b3b8
6720466
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,7 +93,7 @@ pub struct BlameFilePopup { | |
table_state: std::cell::Cell<TableState>, | ||
key_config: SharedKeyConfig, | ||
current_height: std::cell::Cell<usize>, | ||
blame: Option<BlameProcess>, | ||
blame_stack: Vec<BlameProcess>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like us to not keep this stack around. @cruessler and I agreed on working out an asynchronous blaming that will deliver results iteratively so the worst case scenarios are not as slow anymore and this stack will save a lot of data when going only in one direction through the blame history and only optimize for going back fast and not going back and forth. Furthermore I have a gut feeling that it might lead to weird stacks when jumping to a file history from a blame and then to another blame again which will not properly reset the stack There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that makes sense. So, to clarify, do you want to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a friendly ping :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My guess is I also would love to have this feature in gitui. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, also, |
||
app_sender: Sender<AsyncAppNotification>, | ||
git_sender: Sender<AsyncGitNotification>, | ||
repo: RepoPathRef, | ||
|
@@ -123,16 +123,17 @@ impl DrawableComponent for BlameFilePopup { | |
]; | ||
|
||
let number_of_rows: usize = rows.len(); | ||
let syntax_highlight_progress = match self.blame { | ||
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,6 +152,14 @@ 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); | ||
|
||
|
@@ -194,9 +203,9 @@ impl Component for BlameFilePopup { | |
force_all: bool, | ||
) -> CommandBlocking { | ||
let has_result = self | ||
.blame | ||
.as_ref() | ||
.is_some_and(|blame| blame.result().is_some()); | ||
.blame_stack | ||
.last() | ||
.is_some_and(|last| last.result().is_some()); | ||
if self.is_visible() || force_all { | ||
out.push( | ||
CommandInfo::new( | ||
|
@@ -234,11 +243,30 @@ 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, | ||
true, | ||
) | ||
.order(1), | ||
); | ||
} | ||
|
||
visibility_blocking(self) | ||
} | ||
|
||
#[allow(too_many_lines)] | ||
fn event( | ||
&mut self, | ||
event: &crossterm::event::Event, | ||
|
@@ -307,6 +335,42 @@ 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()) | ||
{ | ||
// Avoid loading the same view. | ||
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(), | ||
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 +406,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 +435,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 +449,10 @@ 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| { | ||
!matches!(last, &BlameProcess::Result(_)) | ||
}) | ||
} | ||
|
||
pub fn update_async( | ||
|
@@ -416,7 +482,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,7 +491,7 @@ impl BlameFilePopup { | |
)) = async_blame.last()? | ||
{ | ||
if previous_blame_params == *params { | ||
self.blame = Some( | ||
*self.blame_stack.last_mut().expect("Last will always exist here, as we just got it.") = | ||
BlameProcess::SyntaxHighlighting { | ||
unstyled_file_blame: | ||
SyntaxFileBlame { | ||
|
@@ -436,8 +502,7 @@ impl BlameFilePopup { | |
job: AsyncSingleJob::new( | ||
self.app_sender.clone(), | ||
), | ||
}, | ||
); | ||
}; | ||
self.set_open_selection(); | ||
self.highlight_blame_lines(); | ||
|
||
|
@@ -457,7 +522,7 @@ impl BlameFilePopup { | |
let Some(BlameProcess::SyntaxHighlighting { | ||
ref unstyled_file_blame, | ||
ref job, | ||
}) = self.blame | ||
}) = self.blame_stack.last_mut() | ||
else { | ||
return; | ||
}; | ||
|
@@ -474,16 +539,18 @@ 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() | ||
.expect("Last will always exist here, as we just got it.") = BlameProcess::Result( | ||
SyntaxFileBlame { | ||
file_blame: | ||
unstyled_file_blame | ||
.file_blame | ||
.clone(), | ||
styled_text: Some(syntax), | ||
}, | ||
); | ||
} | ||
} | ||
} | ||
|
@@ -498,7 +565,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 +593,8 @@ impl BlameFilePopup { | |
|
||
/// | ||
fn get_rows(&self, width: usize) -> Vec<Row> { | ||
self.blame | ||
.as_ref() | ||
self.blame_stack | ||
.last() | ||
.and_then(|blame| blame.result()) | ||
.map(|file_blame| { | ||
let styled_text: Option<Text<'_>> = file_blame | ||
|
@@ -556,7 +623,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 +721,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 +740,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 +794,8 @@ impl BlameFilePopup { | |
} | ||
|
||
fn get_selection(&self) -> Option<usize> { | ||
self.blame | ||
.as_ref() | ||
self.blame_stack | ||
.last() | ||
.and_then(|blame| blame.result()) | ||
.and_then(|_| { | ||
let table_state = self.table_state.take(); | ||
|
@@ -742,8 +809,8 @@ impl BlameFilePopup { | |
} | ||
|
||
fn selected_commit(&self) -> Option<CommitId> { | ||
self.blame | ||
.as_ref() | ||
self.blame_stack | ||
.last() | ||
.and_then(|blame| blame.result()) | ||
.and_then(|file_blame| { | ||
let table_state = self.table_state.take(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move this to the unreleased section? Thanks. :)