Fix tab completion with spaces (#272)

Author: certik

crates/shell/src/completion.rs

@@ -50,12 +50,44 @@ 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 {
@@ -156,17 +188,43 @@ 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)
     }
 }
 
+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<Pair>) {
     let (dir_path, partial_name) = match word.rfind('/') {
         Some(last_slash) => (&word[..=last_slash], &word[last_slash + 1..]),
@@ -177,12 +235,15 @@ fn complete_filenames(is_start: bool, word: &str, matches: &mut Vec<Pair>) {
     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<FileMatch> = 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();
 
@@ -257,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();
@@ -273,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))
@@ -300,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))
@@ -325,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))
@@ -349,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))
@@ -362,4 +433,105 @@ 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();
+        let escaped_path = path_to_escaped_string(temp_path);
+
+        // Test 1: completion of "s" should suggest both files
+        let line = format!("cat {}/s", escaped_path);
+        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", escaped_path);
+        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", escaped_path)
+        );
+
+        // Test 3: completion of "some\ fa" (escaped space) should complete to full path
+        let line = format!("cat {}/some\\ fa", escaped_path);
+        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", escaped_path)
+        );
+
+        // Test 4: completion of "some\ fx" (escaped space) should return no matches
+        let line = format!("cat {}/some\\ fx", escaped_path);
+        let pos = line.len();
+        let (_start, matches) = completer
+            .complete(&line, pos, &Context::new(&history))
+            .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();
+        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", escaped_path);
+        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/", escaped_path)
+        );
+
+        // Test 2: completion of "some\ dir/f" should suggest both files
+        let line = format!("cat {}/some\\ dir/f", escaped_path);
+        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", escaped_path);
+        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", escaped_path)
+        );
+    }
 }