From 5b3c7b406487b8002f23d18c99bb9a97ff831eb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C4=8Cert=C3=ADk?= Date: Sat, 8 Nov 2025 08:13:14 -0700 Subject: [PATCH 1/5] Fix completion for " " --- crates/shell/src/completion.rs | 115 +++++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 6 deletions(-) diff --git a/crates/shell/src/completion.rs b/crates/shell/src/completion.rs index 3ef502b..8b14ca1 100644 --- a/crates/shell/src/completion.rs +++ b/crates/shell/src/completion.rs @@ -50,12 +50,43 @@ impl Completer for ShellCompleter { } fn extract_word(line: &str, pos: usize) -> (usize, &str) { - if line.ends_with(' ') { - return (pos, ""); + if pos == 0 { + return (0, ""); } - let words: Vec<_> = line[..pos].split_whitespace().collect(); - let word_start = words.last().map_or(0, |w| line.rfind(w).unwrap()); - (word_start, &line[word_start..pos]) + + let bytes = line.as_bytes(); + + // Walk backwards from pos to find the start of the word + let mut i = pos; + while i > 0 { + i -= 1; + let ch = bytes[i] as char; + + // Check for word boundary characters + if ch == ' ' || ch == '|' || ch == '&' || ch == ';' || ch == '<' || ch == '>' || ch == '\t' { + // Count preceding backslashes to see if this character is escaped + let mut num_backslashes = 0; + let mut j = i; + while j > 0 { + j -= 1; + if bytes[j] == b'\\' { + num_backslashes += 1; + } else { + break; + } + } + + // If even number of backslashes (including 0), the character is NOT escaped + if num_backslashes % 2 == 0 { + // This is an unescaped word boundary + return (i + 1, &line[i + 1..pos]); + } + // Odd number of backslashes means the character is escaped, continue + } + } + + // Reached the beginning of the line + (0, &line[0..pos]) } fn escape_for_shell(s: &str) -> String { @@ -167,6 +198,22 @@ fn resolve_dir_path(dir_path: &str) -> PathBuf { } } +fn unescape_for_completion(s: &str) -> String { + let mut result = String::with_capacity(s.len()); + let mut chars = s.chars(); + while let Some(ch) = chars.next() { + if ch == '\\' { + // Skip the backslash and take the next character literally + if let Some(next_ch) = chars.next() { + result.push(next_ch); + } + } else { + result.push(ch); + } + } + result +} + fn complete_filenames(is_start: bool, word: &str, matches: &mut Vec) { let (dir_path, partial_name) = match word.rfind('/') { Some(last_slash) => (&word[..=last_slash], &word[last_slash + 1..]), @@ -177,12 +224,15 @@ fn complete_filenames(is_start: bool, word: &str, matches: &mut Vec) { let only_executable = (word.starts_with("./") || word.starts_with('/')) && is_start; let show_hidden = partial_name.starts_with('.'); + // Unescape the partial name for matching against actual filenames + let unescaped_partial = unescape_for_completion(partial_name); + let files: Vec = fs::read_dir(&search_dir) .into_iter() .flatten() .flatten() .filter_map(|entry| FileMatch::from_entry(entry, &search_dir, show_hidden)) - .filter(|f| f.name.starts_with(partial_name)) + .filter(|f| f.name.starts_with(&unescaped_partial)) .filter(|f| !only_executable || f.is_executable || f.is_dir) .collect(); @@ -362,4 +412,57 @@ mod tests { assert!(displays.contains(&".bashrc")); assert!(displays.contains(&".config/")); } + + #[tokio::test] + async fn test_complete_files_with_spaces() { + let temp_dir = TempDir::new().unwrap(); + let temp_path = temp_dir.path(); + + // Create two files with spaces in names + fs::File::create(temp_path.join("some file.txt")).unwrap(); + fs::File::create(temp_path.join("some fact.txt")).unwrap(); + + let completer = ShellCompleter::new(HashSet::new()); + let history = DefaultHistory::new(); + + // Test 1: completion of "s" should suggest both files + let line = format!("cat {}/s", temp_path.display()); + let pos = line.len(); + let (_start, matches) = completer + .complete(&line, pos, &Context::new(&history)) + .unwrap(); + assert_eq!(matches.len(), 2); + + // Test 2: completion of "some\ fi" (escaped space) should complete to full path + let line = format!("cat {}/some\\ fi", temp_path.display()); + let pos = line.len(); + let (_start, matches) = completer + .complete(&line, pos, &Context::new(&history)) + .unwrap(); + assert_eq!(matches.len(), 1); + assert_eq!( + matches[0].replacement, + format!("{}/some\\ file.txt", temp_path.display()) + ); + + // Test 3: completion of "some\ fa" (escaped space) should complete to full path + let line = format!("cat {}/some\\ fa", temp_path.display()); + let pos = line.len(); + let (_start, matches) = completer + .complete(&line, pos, &Context::new(&history)) + .unwrap(); + assert_eq!(matches.len(), 1); + assert_eq!( + matches[0].replacement, + format!("{}/some\\ fact.txt", temp_path.display()) + ); + + // Test 4: completion of "some\ fx" (escaped space) should return no matches + let line = format!("cat {}/some\\ fx", temp_path.display()); + let pos = line.len(); + let (_start, matches) = completer + .complete(&line, pos, &Context::new(&history)) + .unwrap(); + assert_eq!(matches.len(), 0); + } } From d39eae30bf51c033c041ad28b89ad3a8d11f3aca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C4=8Cert=C3=ADk?= Date: Sat, 8 Nov 2025 08:15:47 -0700 Subject: [PATCH 2/5] Handle subdirectories also --- crates/shell/src/completion.rs | 58 +++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/crates/shell/src/completion.rs b/crates/shell/src/completion.rs index 8b14ca1..3f0ff1f 100644 --- a/crates/shell/src/completion.rs +++ b/crates/shell/src/completion.rs @@ -187,14 +187,18 @@ fn is_executable(entry: &fs::DirEntry) -> bool { } fn resolve_dir_path(dir_path: &str) -> PathBuf { + // Unescape the directory path to handle spaces and other special characters + let unescaped = unescape_for_completion(dir_path); + if dir_path.starts_with('/') { - PathBuf::from(dir_path) + PathBuf::from(unescaped) } else if let Some(stripped) = dir_path.strip_prefix('~') { + let unescaped_stripped = unescape_for_completion(stripped); dirs::home_dir() - .map(|h| h.join(stripped.strip_prefix('/').unwrap_or(stripped))) - .unwrap_or_else(|| PathBuf::from(dir_path)) + .map(|h| h.join(unescaped_stripped.strip_prefix('/').unwrap_or(&unescaped_stripped))) + .unwrap_or_else(|| PathBuf::from(unescaped)) } else { - PathBuf::from(".").join(dir_path) + PathBuf::from(".").join(unescaped) } } @@ -465,4 +469,50 @@ mod tests { .unwrap(); assert_eq!(matches.len(), 0); } + + #[tokio::test] + async fn test_complete_files_in_directory_with_spaces() { + let temp_dir = TempDir::new().unwrap(); + let temp_path = temp_dir.path(); + + // Create a directory with a space in its name + fs::create_dir(temp_path.join("some dir")).unwrap(); + fs::File::create(temp_path.join("some dir/file1.txt")).unwrap(); + fs::File::create(temp_path.join("some dir/file2.txt")).unwrap(); + + let completer = ShellCompleter::new(HashSet::new()); + let history = DefaultHistory::new(); + + // Test 1: completion of "some\ d" should suggest the directory + let line = format!("cd {}/some\\ d", temp_path.display()); + let pos = line.len(); + let (_start, matches) = completer + .complete(&line, pos, &Context::new(&history)) + .unwrap(); + assert_eq!(matches.len(), 1); + assert_eq!( + matches[0].replacement, + format!("{}/some\\ dir/", temp_path.display()) + ); + + // Test 2: completion of "some\ dir/f" should suggest both files + let line = format!("cat {}/some\\ dir/f", temp_path.display()); + let pos = line.len(); + let (_start, matches) = completer + .complete(&line, pos, &Context::new(&history)) + .unwrap(); + assert_eq!(matches.len(), 2); + + // Test 3: completion of "some\ dir/file1" should complete to file1.txt + let line = format!("cat {}/some\\ dir/file1", temp_path.display()); + let pos = line.len(); + let (_start, matches) = completer + .complete(&line, pos, &Context::new(&history)) + .unwrap(); + assert_eq!(matches.len(), 1); + assert_eq!( + matches[0].replacement, + format!("{}/some\\ dir/file1.txt", temp_path.display()) + ); + } } From 26ef819bd190e8ffaa6ff115d82d65b3e0001800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C4=8Cert=C3=ADk?= Date: Sat, 8 Nov 2025 08:16:38 -0700 Subject: [PATCH 3/5] Fix fmt --- crates/shell/src/completion.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/shell/src/completion.rs b/crates/shell/src/completion.rs index 3f0ff1f..7816a56 100644 --- a/crates/shell/src/completion.rs +++ b/crates/shell/src/completion.rs @@ -63,7 +63,8 @@ fn extract_word(line: &str, pos: usize) -> (usize, &str) { let ch = bytes[i] as char; // Check for word boundary characters - if ch == ' ' || ch == '|' || ch == '&' || ch == ';' || ch == '<' || ch == '>' || ch == '\t' { + if ch == ' ' || ch == '|' || ch == '&' || ch == ';' || ch == '<' || ch == '>' || ch == '\t' + { // Count preceding backslashes to see if this character is escaped let mut num_backslashes = 0; let mut j = i; @@ -195,7 +196,13 @@ fn resolve_dir_path(dir_path: &str) -> PathBuf { } else if let Some(stripped) = dir_path.strip_prefix('~') { let unescaped_stripped = unescape_for_completion(stripped); dirs::home_dir() - .map(|h| h.join(unescaped_stripped.strip_prefix('/').unwrap_or(&unescaped_stripped))) + .map(|h| { + h.join( + unescaped_stripped + .strip_prefix('/') + .unwrap_or(&unescaped_stripped), + ) + }) .unwrap_or_else(|| PathBuf::from(unescaped)) } else { PathBuf::from(".").join(unescaped) From 7eac7942b1468478cb81a7f79e3c3ef7214f583d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C4=8Cert=C3=ADk?= Date: Sat, 8 Nov 2025 08:22:03 -0700 Subject: [PATCH 4/5] Windows fix --- crates/shell/src/completion.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/shell/src/completion.rs b/crates/shell/src/completion.rs index 7816a56..396979d 100644 --- a/crates/shell/src/completion.rs +++ b/crates/shell/src/completion.rs @@ -188,13 +188,31 @@ fn is_executable(entry: &fs::DirEntry) -> bool { } fn resolve_dir_path(dir_path: &str) -> PathBuf { - // Unescape the directory path to handle spaces and other special characters - let unescaped = unescape_for_completion(dir_path); + // On Windows, we need to distinguish between: + // 1. System paths like "C:\Users\..." where backslashes are path separators + // 2. User-typed paths like "some\ dir/" where backslashes are escape characters + // + // Windows absolute paths start with a drive letter (C:\) or UNC path (\\server\share) + let is_windows_absolute = cfg!(windows) + && (dir_path.len() >= 3 && dir_path.chars().nth(1) == Some(':') || // C:\... + dir_path.starts_with("\\\\")); // UNC path + + // Only unescape user-typed relative paths to handle escaped spaces + // Don't unescape Windows absolute paths - their backslashes are path separators + let unescaped = if is_windows_absolute { + dir_path.to_string() + } else { + unescape_for_completion(dir_path) + }; - if dir_path.starts_with('/') { + if dir_path.starts_with('/') || is_windows_absolute { PathBuf::from(unescaped) } else if let Some(stripped) = dir_path.strip_prefix('~') { - let unescaped_stripped = unescape_for_completion(stripped); + let unescaped_stripped = if is_windows_absolute { + stripped.to_string() + } else { + unescape_for_completion(stripped) + }; dirs::home_dir() .map(|h| { h.join( From 9ab0921f51a81c86de8288521eacde2346a1a8fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C4=8Cert=C3=ADk?= Date: Sat, 8 Nov 2025 08:26:09 -0700 Subject: [PATCH 5/5] Better fix --- crates/shell/src/completion.rs | 68 ++++++++++++++++------------------ 1 file changed, 31 insertions(+), 37 deletions(-) diff --git a/crates/shell/src/completion.rs b/crates/shell/src/completion.rs index 396979d..780eb9e 100644 --- a/crates/shell/src/completion.rs +++ b/crates/shell/src/completion.rs @@ -188,31 +188,13 @@ fn is_executable(entry: &fs::DirEntry) -> bool { } fn resolve_dir_path(dir_path: &str) -> PathBuf { - // On Windows, we need to distinguish between: - // 1. System paths like "C:\Users\..." where backslashes are path separators - // 2. User-typed paths like "some\ dir/" where backslashes are escape characters - // - // Windows absolute paths start with a drive letter (C:\) or UNC path (\\server\share) - let is_windows_absolute = cfg!(windows) - && (dir_path.len() >= 3 && dir_path.chars().nth(1) == Some(':') || // C:\... - dir_path.starts_with("\\\\")); // UNC path - - // Only unescape user-typed relative paths to handle escaped spaces - // Don't unescape Windows absolute paths - their backslashes are path separators - let unescaped = if is_windows_absolute { - dir_path.to_string() - } else { - unescape_for_completion(dir_path) - }; + // Unescape the directory path to handle spaces and other special characters + let unescaped = unescape_for_completion(dir_path); - if dir_path.starts_with('/') || is_windows_absolute { + if dir_path.starts_with('/') { PathBuf::from(unescaped) } else if let Some(stripped) = dir_path.strip_prefix('~') { - let unescaped_stripped = if is_windows_absolute { - stripped.to_string() - } else { - unescape_for_completion(stripped) - }; + let unescaped_stripped = unescape_for_completion(stripped); dirs::home_dir() .map(|h| { h.join( @@ -336,8 +318,14 @@ mod tests { use super::*; use rustyline::history::DefaultHistory; use std::fs; + use std::path::Path; use tempfile::TempDir; + // Helper function to convert a path to a shell-escaped string + fn path_to_escaped_string(path: &Path) -> String { + escape_for_shell(&path.display().to_string()) + } + #[tokio::test] async fn test_complete_hidden_files_when_starting_with_dot() { let temp_dir = TempDir::new().unwrap(); @@ -352,7 +340,8 @@ mod tests { // Test completion with "." prefix let completer = ShellCompleter::new(HashSet::new()); let history = DefaultHistory::new(); - let line = format!("cat {}/.gi", temp_path.display()); + let escaped_path = path_to_escaped_string(temp_path); + let line = format!("cat {}/.gi", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -379,7 +368,8 @@ mod tests { // Test completion without "." prefix let completer = ShellCompleter::new(HashSet::new()); let history = DefaultHistory::new(); - let line = format!("cat {}/", temp_path.display()); + let escaped_path = path_to_escaped_string(temp_path); + let line = format!("cat {}/", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -404,7 +394,8 @@ mod tests { // Test completion with ".gith" prefix let completer = ShellCompleter::new(HashSet::new()); let history = DefaultHistory::new(); - let line = format!("cd {}/.gith", temp_path.display()); + let escaped_path = path_to_escaped_string(temp_path); + let line = format!("cd {}/.gith", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -428,7 +419,8 @@ mod tests { // Test completion with just "." prefix let completer = ShellCompleter::new(HashSet::new()); let history = DefaultHistory::new(); - let line = format!("ls {}/.", temp_path.display()); + let escaped_path = path_to_escaped_string(temp_path); + let line = format!("ls {}/.", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -453,9 +445,10 @@ mod tests { let completer = ShellCompleter::new(HashSet::new()); let history = DefaultHistory::new(); + let escaped_path = path_to_escaped_string(temp_path); // Test 1: completion of "s" should suggest both files - let line = format!("cat {}/s", temp_path.display()); + let line = format!("cat {}/s", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -463,7 +456,7 @@ mod tests { assert_eq!(matches.len(), 2); // Test 2: completion of "some\ fi" (escaped space) should complete to full path - let line = format!("cat {}/some\\ fi", temp_path.display()); + let line = format!("cat {}/some\\ fi", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -471,11 +464,11 @@ mod tests { assert_eq!(matches.len(), 1); assert_eq!( matches[0].replacement, - format!("{}/some\\ file.txt", temp_path.display()) + format!("{}/some\\ file.txt", escaped_path) ); // Test 3: completion of "some\ fa" (escaped space) should complete to full path - let line = format!("cat {}/some\\ fa", temp_path.display()); + let line = format!("cat {}/some\\ fa", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -483,11 +476,11 @@ mod tests { assert_eq!(matches.len(), 1); assert_eq!( matches[0].replacement, - format!("{}/some\\ fact.txt", temp_path.display()) + format!("{}/some\\ fact.txt", escaped_path) ); // Test 4: completion of "some\ fx" (escaped space) should return no matches - let line = format!("cat {}/some\\ fx", temp_path.display()); + let line = format!("cat {}/some\\ fx", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -507,9 +500,10 @@ mod tests { let completer = ShellCompleter::new(HashSet::new()); let history = DefaultHistory::new(); + let escaped_path = path_to_escaped_string(temp_path); // Test 1: completion of "some\ d" should suggest the directory - let line = format!("cd {}/some\\ d", temp_path.display()); + let line = format!("cd {}/some\\ d", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -517,11 +511,11 @@ mod tests { assert_eq!(matches.len(), 1); assert_eq!( matches[0].replacement, - format!("{}/some\\ dir/", temp_path.display()) + format!("{}/some\\ dir/", escaped_path) ); // Test 2: completion of "some\ dir/f" should suggest both files - let line = format!("cat {}/some\\ dir/f", temp_path.display()); + let line = format!("cat {}/some\\ dir/f", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -529,7 +523,7 @@ mod tests { assert_eq!(matches.len(), 2); // Test 3: completion of "some\ dir/file1" should complete to file1.txt - let line = format!("cat {}/some\\ dir/file1", temp_path.display()); + let line = format!("cat {}/some\\ dir/file1", escaped_path); let pos = line.len(); let (_start, matches) = completer .complete(&line, pos, &Context::new(&history)) @@ -537,7 +531,7 @@ mod tests { assert_eq!(matches.len(), 1); assert_eq!( matches[0].replacement, - format!("{}/some\\ dir/file1.txt", temp_path.display()) + format!("{}/some\\ dir/file1.txt", escaped_path) ); } }