From 83b8530e1f883cb83a1aec48273feb50a7a70480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 29 Apr 2025 13:06:43 -0300 Subject: [PATCH] agent: Create `TerminalToolCard` and display shell output while it's running (#29546) Also, don't require a worktree to run the terminal tool. Release Notes: - N/A --- Cargo.lock | 3 + crates/assistant_tool/src/assistant_tool.rs | 7 + crates/assistant_tools/Cargo.toml | 4 + crates/assistant_tools/src/edit_file_tool.rs | 31 +- crates/assistant_tools/src/terminal_tool.rs | 693 +++++++++++------- .../src/terminal_tool/description.md | 4 +- crates/assistant_tools/src/web_search_tool.rs | 8 +- crates/image_viewer/src/image_info.rs | 27 +- crates/project/src/terminals.rs | 2 +- crates/terminal/src/terminal.rs | 29 +- crates/terminal_view/src/terminal_element.rs | 2 +- crates/terminal_view/src/terminal_panel.rs | 2 +- crates/util/src/size.rs | 48 ++ crates/util/src/time.rs | 41 ++ crates/util/src/util.rs | 2 + 15 files changed, 560 insertions(+), 343 deletions(-) create mode 100644 crates/util/src/size.rs create mode 100644 crates/util/src/time.rs diff --git a/Cargo.lock b/Cargo.lock index d63b55ecfe..4349156d61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -730,6 +730,9 @@ dependencies = [ "serde", "serde_json", "settings", + "task", + "terminal", + "terminal_view", "tree-sitter-rust", "ui", "unindent", diff --git a/crates/assistant_tool/src/assistant_tool.rs b/crates/assistant_tool/src/assistant_tool.rs index a706f48926..6cd62485e9 100644 --- a/crates/assistant_tool/src/assistant_tool.rs +++ b/crates/assistant_tool/src/assistant_tool.rs @@ -51,6 +51,13 @@ impl ToolUseStatus { ToolUseStatus::Error(out) => out.clone(), } } + + pub fn error(&self) -> Option { + match self { + ToolUseStatus::Error(out) => Some(out.clone()), + _ => None, + } + } } /// The result of running a tool, containing both the asynchronous output diff --git a/crates/assistant_tools/Cargo.toml b/crates/assistant_tools/Cargo.toml index df9edc4a4d..a394ed2c34 100644 --- a/crates/assistant_tools/Cargo.toml +++ b/crates/assistant_tools/Cargo.toml @@ -34,6 +34,9 @@ regex.workspace = true schemars.workspace = true serde.workspace = true serde_json.workspace = true +task.workspace = true +terminal.workspace = true +terminal_view.workspace = true ui.workspace = true util.workspace = true web_search.workspace = true @@ -51,6 +54,7 @@ project = { workspace = true, features = ["test-support"] } rand.workspace = true pretty_assertions.workspace = true settings = { workspace = true, features = ["test-support"] } +task = { workspace = true, features = ["test-support"]} tree-sitter-rust.workspace = true workspace = { workspace = true, features = ["test-support"] } unindent.workspace = true diff --git a/crates/assistant_tools/src/edit_file_tool.rs b/crates/assistant_tools/src/edit_file_tool.rs index a5120877c6..fc88db2351 100644 --- a/crates/assistant_tools/src/edit_file_tool.rs +++ b/crates/assistant_tools/src/edit_file_tool.rs @@ -24,6 +24,8 @@ use ui::{Disclosure, Tooltip, Window, prelude::*}; use util::ResultExt; use workspace::Workspace; +pub struct EditFileTool; + #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct EditFileToolInput { /// A user-friendly markdown description of what's being replaced. This will be shown in the UI. @@ -75,8 +77,6 @@ struct PartialInput { new_string: String, } -pub struct EditFileTool; - const DEFAULT_UI_TEXT: &str = "Editing file"; impl Tool for EditFileTool { @@ -627,7 +627,6 @@ mod tests { #[test] fn still_streaming_ui_text_with_path() { - let tool = EditFileTool; let input = json!({ "path": "src/main.rs", "display_description": "", @@ -635,12 +634,11 @@ mod tests { "new_string": "new code" }); - assert_eq!(tool.still_streaming_ui_text(&input), "src/main.rs"); + assert_eq!(EditFileTool.still_streaming_ui_text(&input), "src/main.rs"); } #[test] fn still_streaming_ui_text_with_description() { - let tool = EditFileTool; let input = json!({ "path": "", "display_description": "Fix error handling", @@ -648,12 +646,14 @@ mod tests { "new_string": "new code" }); - assert_eq!(tool.still_streaming_ui_text(&input), "Fix error handling"); + assert_eq!( + EditFileTool.still_streaming_ui_text(&input), + "Fix error handling", + ); } #[test] fn still_streaming_ui_text_with_path_and_description() { - let tool = EditFileTool; let input = json!({ "path": "src/main.rs", "display_description": "Fix error handling", @@ -661,12 +661,14 @@ mod tests { "new_string": "new code" }); - assert_eq!(tool.still_streaming_ui_text(&input), "Fix error handling"); + assert_eq!( + EditFileTool.still_streaming_ui_text(&input), + "Fix error handling", + ); } #[test] fn still_streaming_ui_text_no_path_or_description() { - let tool = EditFileTool; let input = json!({ "path": "", "display_description": "", @@ -674,14 +676,19 @@ mod tests { "new_string": "new code" }); - assert_eq!(tool.still_streaming_ui_text(&input), DEFAULT_UI_TEXT); + assert_eq!( + EditFileTool.still_streaming_ui_text(&input), + DEFAULT_UI_TEXT, + ); } #[test] fn still_streaming_ui_text_with_null() { - let tool = EditFileTool; let input = serde_json::Value::Null; - assert_eq!(tool.still_streaming_ui_text(&input), DEFAULT_UI_TEXT); + assert_eq!( + EditFileTool.still_streaming_ui_text(&input), + DEFAULT_UI_TEXT, + ); } } diff --git a/crates/assistant_tools/src/terminal_tool.rs b/crates/assistant_tools/src/terminal_tool.rs index bb9d1d93dc..7b08695dd1 100644 --- a/crates/assistant_tools/src/terminal_tool.rs +++ b/crates/assistant_tools/src/terminal_tool.rs @@ -1,23 +1,32 @@ use crate::schema::json_schema_for; use anyhow::{Context as _, Result, anyhow}; -use assistant_tool::{ActionLog, Tool, ToolResult}; -use futures::io::BufReader; -use futures::{AsyncBufReadExt, AsyncReadExt, FutureExt}; -use gpui::{AnyWindowHandle, App, AppContext, Entity, Task}; +use assistant_tool::{ActionLog, Tool, ToolCard, ToolResult, ToolUseStatus}; +use gpui::{ + Animation, AnimationExt, AnyWindowHandle, App, AppContext, Empty, Entity, EntityId, Task, + Transformation, WeakEntity, Window, percentage, +}; use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat}; -use project::Project; +use project::{Project, terminals::TerminalKind}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::future; -use util::get_system_shell; +use std::{ + env, + path::{Path, PathBuf}, + process::ExitStatus, + sync::Arc, + time::{Duration, Instant}, +}; +use terminal_view::TerminalView; +use ui::{Disclosure, IconName, Tooltip, prelude::*}; +use util::{ + get_system_shell, markdown::MarkdownInlineCode, size::format_file_size, + time::duration_alt_display, +}; +use workspace::Workspace; -use std::path::Path; -use std::sync::Arc; -use ui::IconName; -use util::command::new_smol_command; -use util::markdown::MarkdownInlineCode; +const COMMAND_OUTPUT_LIMIT: usize = 16 * 1024; -#[derive(Debug, Serialize, Deserialize, JsonSchema)] +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] pub struct TerminalToolInput { /// The one-liner command to execute. command: String, @@ -75,308 +84,426 @@ impl Tool for TerminalTool { _messages: &[LanguageModelRequestMessage], project: Entity, _action_log: Entity, - _window: Option, + window: Option, cx: &mut App, ) -> ToolResult { + let Some(window) = window else { + return Task::ready(Err(anyhow!("no window options"))).into(); + }; + let input: TerminalToolInput = match serde_json::from_value(input) { Ok(input) => input, Err(err) => return Task::ready(Err(anyhow!(err))).into(), }; - let project = project.read(cx); let input_path = Path::new(&input.cd); - let working_dir = if input.cd == "." { - // Accept "." as meaning "the one worktree" if we only have one worktree. - let mut worktrees = project.worktrees(cx); - - let only_worktree = match worktrees.next() { - Some(worktree) => worktree, - None => { - return Task::ready(Err(anyhow!("No worktrees found in the project"))).into(); - } - }; - - if worktrees.next().is_some() { - return Task::ready(Err(anyhow!( - "'.' is ambiguous in multi-root workspaces. Please specify a root directory explicitly." - ))).into(); - } - - only_worktree.read(cx).abs_path() - } else if input_path.is_absolute() { - // Absolute paths are allowed, but only if they're in one of the project's worktrees. - if !project - .worktrees(cx) - .any(|worktree| input_path.starts_with(&worktree.read(cx).abs_path())) - { - return Task::ready(Err(anyhow!( - "The absolute path must be within one of the project's worktrees" - ))) - .into(); - } - - input_path.into() - } else { - let Some(worktree) = project.worktree_for_root_name(&input.cd, cx) else { - return Task::ready(Err(anyhow!( - "`cd` directory {} not found in the project", - &input.cd - ))) - .into(); - }; - - worktree.read(cx).abs_path() + let working_dir = match working_dir(cx, &input, &project, input_path) { + Ok(dir) => dir, + Err(err) => return Task::ready(Err(anyhow!(err))).into(), }; + let terminal = project.update(cx, |project, cx| { + project.create_terminal( + TerminalKind::Task(task::SpawnInTerminal { + command: get_system_shell(), + args: vec!["-c".into(), input.command.clone()], + cwd: working_dir.clone(), + ..Default::default() + }), + window, + cx, + ) + }); - cx.background_spawn(run_command_limited(working_dir, input.command)) - .into() - } -} + let card = cx.new(|cx| { + TerminalToolCard::new(input.command.clone(), working_dir.clone(), cx.entity_id()) + }); -const LIMIT: usize = 16 * 1024; + let output = cx.spawn({ + let card = card.clone(); + async move |cx| { + let terminal = terminal.await?; + let workspace = window + .downcast::() + .and_then(|handle| handle.entity(cx).ok()) + .context("no workspace entity in root of window")?; -async fn run_command_limited(working_dir: Arc, command: String) -> Result { - let shell = get_system_shell(); + let terminal_view = window.update(cx, |_, window, cx| { + cx.new(|cx| { + TerminalView::new( + terminal.clone(), + workspace.downgrade(), + None, + project.downgrade(), + window, + cx, + ) + }) + })?; + let _ = card.update(cx, |card, _| { + card.terminal = Some(terminal_view.clone()); + card.start_instant = Instant::now(); + }); - let mut cmd = new_smol_command(&shell) - .arg("-c") - .arg(&command) - .current_dir(working_dir) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() - .context("Failed to execute terminal command")?; + let exit_status = terminal + .update(cx, |terminal, cx| terminal.wait_for_completed_task(cx))? + .await; + let (content, content_line_count) = terminal.update(cx, |terminal, _| { + (terminal.get_content(), terminal.total_lines()) + })?; - let mut combined_buffer = String::with_capacity(LIMIT + 1); + let previous_len = content.len(); + let (processed_content, finished_with_empty_output) = + process_content(content, &input.command, exit_status); - let mut out_reader = BufReader::new(cmd.stdout.take().context("Failed to get stdout")?); - let mut out_tmp_buffer = String::with_capacity(512); - let mut err_reader = BufReader::new(cmd.stderr.take().context("Failed to get stderr")?); - let mut err_tmp_buffer = String::with_capacity(512); + let _ = card.update(cx, |card, _| { + card.command_finished = true; + card.exit_status = exit_status; + card.was_content_truncated = processed_content.len() < previous_len; + card.original_content_len = previous_len; + card.content_line_count = content_line_count; + card.finished_with_empty_output = finished_with_empty_output; + card.elapsed_time = Some(card.start_instant.elapsed()); + }); - let mut out_line = Box::pin( - out_reader - .read_line(&mut out_tmp_buffer) - .left_future() - .fuse(), - ); - let mut err_line = Box::pin( - err_reader - .read_line(&mut err_tmp_buffer) - .left_future() - .fuse(), - ); - - let mut has_stdout = true; - let mut has_stderr = true; - while (has_stdout || has_stderr) && combined_buffer.len() < LIMIT + 1 { - futures::select_biased! { - read = out_line => { - drop(out_line); - combined_buffer.extend(out_tmp_buffer.drain(..)); - if read? == 0 { - out_line = Box::pin(future::pending().right_future().fuse()); - has_stdout = false; - } else { - out_line = Box::pin(out_reader.read_line(&mut out_tmp_buffer).left_future().fuse()); - } + Ok(processed_content) } - read = err_line => { - drop(err_line); - combined_buffer.extend(err_tmp_buffer.drain(..)); - if read? == 0 { - err_line = Box::pin(future::pending().right_future().fuse()); - has_stderr = false; - } else { - err_line = Box::pin(err_reader.read_line(&mut err_tmp_buffer).left_future().fuse()); - } - } - }; - } + }); - drop((out_line, err_line)); - - let truncated = combined_buffer.len() > LIMIT; - combined_buffer.truncate(LIMIT); - - consume_reader(out_reader, truncated).await?; - consume_reader(err_reader, truncated).await?; - - // Handle potential errors during status retrieval, including interruption. - match cmd.status().await { - Ok(status) => { - let output_string = if truncated { - // Valid to find `\n` in UTF-8 since 0-127 ASCII characters are not used in - // multi-byte characters. - let last_line_ix = combined_buffer.bytes().rposition(|b| b == b'\n'); - let buffer_content = - &combined_buffer[..last_line_ix.unwrap_or(combined_buffer.len())]; - - format!( - "Command output too long. The first {} bytes:\n\n{}", - buffer_content.len(), - output_block(buffer_content), - ) - } else { - output_block(&combined_buffer) - }; - - let output_with_status = if status.success() { - if output_string.is_empty() { - "Command executed successfully.".to_string() - } else { - output_string - } - } else { - format!( - "Command failed with exit code {} (shell: {}).\n\n{}", - status.code().unwrap_or(-1), - shell, - output_string, - ) - }; - - Ok(output_with_status) - } - Err(err) => { - // Error occurred getting status (potential interruption). Include partial output. - let partial_output = output_block(&combined_buffer); - let error_message = format!( - "Command failed or was interrupted.\nPartial output captured:\n\n{}", - partial_output - ); - Err(anyhow!(err).context(error_message)) + ToolResult { + output, + card: Some(card.into()), } } } -async fn consume_reader( - mut reader: BufReader, - truncated: bool, -) -> Result<(), std::io::Error> { - loop { - let skipped_bytes = reader.fill_buf().await?; - if skipped_bytes.is_empty() { - break; +fn process_content( + content: String, + command: &str, + exit_status: Option, +) -> (String, bool) { + let should_truncate = content.len() > COMMAND_OUTPUT_LIMIT; + + let content = if should_truncate { + let mut end_ix = COMMAND_OUTPUT_LIMIT.min(content.len()); + while !content.is_char_boundary(end_ix) { + end_ix -= 1; } - let skipped_bytes_len = skipped_bytes.len(); - reader.consume_unpin(skipped_bytes_len); + // Don't truncate mid-line, clear the remainder of the last line + end_ix = content[..end_ix].rfind('\n').unwrap_or(end_ix); + &content[..end_ix] + } else { + content.as_str() + }; + let is_empty = content.trim().is_empty(); - // Should only skip if we went over the limit - debug_assert!(truncated); - } - Ok(()) -} - -fn output_block(output: &str) -> String { - format!( + let content = format!( "```\n{}{}```", - output, - if output.ends_with('\n') { "" } else { "\n" } - ) -} + content, + if content.ends_with('\n') { "" } else { "\n" } + ); -#[cfg(test)] -#[cfg(not(windows))] -mod tests { - use gpui::TestAppContext; - - use super::*; - - #[gpui::test(iterations = 10)] - async fn test_run_command_simple(cx: &mut TestAppContext) { - cx.executor().allow_parking(); - - let result = - run_command_limited(Path::new(".").into(), "echo 'Hello, World!'".to_string()).await; - - assert!(result.is_ok()); - assert_eq!(result.unwrap(), "```\nHello, World!\n```"); - } - - #[gpui::test(iterations = 10)] - async fn test_interleaved_stdout_stderr(cx: &mut TestAppContext) { - cx.executor().allow_parking(); - - let command = "echo 'stdout 1' && sleep 0.01 && echo 'stderr 1' >&2 && sleep 0.01 && echo 'stdout 2' && sleep 0.01 && echo 'stderr 2' >&2"; - let result = run_command_limited(Path::new(".").into(), command.to_string()).await; - - assert!(result.is_ok()); - assert_eq!( - result.unwrap(), - "```\nstdout 1\nstderr 1\nstdout 2\nstderr 2\n```" - ); - } - - #[gpui::test(iterations = 10)] - async fn test_multiple_output_reads(cx: &mut TestAppContext) { - cx.executor().allow_parking(); - - // Command with multiple outputs that might require multiple reads - let result = run_command_limited( - Path::new(".").into(), - "echo '1'; sleep 0.01; echo '2'; sleep 0.01; echo '3'".to_string(), + let content = if should_truncate { + format!( + "Command output too long. The first {} bytes:\n\n{}", + content.len(), + content, ) - .await; + } else { + content + }; - assert!(result.is_ok()); - assert_eq!(result.unwrap(), "```\n1\n2\n3\n```"); - } + let content = match exit_status { + Some(exit_status) if exit_status.success() => { + if is_empty { + "Command executed successfully.".to_string() + } else { + content.to_string() + } + } + Some(exit_status) => { + let code = exit_status.code().unwrap_or(-1); + if is_empty { + format!("Command \"{command}\" failed with exit code {code}.") + } else { + format!("Command \"{command}\" failed with exit code {code}.\n\n{content}") + } + } + None => { + format!( + "Command failed or was interrupted.\nPartial output captured:\n\n{}", + content, + ) + } + }; + (content, is_empty) +} - #[gpui::test(iterations = 10)] - async fn test_output_truncation_single_line(cx: &mut TestAppContext) { - cx.executor().allow_parking(); +fn working_dir( + cx: &mut App, + input: &TerminalToolInput, + project: &Entity, + input_path: &Path, +) -> Result, &'static str> { + let project = project.read(cx); - let cmd = format!("echo '{}'; sleep 0.01;", "X".repeat(LIMIT * 2)); + if input.cd == "." { + // Accept "." as meaning "the one worktree" if we only have one worktree. + let mut worktrees = project.worktrees(cx); - let result = run_command_limited(Path::new(".").into(), cmd).await; + match worktrees.next() { + Some(worktree) => { + if worktrees.next().is_some() { + return Err( + "'.' is ambiguous in multi-root workspaces. Please specify a root directory explicitly.", + ); + } + Ok(Some(worktree.read(cx).abs_path().to_path_buf())) + } + None => Ok(None), + } + } else if input_path.is_absolute() { + // Absolute paths are allowed, but only if they're in one of the project's worktrees. + if !project + .worktrees(cx) + .any(|worktree| input_path.starts_with(&worktree.read(cx).abs_path())) + { + return Err("The absolute path must be within one of the project's worktrees"); + } - assert!(result.is_ok()); - let output = result.unwrap(); + Ok(Some(input_path.into())) + } else { + let Some(worktree) = project.worktree_for_root_name(&input.cd, cx) else { + return Err("`cd` directory {} not found in the project"); + }; - let content_start = output.find("```\n").map(|i| i + 4).unwrap_or(0); - let content_end = output.rfind("\n```").unwrap_or(output.len()); - let content_length = content_end - content_start; - - // Output should be exactly the limit - assert_eq!(content_length, LIMIT); - } - - #[gpui::test(iterations = 10)] - async fn test_output_truncation_multiline(cx: &mut TestAppContext) { - cx.executor().allow_parking(); - - let cmd = format!("echo '{}'; ", "X".repeat(120)).repeat(160); - let result = run_command_limited(Path::new(".").into(), cmd).await; - - assert!(result.is_ok()); - let output = result.unwrap(); - - assert!(output.starts_with("Command output too long. The first 16334 bytes:\n\n")); - - let content_start = output.find("```\n").map(|i| i + 4).unwrap_or(0); - let content_end = output.rfind("\n```").unwrap_or(output.len()); - let content_length = content_end - content_start; - - assert!(content_length <= LIMIT); - } - - #[gpui::test(iterations = 10)] - async fn test_command_failure(cx: &mut TestAppContext) { - cx.executor().allow_parking(); - - let result = run_command_limited(Path::new(".").into(), "exit 42".to_string()).await; - - assert!(result.is_ok()); - let output = result.unwrap(); - - // Extract the shell name from path for cleaner test output - let shell_path = std::env::var("SHELL").unwrap_or("bash".to_string()); - - let expected_output = format!( - "Command failed with exit code 42 (shell: {}).\n\n```\n\n```", - shell_path - ); - assert_eq!(output, expected_output); + Ok(Some(worktree.read(cx).abs_path().to_path_buf())) + } +} + +struct TerminalToolCard { + input_command: String, + working_dir: Option, + entity_id: EntityId, + exit_status: Option, + terminal: Option>, + command_finished: bool, + was_content_truncated: bool, + finished_with_empty_output: bool, + content_line_count: usize, + original_content_len: usize, + preview_expanded: bool, + start_instant: Instant, + elapsed_time: Option, +} + +impl TerminalToolCard { + pub fn new(input_command: String, working_dir: Option, entity_id: EntityId) -> Self { + Self { + input_command, + working_dir, + entity_id, + exit_status: None, + terminal: None, + command_finished: false, + was_content_truncated: false, + finished_with_empty_output: false, + original_content_len: 0, + content_line_count: 0, + preview_expanded: true, + start_instant: Instant::now(), + elapsed_time: None, + } + } +} + +impl ToolCard for TerminalToolCard { + fn render( + &mut self, + status: &ToolUseStatus, + _window: &mut Window, + _workspace: WeakEntity, + cx: &mut Context, + ) -> impl IntoElement { + let Some(terminal) = self.terminal.as_ref() else { + return Empty.into_any(); + }; + + let tool_failed = matches!(status, ToolUseStatus::Error(_)); + let command_failed = + self.command_finished && self.exit_status.is_none_or(|code| !code.success()); + if (tool_failed || command_failed) && self.elapsed_time.is_none() { + self.elapsed_time = Some(self.start_instant.elapsed()); + } + let time_elapsed = self + .elapsed_time + .unwrap_or_else(|| self.start_instant.elapsed()); + let should_hide_terminal = + tool_failed || self.finished_with_empty_output || !self.preview_expanded; + + let border_color = cx.theme().colors().border.opacity(0.6); + let header_bg = cx + .theme() + .colors() + .element_background + .blend(cx.theme().colors().editor_foreground.opacity(0.025)); + + let header_label = h_flex() + .w_full() + .max_w_full() + .px_1() + .gap_0p5() + .opacity(0.8) + .child( + h_flex() + .child( + Icon::new(IconName::Terminal) + .size(IconSize::XSmall) + .color(Color::Muted), + ) + .child( + div() + .id(("terminal-tool-header-input-command", self.entity_id)) + .text_size(rems(0.8125)) + .font_buffer(cx) + .child(self.input_command.clone()) + .ml_1p5() + .mr_0p5() + .tooltip({ + let path = self + .working_dir + .as_ref() + .cloned() + .or_else(|| env::current_dir().ok()) + .map(|path| format!("\"{}\"", path.display())) + .unwrap_or_else(|| "current directory".to_string()); + Tooltip::text(if self.command_finished { + format!("Ran in {path}") + } else { + format!("Running in {path}") + }) + }), + ), + ) + .into_any_element(); + + let header = h_flex() + .flex_none() + .p_1() + .gap_1() + .justify_between() + .rounded_t_md() + .bg(header_bg) + .child(header_label) + .map(|header| { + let header = header + .when(self.was_content_truncated, |header| { + let tooltip = + if self.content_line_count + 10 > terminal::MAX_SCROLL_HISTORY_LINES { + "Output exceeded terminal max lines and was \ + truncated, the model received the first 16 KB." + .to_string() + } else { + format!( + "Output is {} long, to avoid unexpected token usage, \ + only 16 KB was sent back to the model.", + format_file_size(self.original_content_len as u64, true), + ) + }; + header.child( + div() + .id(("terminal-tool-truncated-label", self.entity_id)) + .tooltip(Tooltip::text(tooltip)) + .child( + Label::new("(truncated)") + .color(Color::Disabled) + .size(LabelSize::Small), + ), + ) + }) + .when(time_elapsed > Duration::from_secs(10), |header| { + header.child( + Label::new(format!("({})", duration_alt_display(time_elapsed))) + .buffer_font(cx) + .color(Color::Disabled) + .size(LabelSize::Small), + ) + }); + + if tool_failed || command_failed { + header.child( + div() + .id(("terminal-tool-error-code-indicator", self.entity_id)) + .child( + Icon::new(IconName::Close) + .size(IconSize::Small) + .color(Color::Error), + ) + .when(command_failed && self.exit_status.is_some(), |this| { + this.tooltip(Tooltip::text(format!( + "Exited with code {}", + self.exit_status + .and_then(|status| status.code()) + .unwrap_or(-1), + ))) + }) + .when( + !command_failed && tool_failed && status.error().is_some(), + |this| { + this.tooltip(Tooltip::text(format!( + "Error: {}", + status.error().unwrap(), + ))) + }, + ), + ) + } else if self.command_finished { + header.child( + Icon::new(IconName::Check) + .size(IconSize::Small) + .color(Color::Success), + ) + } else { + header.child( + Icon::new(IconName::ArrowCircle) + .size(IconSize::Small) + .color(Color::Info) + .with_animation( + "arrow-circle", + Animation::new(Duration::from_secs(2)).repeat(), + |icon, delta| { + icon.transform(Transformation::rotate(percentage(delta))) + }, + ), + ) + } + }) + .when(!tool_failed && !self.finished_with_empty_output, |header| { + header.child( + Disclosure::new( + ("terminal-tool-disclosure", self.entity_id), + self.preview_expanded, + ) + .opened_icon(IconName::ChevronUp) + .closed_icon(IconName::ChevronDown) + .on_click(cx.listener( + move |this, _event, _window, _cx| { + this.preview_expanded = !this.preview_expanded; + }, + )), + ) + }); + + v_flex() + .mb_2() + .border_1() + .when(tool_failed || command_failed, |card| card.border_dashed()) + .border_color(border_color) + .rounded_lg() + .overflow_hidden() + .child(header) + .when(!should_hide_terminal, |this| { + this.child(div().child(terminal.clone()).min_h(px(250.0))) + }) + .into_any() } } diff --git a/crates/assistant_tools/src/terminal_tool/description.md b/crates/assistant_tools/src/terminal_tool/description.md index 23b44af33a..0b0501aae4 100644 --- a/crates/assistant_tools/src/terminal_tool/description.md +++ b/crates/assistant_tools/src/terminal_tool/description.md @@ -1,6 +1,8 @@ Executes a shell one-liner and returns the combined output. -This tool spawns a process using the user's current shell, combines stdout and stderr into one interleaved stream as they are produced (preserving the order of writes), and captures that stream into a string which is returned. +This tool spawns a process using the user's shell, reads from stdout and stderr (preserving the order of writes), and returns a string with the combined output result. + +The output results will be shown to the user already, only list it again if necessary, avoid being redundant. Make sure you use the `cd` parameter to navigate to one of the root directories of the project. NEVER do it as part of the `command` itself, otherwise it will error. diff --git a/crates/assistant_tools/src/web_search_tool.rs b/crates/assistant_tools/src/web_search_tool.rs index db33c0cacf..be236c7a6d 100644 --- a/crates/assistant_tools/src/web_search_tool.rs +++ b/crates/assistant_tools/src/web_search_tool.rs @@ -23,7 +23,6 @@ pub struct WebSearchToolInput { query: String, } -#[derive(RegisterComponent)] pub struct WebSearchTool; impl Tool for WebSearchTool { @@ -84,6 +83,7 @@ impl Tool for WebSearchTool { } } +#[derive(RegisterComponent)] struct WebSearchToolCard { response: Option>, _task: Task<()>, @@ -185,15 +185,11 @@ impl ToolCard for WebSearchToolCard { } } -impl Component for WebSearchTool { +impl Component for WebSearchToolCard { fn scope() -> ComponentScope { ComponentScope::Agent } - fn sort_name() -> &'static str { - "ToolWebSearch" - } - fn preview(window: &mut Window, cx: &mut App) -> Option { let in_progress_search = cx.new(|cx| WebSearchToolCard { response: None, diff --git a/crates/image_viewer/src/image_info.rs b/crates/image_viewer/src/image_info.rs index 365a2e381d..70a92736aa 100644 --- a/crates/image_viewer/src/image_info.rs +++ b/crates/image_viewer/src/image_info.rs @@ -2,6 +2,7 @@ use gpui::{Context, Entity, IntoElement, ParentElement, Render, Subscription, di use project::image_store::{ImageFormat, ImageMetadata}; use settings::Settings; use ui::prelude::*; +use util::size::format_file_size; use workspace::{ItemHandle, StatusItemView, Workspace}; use crate::{ImageFileSizeUnit, ImageView, ImageViewerSettings}; @@ -36,27 +37,9 @@ impl ImageInfo { } } -fn format_file_size(size: u64, image_unit_type: ImageFileSizeUnit) -> String { - match image_unit_type { - ImageFileSizeUnit::Binary => { - if size < 1024 { - format!("{size}B") - } else if size < 1024 * 1024 { - format!("{:.1}KiB", size as f64 / 1024.0) - } else { - format!("{:.1}MiB", size as f64 / (1024.0 * 1024.0)) - } - } - ImageFileSizeUnit::Decimal => { - if size < 1000 { - format!("{size}B") - } else if size < 1000 * 1000 { - format!("{:.1}KB", size as f64 / 1000.0) - } else { - format!("{:.1}MB", size as f64 / (1000.0 * 1000.0)) - } - } - } +fn format_image_size(size: u64, image_unit_type: ImageFileSizeUnit) -> String { + let use_decimal = matches!(image_unit_type, ImageFileSizeUnit::Decimal); + format_file_size(size, use_decimal) } impl Render for ImageInfo { @@ -69,7 +52,7 @@ impl Render for ImageInfo { let mut components = Vec::new(); components.push(format!("{}x{}", metadata.width, metadata.height)); - components.push(format_file_size(metadata.file_size, settings.unit)); + components.push(format_image_size(metadata.file_size, settings.unit)); if let Some(colors) = metadata.colors { components.push(format!( diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index 81467a1a7b..14418e85b5 100644 --- a/crates/project/src/terminals.rs +++ b/crates/project/src/terminals.rs @@ -514,7 +514,7 @@ impl Project { terminal_handle: &Entity, cx: &mut App, ) { - terminal_handle.update(cx, |terminal, _| terminal.input_bytes(command.into_bytes())); + terminal_handle.update(cx, |terminal, _| terminal.input(command)); } pub fn local_terminal_handles(&self) -> &Vec> { diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index a2a184f6a3..0ed8cc1acb 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -313,7 +313,7 @@ impl Display for TerminalError { // https://github.com/alacritty/alacritty/blob/cb3a79dbf6472740daca8440d5166c1d4af5029e/extra/man/alacritty.5.scd?plain=1#L207-L213 const DEFAULT_SCROLL_HISTORY_LINES: usize = 10_000; -const MAX_SCROLL_HISTORY_LINES: usize = 100_000; +pub const MAX_SCROLL_HISTORY_LINES: usize = 100_000; const URL_REGEX: &str = r#"(ipfs:|ipns:|magnet:|mailto:|gemini://|gopher://|https://|http://|news:|file://|git://|ssh:|ftp://)[^\u{0000}-\u{001F}\u{007F}-\u{009F}<>"\s{-}\^⟨⟩`]+"#; // Optional suffix matches MSBuild diagnostic suffixes for path parsing in PathLikeWithPosition // https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-diagnostic-format-for-tasks @@ -1216,15 +1216,11 @@ impl Terminal { } ///Write the Input payload to the tty. - fn write_to_pty(&self, input: String) { - self.pty_tx.notify(input.into_bytes()); + fn write_to_pty(&self, input: impl Into>) { + self.pty_tx.notify(input.into()); } - fn write_bytes_to_pty(&self, input: Vec) { - self.pty_tx.notify(input); - } - - pub fn input(&mut self, input: String) { + pub fn input(&mut self, input: impl Into>) { self.events .push_back(InternalEvent::Scroll(AlacScroll::Bottom)); self.events.push_back(InternalEvent::SetSelection(None)); @@ -1232,14 +1228,6 @@ impl Terminal { self.write_to_pty(input); } - pub fn input_bytes(&mut self, input: Vec) { - self.events - .push_back(InternalEvent::Scroll(AlacScroll::Bottom)); - self.events.push_back(InternalEvent::SetSelection(None)); - - self.write_bytes_to_pty(input); - } - pub fn toggle_vi_mode(&mut self) { self.events.push_back(InternalEvent::ToggleViMode); } @@ -1420,6 +1408,13 @@ impl Terminal { } } + pub fn get_content(&self) -> String { + let term = self.term.lock_unfair(); + let start = AlacPoint::new(term.topmost_line(), Column(0)); + let end = AlacPoint::new(term.bottommost_line(), term.last_column()); + term.bounds_to_string(start, end) + } + pub fn last_n_non_empty_lines(&self, n: usize) -> Vec { let term = self.term.clone(); let terminal = term.lock_unfair(); @@ -1858,6 +1853,8 @@ impl Terminal { let completion_receiver = task.completion_rx.clone(); return cx .spawn(async move |_| completion_receiver.recv().await.log_err().flatten()); + } else if let Ok(status) = task.completion_rx.try_recv() { + return Task::ready(status); } } Task::ready(None) diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index e41f535f3e..71e89286dc 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/crates/terminal_view/src/terminal_element.rs @@ -1036,7 +1036,7 @@ impl InputHandler for TerminalInputHandler { cx: &mut App, ) { self.terminal.update(cx, |terminal, _| { - terminal.input(text.into()); + terminal.input(text); }); self.workspace diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index d9cd791068..ad2a55e705 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -1133,7 +1133,7 @@ async fn wait_for_terminals_tasks( }) .ok() }); - let _: Vec<_> = join_all(pending_tasks).await; + join_all(pending_tasks).await; } fn add_paths_to_terminal( diff --git a/crates/util/src/size.rs b/crates/util/src/size.rs new file mode 100644 index 0000000000..084a0e5a56 --- /dev/null +++ b/crates/util/src/size.rs @@ -0,0 +1,48 @@ +pub fn format_file_size(size: u64, use_decimal: bool) -> String { + if use_decimal { + if size < 1000 { + format!("{size}B") + } else if size < 1000 * 1000 { + format!("{:.1}KB", size as f64 / 1000.0) + } else { + format!("{:.1}MB", size as f64 / (1000.0 * 1000.0)) + } + } else { + if size < 1024 { + format!("{size}B") + } else if size < 1024 * 1024 { + format!("{:.1}KiB", size as f64 / 1024.0) + } else { + format!("{:.1}MiB", size as f64 / (1024.0 * 1024.0)) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_format_file_size_decimal() { + assert_eq!(format_file_size(0, true), "0B"); + assert_eq!(format_file_size(999, true), "999B"); + assert_eq!(format_file_size(1000, true), "1.0KB"); + assert_eq!(format_file_size(1500, true), "1.5KB"); + assert_eq!(format_file_size(999999, true), "1000.0KB"); + assert_eq!(format_file_size(1000000, true), "1.0MB"); + assert_eq!(format_file_size(1500000, true), "1.5MB"); + assert_eq!(format_file_size(10000000, true), "10.0MB"); + } + + #[test] + fn test_format_file_size_binary() { + assert_eq!(format_file_size(0, false), "0B"); + assert_eq!(format_file_size(1023, false), "1023B"); + assert_eq!(format_file_size(1024, false), "1.0KiB"); + assert_eq!(format_file_size(1536, false), "1.5KiB"); + assert_eq!(format_file_size(1048575, false), "1024.0KiB"); + assert_eq!(format_file_size(1048576, false), "1.0MiB"); + assert_eq!(format_file_size(1572864, false), "1.5MiB"); + assert_eq!(format_file_size(10485760, false), "10.0MiB"); + } +} diff --git a/crates/util/src/time.rs b/crates/util/src/time.rs new file mode 100644 index 0000000000..365de6f432 --- /dev/null +++ b/crates/util/src/time.rs @@ -0,0 +1,41 @@ +use std::time::Duration; + +pub fn duration_alt_display(duration: Duration) -> String { + if duration < Duration::from_secs(60) { + format!("{}s", duration.as_secs()) + } else { + duration_clock_format(duration) + } +} + +fn duration_clock_format(duration: Duration) -> String { + let hours = duration.as_secs() / 3600; + let minutes = (duration.as_secs() % 3600) / 60; + let seconds = duration.as_secs() % 60; + + if hours > 0 { + format!("{hours}:{minutes:02}:{seconds:02}") + } else if minutes > 0 { + format!("{minutes}:{seconds:02}") + } else { + format!("{seconds}") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_duration_to_clock_format() { + use duration_clock_format as f; + assert_eq!("0", f(Duration::from_secs(0))); + assert_eq!("59", f(Duration::from_secs(59))); + assert_eq!("1:00", f(Duration::from_secs(60))); + assert_eq!("10:00", f(Duration::from_secs(600))); + assert_eq!("1:00:00", f(Duration::from_secs(3600))); + assert_eq!("3:02:01", f(Duration::from_secs(3600 * 3 + 60 * 2 + 1))); + assert_eq!("23:59:59", f(Duration::from_secs(3600 * 24 - 1))); + assert_eq!("100:00:00", f(Duration::from_secs(3600 * 100))); + } +} diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index e19e8ddc84..25bb104a5f 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -4,8 +4,10 @@ pub mod fs; pub mod markdown; pub mod paths; pub mod serde; +pub mod size; #[cfg(any(test, feature = "test-support"))] pub mod test; +pub mod time; use anyhow::Result; use futures::Future;