Skip to content

Commit d3078b9

Browse files
authored
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
1 parent 379106d commit d3078b9

File tree

7 files changed

+89
-47
lines changed

7 files changed

+89
-47
lines changed

codex-rs/tui/src/app.rs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,11 @@ impl App<'_> {
345345
AppState::Chat { widget } => widget.submit_op(op),
346346
AppState::Onboarding { .. } => {}
347347
},
348+
AppEvent::DiffResult(text) => {
349+
if let AppState::Chat { widget } = &mut self.app_state {
350+
widget.add_diff_output(text);
351+
}
352+
}
348353
AppEvent::DispatchCommand(command) => match command {
349354
SlashCommand::New => {
350355
// User accepted – switch to chat view.
@@ -382,25 +387,24 @@ impl App<'_> {
382387
break;
383388
}
384389
SlashCommand::Diff => {
385-
let (is_git_repo, diff_text) = match get_git_diff() {
386-
Ok(v) => v,
387-
Err(e) => {
388-
let msg = format!("Failed to compute diff: {e}");
389-
if let AppState::Chat { widget } = &mut self.app_state {
390-
widget.add_diff_output(msg);
391-
}
392-
continue;
393-
}
394-
};
395-
396390
if let AppState::Chat { widget } = &mut self.app_state {
397-
let text = if is_git_repo {
398-
diff_text
399-
} else {
400-
"`/diff` — _not inside a git repository_".to_string()
401-
};
402-
widget.add_diff_output(text);
391+
widget.add_diff_in_progress();
403392
}
393+
394+
let tx = self.app_event_tx.clone();
395+
tokio::spawn(async move {
396+
let text = match get_git_diff().await {
397+
Ok((is_git_repo, diff_text)) => {
398+
if is_git_repo {
399+
diff_text
400+
} else {
401+
"`/diff` — _not inside a git repository_".to_string()
402+
}
403+
}
404+
Err(e) => format!("Failed to compute diff: {e}"),
405+
};
406+
tx.send(AppEvent::DiffResult(text));
407+
});
404408
}
405409
SlashCommand::Mention => {
406410
if let AppState::Chat { widget } = &mut self.app_state {

codex-rs/tui/src/app_event.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ pub(crate) enum AppEvent {
5151
matches: Vec<FileMatch>,
5252
},
5353

54+
/// Result of computing a `/diff` command.
55+
DiffResult(String),
56+
5457
InsertHistory(Vec<Line<'static>>),
5558

5659
StartCommitAnimation,

codex-rs/tui/src/bottom_pane/bottom_pane_view.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,8 @@ pub(crate) trait BottomPaneView<'a> {
4141
) -> Option<ApprovalRequest> {
4242
Some(request)
4343
}
44+
45+
/// Optional hook for views that expose a live status line. Views that do not
46+
/// support this can ignore the call.
47+
fn update_status_text(&mut self, _text: String) {}
4448
}

codex-rs/tui/src/bottom_pane/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,19 @@ impl BottomPane<'_> {
210210
}
211211
}
212212

213+
/// Update the live status text shown while a task is running.
214+
/// If a modal view is active (i.e., not the status indicator), this is a no‑op.
215+
pub(crate) fn update_status_text(&mut self, text: String) {
216+
if !self.is_task_running || !self.status_view_active {
217+
return;
218+
}
219+
if let Some(mut view) = self.active_view.take() {
220+
view.update_status_text(text);
221+
self.active_view = Some(view);
222+
self.request_redraw();
223+
}
224+
}
225+
213226
pub(crate) fn composer_is_empty(&self) -> bool {
214227
self.composer.is_empty()
215228
}

codex-rs/tui/src/bottom_pane/status_indicator_view.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,8 @@ impl BottomPaneView<'_> for StatusIndicatorView {
4343
self.view.interrupt();
4444
}
4545
}
46+
47+
fn update_status_text(&mut self, text: String) {
48+
self.update_text(text);
49+
}
4650
}

codex-rs/tui/src/chatwidget.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,8 +659,17 @@ impl ChatWidget<'_> {
659659
self.app_event_tx.send(AppEvent::RequestRedraw);
660660
}
661661

662+
pub(crate) fn add_diff_in_progress(&mut self) {
663+
self.bottom_pane.set_task_running(true);
664+
self.bottom_pane
665+
.update_status_text("computing diff".to_string());
666+
self.request_redraw();
667+
}
668+
662669
pub(crate) fn add_diff_output(&mut self, diff_output: String) {
663-
self.add_to_history(&history_cell::new_diff_output(diff_output.clone()));
670+
self.bottom_pane.set_task_running(false);
671+
self.add_to_history(&history_cell::new_diff_output(diff_output));
672+
self.mark_needs_redraw();
664673
}
665674

666675
pub(crate) fn add_status_output(&mut self) {

codex-rs/tui/src/get_git_diff.rs

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,26 @@
77
88
use std::io;
99
use std::path::Path;
10-
use std::process::Command;
1110
use std::process::Stdio;
11+
use tokio::process::Command;
1212

1313
/// Return value of [`get_git_diff`].
1414
///
1515
/// * `bool` – Whether the current working directory is inside a Git repo.
1616
/// * `String` – The concatenated diff (may be empty).
17-
pub(crate) fn get_git_diff() -> io::Result<(bool, String)> {
17+
pub(crate) async fn get_git_diff() -> io::Result<(bool, String)> {
1818
// First check if we are inside a Git repository.
19-
if !inside_git_repo()? {
19+
if !inside_git_repo().await? {
2020
return Ok((false, String::new()));
2121
}
2222

23-
// 1. Diff for tracked files.
24-
let tracked_diff = run_git_capture_diff(&["diff", "--color"])?;
25-
26-
// 2. Determine untracked files.
27-
let untracked_output = run_git_capture_stdout(&["ls-files", "--others", "--exclude-standard"])?;
23+
// Run tracked diff and untracked file listing in parallel.
24+
let (tracked_diff_res, untracked_output_res) = tokio::join!(
25+
run_git_capture_diff(&["diff", "--color"]),
26+
run_git_capture_stdout(&["ls-files", "--others", "--exclude-standard"]),
27+
);
28+
let tracked_diff = tracked_diff_res?;
29+
let untracked_output = untracked_output_res?;
2830

2931
let mut untracked_diff = String::new();
3032
let null_device: &Path = if cfg!(windows) {
@@ -33,26 +35,26 @@ pub(crate) fn get_git_diff() -> io::Result<(bool, String)> {
3335
Path::new("/dev/null")
3436
};
3537

38+
let null_path = null_device.to_str().unwrap_or("/dev/null").to_string();
39+
let mut join_set: tokio::task::JoinSet<io::Result<String>> = tokio::task::JoinSet::new();
3640
for file in untracked_output
3741
.split('\n')
3842
.map(str::trim)
3943
.filter(|s| !s.is_empty())
4044
{
41-
// Use `git diff --no-index` to generate a diff against the null device.
42-
let args = [
43-
"diff",
44-
"--color",
45-
"--no-index",
46-
"--",
47-
null_device.to_str().unwrap_or("/dev/null"),
48-
file,
49-
];
50-
51-
match run_git_capture_diff(&args) {
52-
Ok(diff) => untracked_diff.push_str(&diff),
53-
// If the file disappeared between ls-files and diff we ignore the error.
54-
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
55-
Err(err) => return Err(err),
45+
let null_path = null_path.clone();
46+
let file = file.to_string();
47+
join_set.spawn(async move {
48+
let args = ["diff", "--color", "--no-index", "--", &null_path, &file];
49+
run_git_capture_diff(&args).await
50+
});
51+
}
52+
while let Some(res) = join_set.join_next().await {
53+
match res {
54+
Ok(Ok(diff)) => untracked_diff.push_str(&diff),
55+
Ok(Err(err)) if err.kind() == io::ErrorKind::NotFound => {}
56+
Ok(Err(err)) => return Err(err),
57+
Err(_) => {}
5658
}
5759
}
5860

@@ -61,12 +63,13 @@ pub(crate) fn get_git_diff() -> io::Result<(bool, String)> {
6163

6264
/// Helper that executes `git` with the given `args` and returns `stdout` as a
6365
/// UTF-8 string. Any non-zero exit status is considered an *error*.
64-
fn run_git_capture_stdout(args: &[&str]) -> io::Result<String> {
66+
async fn run_git_capture_stdout(args: &[&str]) -> io::Result<String> {
6567
let output = Command::new("git")
6668
.args(args)
6769
.stdout(Stdio::piped())
6870
.stderr(Stdio::null())
69-
.output()?;
71+
.output()
72+
.await?;
7073

7174
if output.status.success() {
7275
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
@@ -80,12 +83,13 @@ fn run_git_capture_stdout(args: &[&str]) -> io::Result<String> {
8083

8184
/// Like [`run_git_capture_stdout`] but treats exit status 1 as success and
8285
/// returns stdout. Git returns 1 for diffs when differences are present.
83-
fn run_git_capture_diff(args: &[&str]) -> io::Result<String> {
86+
async fn run_git_capture_diff(args: &[&str]) -> io::Result<String> {
8487
let output = Command::new("git")
8588
.args(args)
8689
.stdout(Stdio::piped())
8790
.stderr(Stdio::null())
88-
.output()?;
91+
.output()
92+
.await?;
8993

9094
if output.status.success() || output.status.code() == Some(1) {
9195
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
@@ -98,12 +102,13 @@ fn run_git_capture_diff(args: &[&str]) -> io::Result<String> {
98102
}
99103

100104
/// Determine if the current directory is inside a Git repository.
101-
fn inside_git_repo() -> io::Result<bool> {
105+
async fn inside_git_repo() -> io::Result<bool> {
102106
let status = Command::new("git")
103107
.args(["rev-parse", "--is-inside-work-tree"])
104108
.stdout(Stdio::null())
105109
.stderr(Stdio::null())
106-
.status();
110+
.status()
111+
.await;
107112

108113
match status {
109114
Ok(s) if s.success() => Ok(true),

0 commit comments

Comments
 (0)