Skip to content

Commit 0659330

Browse files
authored
chore: slash command refactor cleanup (#274)
* checkpoint * checkpoint 2 * checkpoint 3 * checkpoint 4 * checkpoint 5 * checkpoint 6 * test fixes * blah
1 parent b258c01 commit 0659330

File tree

8 files changed

+127
-152
lines changed

8 files changed

+127
-152
lines changed

crates/chat-cli/src/cli/chat/mod.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ pub enum ChatError {
394394
#[error("interrupted")]
395395
Interrupted { tool_uses: Option<Vec<QueuedTool>> },
396396
#[error(
397-
"Tool approval required but --no-interactive was specified. Use --trust-all-tools to automatically approve tools."
397+
"Tool approval required but --non-interactive was specified. Use --trust-all-tools to automatically approve tools."
398398
)]
399399
NonInteractiveToolApproval,
400400
#[error(transparent)]
@@ -638,7 +638,7 @@ impl ChatSession {
638638
_ => (),
639639
}
640640

641-
("Tool use was interupted", Report::from(err))
641+
("Tool use was interrupted", Report::from(err))
642642
},
643643
ChatError::Client(err) => match err {
644644
// Errors from attempting to send too large of a conversation history. In
@@ -2193,7 +2193,6 @@ mod tests {
21932193

21942194
#[tokio::test]
21952195
async fn test_flow() {
2196-
// let _ = tracing_subscriber::fmt::try_init();
21972196
let mut ctx = Context::new();
21982197
let test_client = create_stream(serde_json::json!([
21992198
[
@@ -2242,7 +2241,7 @@ mod tests {
22422241
)
22432242
.await
22442243
.unwrap()
2245-
.next(&mut ctx, &mut database, &telemetry)
2244+
.spawn(&mut ctx, &mut database, &telemetry)
22462245
.await
22472246
.unwrap();
22482247

crates/chat-cli/src/cli/chat/tool_manager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,7 @@ impl ToolManager {
903903
let _ = queue!(
904904
output,
905905
style::Print(
906-
"Not all mcp servers loaded. Configure no-interactive timeout with q settings mcp.noInteractiveTimeout"
906+
"Not all mcp servers loaded. Configure non-interactive timeout with q settings mcp.noInteractiveTimeout"
907907
),
908908
style::Print("\n------\n")
909909
);

crates/chat-cli/src/cli/chat/tools/fs_read.rs

Lines changed: 20 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -548,39 +548,12 @@ fn format_mode(mode: u32) -> [char; 9] {
548548

549549
#[cfg(test)]
550550
mod tests {
551-
use std::sync::Arc;
552-
553551
use super::*;
554-
555-
const TEST_FILE_CONTENTS: &str = "\
556-
1: Hello world!
557-
2: This is line 2
558-
3: asdf
559-
4: Hello world!
560-
";
561-
562-
const TEST_FILE_PATH: &str = "/test_file.txt";
563-
const TEST_HIDDEN_FILE_PATH: &str = "/aaaa2/.hidden";
564-
565-
/// Sets up the following filesystem structure:
566-
/// ```text
567-
/// test_file.txt
568-
/// /home/testuser/
569-
/// /aaaa1/
570-
/// /bbbb1/
571-
/// /cccc1/
572-
/// /aaaa2/
573-
/// .hidden
574-
/// ```
575-
async fn setup_test_directory() -> Arc<Context> {
576-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
577-
let fs = ctx.fs();
578-
fs.write(TEST_FILE_PATH, TEST_FILE_CONTENTS).await.unwrap();
579-
fs.create_dir_all("/aaaa1/bbbb1/cccc1").await.unwrap();
580-
fs.create_dir_all("/aaaa2").await.unwrap();
581-
fs.write(TEST_HIDDEN_FILE_PATH, "this is a hidden file").await.unwrap();
582-
ctx
583-
}
552+
use crate::cli::chat::util::test::{
553+
TEST_FILE_CONTENTS,
554+
TEST_FILE_PATH,
555+
setup_test_directory,
556+
};
584557

585558
#[test]
586559
fn test_negative_index_conversion() {
@@ -772,13 +745,12 @@ mod tests {
772745

773746
#[tokio::test]
774747
async fn test_fs_read_non_utf8_binary_file() {
775-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
776-
let fs = ctx.fs();
748+
let ctx = Context::new();
777749
let mut stdout = std::io::stdout();
778750

779751
let binary_data = vec![0xff, 0xfe, 0xfd, 0xfc, 0xfb, 0xfa, 0xf9, 0xf8];
780752
let binary_file_path = "/binary_test.dat";
781-
fs.write(binary_file_path, &binary_data).await.unwrap();
753+
ctx.fs.write(binary_file_path, &binary_data).await.unwrap();
782754

783755
let v = serde_json::json!({
784756
"path": binary_file_path,
@@ -804,13 +776,12 @@ mod tests {
804776

805777
#[tokio::test]
806778
async fn test_fs_read_latin1_encoded_file() {
807-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
808-
let fs = ctx.fs();
779+
let ctx = Context::new();
809780
let mut stdout = std::io::stdout();
810781

811782
let latin1_data = vec![99, 97, 102, 233]; // "café" in Latin-1
812783
let latin1_file_path = "/latin1_test.txt";
813-
fs.write(latin1_file_path, &latin1_data).await.unwrap();
784+
ctx.fs.write(latin1_file_path, &latin1_data).await.unwrap();
814785

815786
let v = serde_json::json!({
816787
"path": latin1_file_path,
@@ -836,8 +807,7 @@ mod tests {
836807

837808
#[tokio::test]
838809
async fn test_fs_search_non_utf8_file() {
839-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
840-
let fs = ctx.fs();
810+
let ctx = Context::new();
841811
let mut stdout = std::io::stdout();
842812

843813
let mut mixed_data = Vec::new();
@@ -846,7 +816,7 @@ mod tests {
846816
mixed_data.extend_from_slice(b"\nGoodbye world\n");
847817

848818
let mixed_file_path = "/mixed_encoding_test.txt";
849-
fs.write(mixed_file_path, &mixed_data).await.unwrap();
819+
ctx.fs.write(mixed_file_path, &mixed_data).await.unwrap();
850820

851821
let v = serde_json::json!({
852822
"mode": "Search",
@@ -896,8 +866,7 @@ mod tests {
896866

897867
#[tokio::test]
898868
async fn test_fs_read_windows1252_encoded_file() {
899-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
900-
let fs = ctx.fs();
869+
let ctx = Context::new();
901870
let mut stdout = std::io::stdout();
902871

903872
let mut windows1252_data = Vec::new();
@@ -907,7 +876,7 @@ mod tests {
907876
windows1252_data.push(0x94); // Right double quotation mark in Windows-1252
908877

909878
let windows1252_file_path = "/windows1252_test.txt";
910-
fs.write(windows1252_file_path, &windows1252_data).await.unwrap();
879+
ctx.fs.write(windows1252_file_path, &windows1252_data).await.unwrap();
911880

912881
let v = serde_json::json!({
913882
"path": windows1252_file_path,
@@ -933,8 +902,7 @@ mod tests {
933902

934903
#[tokio::test]
935904
async fn test_fs_search_pattern_with_replacement_chars() {
936-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
937-
let fs = ctx.fs();
905+
let ctx = Context::new();
938906
let mut stdout = std::io::stdout();
939907

940908
let mut data_with_invalid_utf8 = Vec::new();
@@ -943,7 +911,10 @@ mod tests {
943911
data_with_invalid_utf8.extend_from_slice(b"\nLine 2: hello world\n");
944912

945913
let invalid_utf8_file_path = "/invalid_utf8_search_test.txt";
946-
fs.write(invalid_utf8_file_path, &data_with_invalid_utf8).await.unwrap();
914+
ctx.fs
915+
.write(invalid_utf8_file_path, &data_with_invalid_utf8)
916+
.await
917+
.unwrap();
947918

948919
let v = serde_json::json!({
949920
"mode": "Search",
@@ -968,13 +939,12 @@ mod tests {
968939

969940
#[tokio::test]
970941
async fn test_fs_read_empty_file_with_invalid_utf8() {
971-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
972-
let fs = ctx.fs();
942+
let ctx = Context::new();
973943
let mut stdout = std::io::stdout();
974944

975945
let invalid_only_data = vec![0xff, 0xfe, 0xfd];
976946
let invalid_only_file_path = "/invalid_only_test.txt";
977-
fs.write(invalid_only_file_path, &invalid_only_data).await.unwrap();
947+
ctx.fs.write(invalid_only_file_path, &invalid_only_data).await.unwrap();
978948

979949
let v = serde_json::json!({
980950
"path": invalid_only_file_path,

crates/chat-cli/src/cli/chat/tools/fs_write.rs

Lines changed: 67 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -596,38 +596,11 @@ fn syntect_to_crossterm_color(syntect: syntect::highlighting::Color) -> style::C
596596
#[cfg(test)]
597597
mod tests {
598598
use super::*;
599-
600-
const TEST_FILE_CONTENTS: &str = "\
601-
1: Hello world!
602-
2: This is line 2
603-
3: asdf
604-
4: Hello world!
605-
";
606-
607-
const TEST_FILE_PATH: &str = "/test_file.txt";
608-
const TEST_HIDDEN_FILE_PATH: &str = "/aaaa2/.hidden";
609-
610-
/// Sets up the following filesystem structure:
611-
/// ```text
612-
/// test_file.txt
613-
/// /home/testuser/
614-
/// /aaaa1/
615-
/// /bbbb1/
616-
/// /cccc1/
617-
/// /aaaa2/
618-
/// .hidden
619-
/// ```
620-
async fn setup_test_directory() -> Context {
621-
let ctx = Context::new();
622-
ctx.fs.write(TEST_FILE_PATH, TEST_FILE_CONTENTS).await.unwrap();
623-
ctx.fs.create_dir_all("/aaaa1/bbbb1/cccc1").await.unwrap();
624-
ctx.fs.create_dir_all("/aaaa2").await.unwrap();
625-
ctx.fs
626-
.write(TEST_HIDDEN_FILE_PATH, "this is a hidden file")
627-
.await
628-
.unwrap();
629-
ctx
630-
}
599+
use crate::cli::chat::util::test::{
600+
TEST_FILE_CONTENTS,
601+
TEST_FILE_PATH,
602+
setup_test_directory,
603+
};
631604

632605
#[test]
633606
fn test_fs_write_deserialize() {
@@ -960,74 +933,75 @@ mod tests {
960933
assert_eq!(terminal_width_required_for_line_count(100), 3);
961934
assert_eq!(terminal_width_required_for_line_count(999), 3);
962935
}
963-
}
964-
#[tokio::test]
965-
async fn test_fs_write_with_tilde_paths() {
966-
// Create a test context
967-
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
968-
let mut stdout = std::io::stdout();
969-
970-
// Get the home directory from the context
971-
let home_dir = ctx.env().home().unwrap_or_default();
972-
println!("Test home directory: {:?}", home_dir);
973-
974-
// Create a file directly in the home directory first to ensure it exists
975-
let home_path = ctx.fs().chroot_path(&home_dir);
976-
println!("Chrooted home path: {:?}", home_path);
977-
978-
// Ensure the home directory exists
979-
ctx.fs().create_dir_all(&home_path).await.unwrap();
980-
981-
let v = serde_json::json!({
982-
"path": "~/file.txt",
983-
"command": "create",
984-
"file_text": "content in home file"
985-
});
986-
987-
let result = serde_json::from_value::<FsWrite>(v)
988-
.unwrap()
989-
.invoke(&ctx, &mut stdout)
990-
.await;
991-
992-
match &result {
993-
Ok(_) => println!("Writing to ~/file.txt succeeded"),
994-
Err(e) => println!("Writing to ~/file.txt failed: {:?}", e),
995-
}
996936

997-
assert!(result.is_ok(), "Writing to ~/file.txt should succeed");
937+
#[tokio::test]
938+
async fn test_fs_write_with_tilde_paths() {
939+
// Create a test context
940+
let ctx = Context::new();
941+
let mut stdout = std::io::stdout();
942+
943+
// Get the home directory from the context
944+
let home_dir = ctx.env.home().unwrap_or_default();
945+
println!("Test home directory: {:?}", home_dir);
998946

999-
// Verify content was written correctly
1000-
let file_path = home_path.join("file.txt");
1001-
println!("Checking file at: {:?}", file_path);
947+
// Create a file directly in the home directory first to ensure it exists
948+
let home_path = ctx.fs.chroot_path(&home_dir);
949+
println!("Chrooted home path: {:?}", home_path);
1002950

1003-
let content_result = ctx.fs().read_to_string(&file_path).await;
1004-
match &content_result {
1005-
Ok(content) => println!("Read content: {:?}", content),
1006-
Err(e) => println!("Failed to read content: {:?}", e),
1007-
}
951+
// Ensure the home directory exists
952+
ctx.fs.create_dir_all(&home_path).await.unwrap();
953+
954+
let v = serde_json::json!({
955+
"path": "~/file.txt",
956+
"command": "create",
957+
"file_text": "content in home file"
958+
});
959+
960+
let result = serde_json::from_value::<FsWrite>(v)
961+
.unwrap()
962+
.invoke(&ctx, &mut stdout)
963+
.await;
964+
965+
match &result {
966+
Ok(_) => println!("Writing to ~/file.txt succeeded"),
967+
Err(e) => println!("Writing to ~/file.txt failed: {:?}", e),
968+
}
969+
970+
assert!(result.is_ok(), "Writing to ~/file.txt should succeed");
971+
972+
// Verify content was written correctly
973+
let file_path = home_path.join("file.txt");
974+
println!("Checking file at: {:?}", file_path);
975+
976+
let content_result = ctx.fs.read_to_string(&file_path).await;
977+
match &content_result {
978+
Ok(content) => println!("Read content: {:?}", content),
979+
Err(e) => println!("Failed to read content: {:?}", e),
980+
}
1008981

1009-
assert!(content_result.is_ok(), "Should be able to read from expanded path");
1010-
assert_eq!(content_result.unwrap(), "content in home file\n");
982+
assert!(content_result.is_ok(), "Should be able to read from expanded path");
983+
assert_eq!(content_result.unwrap(), "content in home file\n");
1011984

1012-
// Test with "~/nested/path/file.txt" to ensure deep paths work
1013-
let nested_dir = home_path.join("nested").join("path");
1014-
ctx.fs().create_dir_all(&nested_dir).await.unwrap();
985+
// Test with "~/nested/path/file.txt" to ensure deep paths work
986+
let nested_dir = home_path.join("nested").join("path");
987+
ctx.fs.create_dir_all(&nested_dir).await.unwrap();
1015988

1016-
let v = serde_json::json!({
1017-
"path": "~/nested/path/file.txt",
1018-
"command": "create",
1019-
"file_text": "content in nested path"
1020-
});
989+
let v = serde_json::json!({
990+
"path": "~/nested/path/file.txt",
991+
"command": "create",
992+
"file_text": "content in nested path"
993+
});
1021994

1022-
let result = serde_json::from_value::<FsWrite>(v)
1023-
.unwrap()
1024-
.invoke(&ctx, &mut stdout)
1025-
.await;
995+
let result = serde_json::from_value::<FsWrite>(v)
996+
.unwrap()
997+
.invoke(&ctx, &mut stdout)
998+
.await;
1026999

1027-
assert!(result.is_ok(), "Writing to ~/nested/path/file.txt should succeed");
1000+
assert!(result.is_ok(), "Writing to ~/nested/path/file.txt should succeed");
10281001

1029-
// Verify nested path content
1030-
let nested_file_path = nested_dir.join("file.txt");
1031-
let nested_content = ctx.fs().read_to_string(&nested_file_path).await.unwrap();
1032-
assert_eq!(nested_content, "content in nested path\n");
1002+
// Verify nested path content
1003+
let nested_file_path = nested_dir.join("file.txt");
1004+
let nested_content = ctx.fs.read_to_string(&nested_file_path).await.unwrap();
1005+
assert_eq!(nested_content, "content in nested path\n");
1006+
}
10331007
}

0 commit comments

Comments
 (0)