Fix tools' `ui_text` to use inline code escaping (#27543)

Michael Sloan created 9 months ago

Markdown escaping was added in #27502.

Release Notes:

- N/A

Change summary

crates/assistant_tools/src/bash_tool.rs             |  5 ++---
crates/assistant_tools/src/copy_path_tool.rs        |  6 +++---
crates/assistant_tools/src/create_directory_tool.rs |  5 ++++-
crates/assistant_tools/src/create_file_tool.rs      |  4 ++--
crates/assistant_tools/src/diagnostics_tool.rs      |  4 ++--
crates/assistant_tools/src/list_directory_tool.rs   |  4 ++--
crates/assistant_tools/src/move_path_tool.rs        | 10 +++++-----
crates/assistant_tools/src/read_file_tool.rs        |  4 ++--
crates/assistant_tools/src/regex_search_tool.rs     |  6 +++---
crates/util/src/markdown.rs                         | 10 +++++++++-
10 files changed, 34 insertions(+), 24 deletions(-)

Detailed changes

crates/assistant_tools/src/bash_tool.rs

@@ -45,11 +45,10 @@ impl Tool for BashTool {
     fn ui_text(&self, input: &serde_json::Value) -> String {
         match serde_json::from_value::<BashToolInput>(input.clone()) {
             Ok(input) => {
-                let cmd = MarkdownString::escape(&input.command);
                 if input.command.contains('\n') {
-                    format!("```bash\n{cmd}\n```")
+                    MarkdownString::code_block("bash", &input.command).0
                 } else {
-                    format!("`{cmd}`")
+                    MarkdownString::inline_code(&input.command).0
                 }
             }
             Err(_) => "Run bash command".to_string(),

crates/assistant_tools/src/copy_path_tool.rs

@@ -61,9 +61,9 @@ impl Tool for CopyPathTool {
     fn ui_text(&self, input: &serde_json::Value) -> String {
         match serde_json::from_value::<CopyPathToolInput>(input.clone()) {
             Ok(input) => {
-                let src = MarkdownString::escape(&input.source_path);
-                let dest = MarkdownString::escape(&input.destination_path);
-                format!("Copy `{src}` to `{dest}`")
+                let src = MarkdownString::inline_code(&input.source_path);
+                let dest = MarkdownString::inline_code(&input.destination_path);
+                format!("Copy {src} to {dest}")
             }
             Err(_) => "Copy path".to_string(),
         }

crates/assistant_tools/src/create_directory_tool.rs

@@ -51,7 +51,10 @@ impl Tool for CreateDirectoryTool {
     fn ui_text(&self, input: &serde_json::Value) -> String {
         match serde_json::from_value::<CreateDirectoryToolInput>(input.clone()) {
             Ok(input) => {
-                format!("Create directory `{}`", MarkdownString::escape(&input.path))
+                format!(
+                    "Create directory {}",
+                    MarkdownString::inline_code(&input.path)
+                )
             }
             Err(_) => "Create directory".to_string(),
         }

crates/assistant_tools/src/create_file_tool.rs

@@ -58,8 +58,8 @@ impl Tool for CreateFileTool {
     fn ui_text(&self, input: &serde_json::Value) -> String {
         match serde_json::from_value::<CreateFileToolInput>(input.clone()) {
             Ok(input) => {
-                let path = MarkdownString::escape(&input.path);
-                format!("Create file `{path}`")
+                let path = MarkdownString::inline_code(&input.path);
+                format!("Create file {path}")
             }
             Err(_) => "Create file".to_string(),
         }

crates/assistant_tools/src/diagnostics_tool.rs

@@ -66,11 +66,11 @@ impl Tool for DiagnosticsTool {
         if let Some(path) = serde_json::from_value::<DiagnosticsToolInput>(input.clone())
             .ok()
             .and_then(|input| match input.path {
-                Some(path) if !path.is_empty() => Some(MarkdownString::escape(&path)),
+                Some(path) if !path.is_empty() => Some(MarkdownString::inline_code(&path)),
                 _ => None,
             })
         {
-            format!("Check diagnostics for `{path}`")
+            format!("Check diagnostics for {path}")
         } else {
             "Check project diagnostics".to_string()
         }

crates/assistant_tools/src/list_directory_tool.rs

@@ -63,8 +63,8 @@ impl Tool for ListDirectoryTool {
     fn ui_text(&self, input: &serde_json::Value) -> String {
         match serde_json::from_value::<ListDirectoryToolInput>(input.clone()) {
             Ok(input) => {
-                let path = MarkdownString::escape(&input.path);
-                format!("List the `{path}` directory's contents")
+                let path = MarkdownString::inline_code(&input.path);
+                format!("List the {path} directory's contents")
             }
             Err(_) => "List directory".to_string(),
         }

crates/assistant_tools/src/move_path_tool.rs

@@ -61,8 +61,8 @@ impl Tool for MovePathTool {
     fn ui_text(&self, input: &serde_json::Value) -> String {
         match serde_json::from_value::<MovePathToolInput>(input.clone()) {
             Ok(input) => {
-                let src = MarkdownString::escape(&input.source_path);
-                let dest = MarkdownString::escape(&input.destination_path);
+                let src = MarkdownString::inline_code(&input.source_path);
+                let dest = MarkdownString::inline_code(&input.destination_path);
                 let src_path = Path::new(&input.source_path);
                 let dest_path = Path::new(&input.destination_path);
 
@@ -71,11 +71,11 @@ impl Tool for MovePathTool {
                     .and_then(|os_str| os_str.to_os_string().into_string().ok())
                 {
                     Some(filename) if src_path.parent() == dest_path.parent() => {
-                        let filename = MarkdownString::escape(&filename);
-                        format!("Rename `{src}` to `{filename}`")
+                        let filename = MarkdownString::inline_code(&filename);
+                        format!("Rename {src} to {filename}")
                     }
                     _ => {
-                        format!("Move `{src}` to `{dest}`")
+                        format!("Move {src} to {dest}")
                     }
                 }
             }

crates/assistant_tools/src/read_file_tool.rs

@@ -66,8 +66,8 @@ impl Tool for ReadFileTool {
     fn ui_text(&self, input: &serde_json::Value) -> String {
         match serde_json::from_value::<ReadFileToolInput>(input.clone()) {
             Ok(input) => {
-                let path = MarkdownString::escape(&input.path.display().to_string());
-                format!("Read file `{path}`")
+                let path = MarkdownString::inline_code(&input.path.display().to_string());
+                format!("Read file {path}")
             }
             Err(_) => "Read file".to_string(),
         }

crates/assistant_tools/src/regex_search_tool.rs

@@ -64,12 +64,12 @@ impl Tool for RegexSearchTool {
         match serde_json::from_value::<RegexSearchToolInput>(input.clone()) {
             Ok(input) => {
                 let page = input.page();
-                let regex = MarkdownString::escape(&input.regex);
+                let regex = MarkdownString::inline_code(&input.regex);
 
                 if page > 1 {
-                    format!("Get page {page} of search results for regex “`{regex}`”")
+                    format!("Get page {page} of search results for regex “{regex}”")
                 } else {
-                    format!("Search files for regex “`{regex}`”")
+                    format!("Search files for regex “{regex}”")
                 }
             }
             Err(_) => "Search with regex".to_string(),

crates/util/src/markdown.rs

@@ -11,7 +11,9 @@ impl Display for MarkdownString {
 }
 
 impl MarkdownString {
-    /// Escapes markdown special characters.
+    /// Escapes markdown special characters in markdown text blocks. Markdown code blocks follow
+    /// different rules and `MarkdownString::inline_code` or `MarkdownString::code_block` should be
+    /// used in that case.
     ///
     /// Also escapes the following markdown extensions:
     ///
@@ -134,6 +136,12 @@ impl MarkdownString {
             Self(format!("{backticks}{space}{text}{space}{backticks}"))
         }
     }
+
+    /// Returns markdown for code blocks, wrapped in 3 or more backticks as needed.
+    pub fn code_block(tag: &str, text: &str) -> Self {
+        let backticks = "`".repeat(3.max(count_max_consecutive_chars(text, '`') + 1));
+        Self(format!("{backticks}{tag}\n{text}\n{backticks}\n"))
+    }
 }
 
 // Copied from `pulldown-cmark-to-cmark-20.0.0` with changed names.