Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cli/assets/completions/_usage
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ _usage() {

local spec_file="${TMPDIR:-/tmp}/usage__usage_spec_usage.spec"
usage --usage-spec >| "$spec_file"
_arguments "*: :(($(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}" )))"
local -a completions=()
while IFS= read -r line; do
completions+=("$line")
done < <(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}")
_describe 'completions' completions -S ''
return 0
}

Expand Down
27 changes: 18 additions & 9 deletions cli/src/cli/complete_word.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ impl CompleteWord {
}
}
"zsh" => {
let c = c.replace(":", "\\\\:");
if any_descriptions {
let description = description.replace("'", "'\\''");
println!("'{c}'\\:'{description}'")
let c = c.replace(':', "\\:");
let description = description.replace(':', "\\:");
println!("{c}:{description}")
} else {
println!("'{c}'")
println!("{c}")
}
}
_ => miette::bail!("unsupported shell: {}", shell),
Expand Down Expand Up @@ -335,13 +335,22 @@ impl CompleteWord {
let name = name.to_string_lossy();
!name.starts_with('.') && name.starts_with(&prefix)
})
.map(|de| de.path())
.filter(|p| filter(p))
.map(|p| {
p.strip_prefix(base)
.filter(|de| filter(&de.path()))
.map(|de| {
let p = de.path();
let is_dir = de
.file_type()
.map(|ft| ft.is_dir())
.unwrap_or_else(|_| p.is_dir());
let mut s = p
.strip_prefix(base)
.unwrap_or(&p)
.to_string_lossy()
.to_string()
.to_string();
if is_dir {
s.push('/');
}
s
})
.sorted()
.collect()
Comment on lines +338 to 356
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 de.path() called twice per entry

de.path() allocates a new PathBuf on each call. In the current chain it is called once in .filter(|de| filter(&de.path())) and again inside .map() via let p = de.path(). For large directories this doubles the allocation count unnecessarily.

Consider folding the filter into the map to call de.path() only once:

Suggested change
.filter(|de| filter(&de.path()))
.map(|de| {
let p = de.path();
let is_dir = de
.file_type()
.map(|ft| ft.is_dir())
.unwrap_or_else(|_| p.is_dir());
let mut s = p
.strip_prefix(base)
.unwrap_or(&p)
.to_string_lossy()
.to_string()
.to_string();
if is_dir {
s.push('/');
}
s
})
.sorted()
.collect()
.filter_map(|de| {
let p = de.path();
if !filter(&p) {
return None;
}
let is_dir = de
.file_type()
.map(|ft| ft.is_dir())
.unwrap_or_else(|_| p.is_dir());
let mut s = p
.strip_prefix(base)
.unwrap_or(&p)
.to_string_lossy()
.to_string();
if is_dir {
s.push('/');
}
Some(s)
})

Fix in Claude Code

Expand Down
104 changes: 103 additions & 1 deletion cli/tests/shell_completions_integration.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
use std::env;
use std::fs;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process::Command;

/// Helper to run usage complete-word and return stdout
fn run_complete_word(usage_bin: &Path, shell: &str, spec_file: &Path, words: &[&str]) -> String {
let mut args = vec![
"complete-word".to_string(),
"--shell".to_string(),
shell.to_string(),
"-f".to_string(),
spec_file.to_str().unwrap().to_string(),
"--".to_string(),
];
args.extend(words.iter().map(|w| w.to_string()));

let output = Command::new(usage_bin)
.args(&args)
.output()
.expect("Failed to run usage complete-word");

String::from_utf8_lossy(&output.stdout).to_string()
}

/// Build the usage binary and return its path
fn build_usage_binary() -> PathBuf {
let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
Expand Down Expand Up @@ -673,3 +693,85 @@ Write-Host "COMPLETION_TEST_DONE"
// Cleanup
let _ = fs::remove_dir_all(&temp_dir);
}

#[test]
fn test_zsh_complete_word_output_format() {
let usage_bin = build_usage_binary();

let temp_dir = env::temp_dir().join(format!("usage_zsh_fmt_test_{}", std::process::id()));
fs::create_dir_all(&temp_dir).unwrap();

// Spec with subcommands (which have descriptions)
let usage_spec = r#"name testcli
bin testcli
flag "-v --verbose" help="Verbose output"
arg <file> help="Input file"
cmd sub help="A subcommand"
cmd other help="Another subcommand"
"#;
let spec_file = temp_dir.join("test.spec");
fs::write(&spec_file, usage_spec).unwrap();

// Test zsh output format: should be `name:description` for _describe
let output = run_complete_word(&usage_bin, "zsh", &spec_file, &["testcli", ""]);
let lines: Vec<&str> = output.lines().collect();

// Should have completions with description format "name:description"
assert!(
lines.iter().any(|l| l.contains("sub:A subcommand")),
"Expected 'sub:A subcommand' in zsh output, got: {:?}",
lines
);
assert!(
lines.iter().any(|l| l.contains("other:Another subcommand")),
"Expected 'other:Another subcommand' in zsh output, got: {:?}",
lines
);

// Cleanup
let _ = fs::remove_dir_all(&temp_dir);
}

#[test]
fn test_complete_path_adds_trailing_slash_for_directories() {
let usage_bin = build_usage_binary();

let temp_dir = env::temp_dir().join(format!("usage_path_test_{}", std::process::id()));
fs::create_dir_all(&temp_dir).unwrap();

// Create a test directory structure
let test_dir = temp_dir.join("testdir");
fs::create_dir_all(test_dir.join("subdir")).unwrap();
fs::write(test_dir.join("file.txt"), "hello").unwrap();

// Spec with a path-type arg
let usage_spec = r#"name testcli
bin testcli
arg <path>
complete path type="path"
"#;
let spec_file = temp_dir.join("test.spec");
fs::write(&spec_file, usage_spec).unwrap();

// Complete with a path prefix pointing to our test directory
let test_dir_str = format!("{}/", test_dir.to_str().unwrap());
let output = run_complete_word(&usage_bin, "bash", &spec_file, &["testcli", &test_dir_str]);
let lines: Vec<&str> = output.lines().collect();

// Directory should have trailing slash
assert!(
lines.iter().any(|l| l.ends_with("subdir/")),
"Expected directory completion to end with '/', got: {:?}",
lines
);

// File should NOT have trailing slash
assert!(
lines.iter().any(|l| l.ends_with("file.txt")),
"Expected file completion without trailing '/', got: {:?}",
lines
);

// Cleanup
let _ = fs::remove_dir_all(&temp_dir);
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ _mycli() {
if [[ ! -f "$spec_file" ]]; then
mycli complete --usage >| "$spec_file"
fi
_arguments "*: :(($(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}" )))"
local -a completions=()
while IFS= read -r line; do
completions+=("$line")
done < <(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}")
_describe 'completions' completions -S ''
return 0
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ cmd plugin {
}
}
__USAGE_EOF__
_arguments "*: :(($(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}" )))"
local -a completions=()
while IFS= read -r line; do
completions+=("$line")
done < <(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}")
_describe 'completions' completions -S ''
return 0
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ _mycli() {

local spec_file="${TMPDIR:-/tmp}/usage__usage_spec_mycli.spec"
mycli complete --usage >| "$spec_file"
_arguments "*: :(($(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}" )))"
local -a completions=()
while IFS= read -r line; do
completions+=("$line")
done < <(command usage complete-word --shell zsh -f "$spec_file" -- "${words[@]}")
_describe 'completions' completions -S ''
return 0
}

Expand Down
6 changes: 5 additions & 1 deletion lib/src/complete/zsh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@ fi"#
r#"
local spec_file="${{TMPDIR:-/tmp}}/usage_{spec_variable}.spec"
{file_write_logic}
_arguments "*: :(($(command {usage_bin} complete-word --shell zsh -f "$spec_file" -- "${{words[@]}}" )))"
local -a completions=()
while IFS= read -r line; do
completions+=("$line")
done < <(command {usage_bin} complete-word --shell zsh -f "$spec_file" -- "${{words[@]}}")
_describe 'completions' completions -S ''
return 0
}}

Expand Down
Loading