Show progress indicator for /diff command (#2245)

## Summary
- Show a temporary Working on diff state in the bottom pan 
- Add `DiffResult` app event and dispatch git diff asynchronously

## Testing
- `just fmt`
- `just fix` *(fails: `let` expressions in this position are unstable)*
- `cargo test --all-features` *(fails: `let` expressions in this
position are unstable)*

------
https://chatgpt.com/codex/tasks/task_i_689a839f32b88321840a893551d5fbef
This commit is contained in:
aibrahim-oai
2025-08-15 15:32:41 -07:00
committed by GitHub
parent 379106d3eb
commit d3078b9adc
7 changed files with 89 additions and 47 deletions

View File

@@ -345,6 +345,11 @@ impl App<'_> {
AppState::Chat { widget } => widget.submit_op(op), AppState::Chat { widget } => widget.submit_op(op),
AppState::Onboarding { .. } => {} AppState::Onboarding { .. } => {}
}, },
AppEvent::DiffResult(text) => {
if let AppState::Chat { widget } = &mut self.app_state {
widget.add_diff_output(text);
}
}
AppEvent::DispatchCommand(command) => match command { AppEvent::DispatchCommand(command) => match command {
SlashCommand::New => { SlashCommand::New => {
// User accepted switch to chat view. // User accepted switch to chat view.
@@ -382,25 +387,24 @@ impl App<'_> {
break; break;
} }
SlashCommand::Diff => { SlashCommand::Diff => {
let (is_git_repo, diff_text) = match get_git_diff() {
Ok(v) => v,
Err(e) => {
let msg = format!("Failed to compute diff: {e}");
if let AppState::Chat { widget } = &mut self.app_state {
widget.add_diff_output(msg);
}
continue;
}
};
if let AppState::Chat { widget } = &mut self.app_state { if let AppState::Chat { widget } = &mut self.app_state {
let text = if is_git_repo { widget.add_diff_in_progress();
diff_text
} else {
"`/diff` — _not inside a git repository_".to_string()
};
widget.add_diff_output(text);
} }
let tx = self.app_event_tx.clone();
tokio::spawn(async move {
let text = match get_git_diff().await {
Ok((is_git_repo, diff_text)) => {
if is_git_repo {
diff_text
} else {
"`/diff` — _not inside a git repository_".to_string()
}
}
Err(e) => format!("Failed to compute diff: {e}"),
};
tx.send(AppEvent::DiffResult(text));
});
} }
SlashCommand::Mention => { SlashCommand::Mention => {
if let AppState::Chat { widget } = &mut self.app_state { if let AppState::Chat { widget } = &mut self.app_state {

View File

@@ -51,6 +51,9 @@ pub(crate) enum AppEvent {
matches: Vec<FileMatch>, matches: Vec<FileMatch>,
}, },
/// Result of computing a `/diff` command.
DiffResult(String),
InsertHistory(Vec<Line<'static>>), InsertHistory(Vec<Line<'static>>),
StartCommitAnimation, StartCommitAnimation,

View File

@@ -41,4 +41,8 @@ pub(crate) trait BottomPaneView<'a> {
) -> Option<ApprovalRequest> { ) -> Option<ApprovalRequest> {
Some(request) Some(request)
} }
/// Optional hook for views that expose a live status line. Views that do not
/// support this can ignore the call.
fn update_status_text(&mut self, _text: String) {}
} }

View File

@@ -210,6 +210,19 @@ impl BottomPane<'_> {
} }
} }
/// Update the live status text shown while a task is running.
/// If a modal view is active (i.e., not the status indicator), this is a noop.
pub(crate) fn update_status_text(&mut self, text: String) {
if !self.is_task_running || !self.status_view_active {
return;
}
if let Some(mut view) = self.active_view.take() {
view.update_status_text(text);
self.active_view = Some(view);
self.request_redraw();
}
}
pub(crate) fn composer_is_empty(&self) -> bool { pub(crate) fn composer_is_empty(&self) -> bool {
self.composer.is_empty() self.composer.is_empty()
} }

View File

@@ -43,4 +43,8 @@ impl BottomPaneView<'_> for StatusIndicatorView {
self.view.interrupt(); self.view.interrupt();
} }
} }
fn update_status_text(&mut self, text: String) {
self.update_text(text);
}
} }

View File

@@ -659,8 +659,17 @@ impl ChatWidget<'_> {
self.app_event_tx.send(AppEvent::RequestRedraw); self.app_event_tx.send(AppEvent::RequestRedraw);
} }
pub(crate) fn add_diff_in_progress(&mut self) {
self.bottom_pane.set_task_running(true);
self.bottom_pane
.update_status_text("computing diff".to_string());
self.request_redraw();
}
pub(crate) fn add_diff_output(&mut self, diff_output: String) { pub(crate) fn add_diff_output(&mut self, diff_output: String) {
self.add_to_history(&history_cell::new_diff_output(diff_output.clone())); self.bottom_pane.set_task_running(false);
self.add_to_history(&history_cell::new_diff_output(diff_output));
self.mark_needs_redraw();
} }
pub(crate) fn add_status_output(&mut self) { pub(crate) fn add_status_output(&mut self) {

View File

@@ -7,24 +7,26 @@
use std::io; use std::io;
use std::path::Path; use std::path::Path;
use std::process::Command;
use std::process::Stdio; use std::process::Stdio;
use tokio::process::Command;
/// Return value of [`get_git_diff`]. /// Return value of [`get_git_diff`].
/// ///
/// * `bool` Whether the current working directory is inside a Git repo. /// * `bool` Whether the current working directory is inside a Git repo.
/// * `String` The concatenated diff (may be empty). /// * `String` The concatenated diff (may be empty).
pub(crate) fn get_git_diff() -> io::Result<(bool, String)> { pub(crate) async fn get_git_diff() -> io::Result<(bool, String)> {
// First check if we are inside a Git repository. // First check if we are inside a Git repository.
if !inside_git_repo()? { if !inside_git_repo().await? {
return Ok((false, String::new())); return Ok((false, String::new()));
} }
// 1. Diff for tracked files. // Run tracked diff and untracked file listing in parallel.
let tracked_diff = run_git_capture_diff(&["diff", "--color"])?; let (tracked_diff_res, untracked_output_res) = tokio::join!(
run_git_capture_diff(&["diff", "--color"]),
// 2. Determine untracked files. run_git_capture_stdout(&["ls-files", "--others", "--exclude-standard"]),
let untracked_output = run_git_capture_stdout(&["ls-files", "--others", "--exclude-standard"])?; );
let tracked_diff = tracked_diff_res?;
let untracked_output = untracked_output_res?;
let mut untracked_diff = String::new(); let mut untracked_diff = String::new();
let null_device: &Path = if cfg!(windows) { let null_device: &Path = if cfg!(windows) {
@@ -33,26 +35,26 @@ pub(crate) fn get_git_diff() -> io::Result<(bool, String)> {
Path::new("/dev/null") Path::new("/dev/null")
}; };
let null_path = null_device.to_str().unwrap_or("/dev/null").to_string();
let mut join_set: tokio::task::JoinSet<io::Result<String>> = tokio::task::JoinSet::new();
for file in untracked_output for file in untracked_output
.split('\n') .split('\n')
.map(str::trim) .map(str::trim)
.filter(|s| !s.is_empty()) .filter(|s| !s.is_empty())
{ {
// Use `git diff --no-index` to generate a diff against the null device. let null_path = null_path.clone();
let args = [ let file = file.to_string();
"diff", join_set.spawn(async move {
"--color", let args = ["diff", "--color", "--no-index", "--", &null_path, &file];
"--no-index", run_git_capture_diff(&args).await
"--", });
null_device.to_str().unwrap_or("/dev/null"), }
file, while let Some(res) = join_set.join_next().await {
]; match res {
Ok(Ok(diff)) => untracked_diff.push_str(&diff),
match run_git_capture_diff(&args) { Ok(Err(err)) if err.kind() == io::ErrorKind::NotFound => {}
Ok(diff) => untracked_diff.push_str(&diff), Ok(Err(err)) => return Err(err),
// If the file disappeared between ls-files and diff we ignore the error. Err(_) => {}
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
Err(err) => return Err(err),
} }
} }
@@ -61,12 +63,13 @@ pub(crate) fn get_git_diff() -> io::Result<(bool, String)> {
/// Helper that executes `git` with the given `args` and returns `stdout` as a /// Helper that executes `git` with the given `args` and returns `stdout` as a
/// UTF-8 string. Any non-zero exit status is considered an *error*. /// UTF-8 string. Any non-zero exit status is considered an *error*.
fn run_git_capture_stdout(args: &[&str]) -> io::Result<String> { async fn run_git_capture_stdout(args: &[&str]) -> io::Result<String> {
let output = Command::new("git") let output = Command::new("git")
.args(args) .args(args)
.stdout(Stdio::piped()) .stdout(Stdio::piped())
.stderr(Stdio::null()) .stderr(Stdio::null())
.output()?; .output()
.await?;
if output.status.success() { if output.status.success() {
Ok(String::from_utf8_lossy(&output.stdout).into_owned()) Ok(String::from_utf8_lossy(&output.stdout).into_owned())
@@ -80,12 +83,13 @@ fn run_git_capture_stdout(args: &[&str]) -> io::Result<String> {
/// Like [`run_git_capture_stdout`] but treats exit status 1 as success and /// Like [`run_git_capture_stdout`] but treats exit status 1 as success and
/// returns stdout. Git returns 1 for diffs when differences are present. /// returns stdout. Git returns 1 for diffs when differences are present.
fn run_git_capture_diff(args: &[&str]) -> io::Result<String> { async fn run_git_capture_diff(args: &[&str]) -> io::Result<String> {
let output = Command::new("git") let output = Command::new("git")
.args(args) .args(args)
.stdout(Stdio::piped()) .stdout(Stdio::piped())
.stderr(Stdio::null()) .stderr(Stdio::null())
.output()?; .output()
.await?;
if output.status.success() || output.status.code() == Some(1) { if output.status.success() || output.status.code() == Some(1) {
Ok(String::from_utf8_lossy(&output.stdout).into_owned()) Ok(String::from_utf8_lossy(&output.stdout).into_owned())
@@ -98,12 +102,13 @@ fn run_git_capture_diff(args: &[&str]) -> io::Result<String> {
} }
/// Determine if the current directory is inside a Git repository. /// Determine if the current directory is inside a Git repository.
fn inside_git_repo() -> io::Result<bool> { async fn inside_git_repo() -> io::Result<bool> {
let status = Command::new("git") let status = Command::new("git")
.args(["rev-parse", "--is-inside-work-tree"]) .args(["rev-parse", "--is-inside-work-tree"])
.stdout(Stdio::null()) .stdout(Stdio::null())
.stderr(Stdio::null()) .stderr(Stdio::null())
.status(); .status()
.await;
match status { match status {
Ok(s) if s.success() => Ok(true), Ok(s) if s.success() => Ok(true),