From 2f07f3cc61dcbc9a9d81f92abb2aa7e27cc3dca5 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 11:27:55 -0500 Subject: [PATCH 01/14] refactor(complete): Remove manual path splitting --- clap_complete/src/engine/complete.rs | 29 +++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index 0c90b45c2a3..238ef525d39 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -355,12 +355,10 @@ fn complete_path( return Vec::new(); } }; - let (existing, prefix) = value_os - .split_once("\\") - .unwrap_or((OsStr::new(""), value_os)); - let root = current_dir.join(existing); - debug!("complete_path: root={root:?}, prefix={prefix:?}"); + let absolute = current_dir.join(value_os); + let (root, prefix) = split_file_name(&absolute); let prefix = prefix.to_string_lossy(); + debug!("complete_path: root={root:?}, prefix={prefix:?}"); for entry in std::fs::read_dir(&root) .ok() @@ -392,6 +390,27 @@ fn complete_path( completions } +fn split_file_name(path: &std::path::Path) -> (&std::path::Path, &OsStr) { + // Workaround that `Path::new("name/").file_name()` reports `"name"` + if path_has_name(path) { + ( + path.parent().unwrap_or_else(|| std::path::Path::new("")), + path.file_name().expect("not called with `..`"), + ) + } else { + (path, Default::default()) + } +} + +fn path_has_name(path: &std::path::Path) -> bool { + let path = path.as_os_str().as_encoded_bytes(); + let Some(trailing) = path.last() else { + return false; + }; + let trailing = *trailing as char; + !std::path::is_separator(trailing) +} + fn complete_custom_arg_value( value: &OsStr, completer: &ArgValueCandidates, From ebb1302e9058c14c03aea1919cdab8be6f05f6c0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 11:29:08 -0500 Subject: [PATCH 02/14] refactor(complete): Remove no-op help calls --- clap_complete/src/engine/complete.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index 238ef525d39..edeac7886a4 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -375,14 +375,12 @@ fn complete_path( let path = entry.path(); let mut suggestion = pathdiff::diff_paths(&path, current_dir).unwrap_or(path); suggestion.push(""); // Ensure trailing `/` - completions - .push(CompletionCandidate::new(suggestion.as_os_str().to_owned()).help(None)); + completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); } else { let path = entry.path(); if is_wanted(&path) { let suggestion = pathdiff::diff_paths(&path, current_dir).unwrap_or(path); - completions - .push(CompletionCandidate::new(suggestion.as_os_str().to_owned()).help(None)); + completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); } } } From 78157e6ce630eda00e4e2fcb3e8ed9b8cf59b890 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 11:58:11 -0500 Subject: [PATCH 03/14] fix(complete): Preserve absolute paths --- Cargo.lock | 1 - clap_complete/Cargo.toml | 5 ++-- clap_complete/src/engine/complete.rs | 40 +++++++++++++++------------- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 50ddf8d0396..14e3b9d8a96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -482,7 +482,6 @@ dependencies = [ "completest", "completest-pty", "is_executable", - "pathdiff", "shlex", "snapbox", "trycmd", diff --git a/clap_complete/Cargo.toml b/clap_complete/Cargo.toml index cef7ec1dbc7..091a431ceeb 100644 --- a/clap_complete/Cargo.toml +++ b/clap_complete/Cargo.toml @@ -37,7 +37,6 @@ bench = false clap = { path = "../", version = "4.5.15", default-features = false, features = ["std"] } clap_lex = { path = "../clap_lex", version = "0.7.0", optional = true } is_executable = { version = "1.0.1", optional = true } -pathdiff = { version = "0.2.1", optional = true } shlex = { version = "1.1.0", optional = true } unicode-xid = { version = "0.2.2", optional = true } @@ -57,8 +56,8 @@ required-features = ["unstable-dynamic", "unstable-command"] [features] default = [] unstable-doc = ["unstable-dynamic", "unstable-command"] # for docs.rs -unstable-dynamic = ["dep:clap_lex", "dep:shlex", "dep:is_executable", "dep:pathdiff", "clap/unstable-ext"] -unstable-command = ["unstable-dynamic", "dep:unicode-xid", "clap/derive", "dep:is_executable", "dep:pathdiff", "clap/unstable-ext"] +unstable-dynamic = ["dep:clap_lex", "dep:shlex", "dep:is_executable", "clap/unstable-ext"] +unstable-command = ["unstable-dynamic", "dep:unicode-xid", "clap/derive", "dep:is_executable", "clap/unstable-ext"] debug = ["clap/debug"] [lints] diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index edeac7886a4..c35dc5c7b39 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -348,38 +348,42 @@ fn complete_path( ) -> Vec { let mut completions = Vec::new(); - let current_dir = match current_dir { - Some(current_dir) => current_dir, - None => { - // Can't complete without a `current_dir` - return Vec::new(); - } + let value_path = std::path::Path::new(value_os); + let (prefix, current) = split_file_name(value_path); + let current = current.to_string_lossy(); + let search_root = if prefix.is_absolute() { + prefix.to_owned() + } else { + let current_dir = match current_dir { + Some(current_dir) => current_dir, + None => { + // Can't complete without a `current_dir` + return Vec::new(); + } + }; + current_dir.join(prefix) }; - let absolute = current_dir.join(value_os); - let (root, prefix) = split_file_name(&absolute); - let prefix = prefix.to_string_lossy(); - debug!("complete_path: root={root:?}, prefix={prefix:?}"); + debug!("complete_path: search_root={search_root:?}, prefix={prefix:?}"); - for entry in std::fs::read_dir(&root) + for entry in std::fs::read_dir(&search_root) .ok() .into_iter() .flatten() .filter_map(Result::ok) { let raw_file_name = entry.file_name(); - if !raw_file_name.starts_with(&prefix) { + if !raw_file_name.starts_with(¤t) { continue; } if entry.metadata().map(|m| m.is_dir()).unwrap_or(false) { - let path = entry.path(); - let mut suggestion = pathdiff::diff_paths(&path, current_dir).unwrap_or(path); + let mut suggestion = prefix.join(raw_file_name); suggestion.push(""); // Ensure trailing `/` completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); } else { let path = entry.path(); if is_wanted(&path) { - let suggestion = pathdiff::diff_paths(&path, current_dir).unwrap_or(path); + let suggestion = prefix.join(raw_file_name); completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); } } @@ -401,12 +405,12 @@ fn split_file_name(path: &std::path::Path) -> (&std::path::Path, &OsStr) { } fn path_has_name(path: &std::path::Path) -> bool { - let path = path.as_os_str().as_encoded_bytes(); - let Some(trailing) = path.last() else { + let path_bytes = path.as_os_str().as_encoded_bytes(); + let Some(trailing) = path_bytes.last() else { return false; }; let trailing = *trailing as char; - !std::path::is_separator(trailing) + !std::path::is_separator(trailing) && path.file_name().is_some() } fn complete_custom_arg_value( From 218ee5d9dc7df055d863cd95b66f806f1a4d4fd1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 13:19:39 -0500 Subject: [PATCH 04/14] refactor(complete): Move complete_path to long term location I'd like to remove `ValueHint` eventually --- clap_complete/src/engine/complete.rs | 73 +-------------------------- clap_complete/src/engine/custom.rs | 74 ++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 72 deletions(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index c35dc5c7b39..ee93acef229 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -3,6 +3,7 @@ use std::ffi::OsString; use clap_lex::OsStrExt as _; +use super::custom::complete_path; use super::ArgValueCandidates; use super::CompletionCandidate; @@ -341,78 +342,6 @@ fn rsplit_delimiter<'s, 'o>( Some((Some(prefix), Ok(value))) } -fn complete_path( - value_os: &OsStr, - current_dir: Option<&std::path::Path>, - is_wanted: impl Fn(&std::path::Path) -> bool, -) -> Vec { - let mut completions = Vec::new(); - - let value_path = std::path::Path::new(value_os); - let (prefix, current) = split_file_name(value_path); - let current = current.to_string_lossy(); - let search_root = if prefix.is_absolute() { - prefix.to_owned() - } else { - let current_dir = match current_dir { - Some(current_dir) => current_dir, - None => { - // Can't complete without a `current_dir` - return Vec::new(); - } - }; - current_dir.join(prefix) - }; - debug!("complete_path: search_root={search_root:?}, prefix={prefix:?}"); - - for entry in std::fs::read_dir(&search_root) - .ok() - .into_iter() - .flatten() - .filter_map(Result::ok) - { - let raw_file_name = entry.file_name(); - if !raw_file_name.starts_with(¤t) { - continue; - } - - if entry.metadata().map(|m| m.is_dir()).unwrap_or(false) { - let mut suggestion = prefix.join(raw_file_name); - suggestion.push(""); // Ensure trailing `/` - completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); - } else { - let path = entry.path(); - if is_wanted(&path) { - let suggestion = prefix.join(raw_file_name); - completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); - } - } - } - - completions -} - -fn split_file_name(path: &std::path::Path) -> (&std::path::Path, &OsStr) { - // Workaround that `Path::new("name/").file_name()` reports `"name"` - if path_has_name(path) { - ( - path.parent().unwrap_or_else(|| std::path::Path::new("")), - path.file_name().expect("not called with `..`"), - ) - } else { - (path, Default::default()) - } -} - -fn path_has_name(path: &std::path::Path) -> bool { - let path_bytes = path.as_os_str().as_encoded_bytes(); - let Some(trailing) = path_bytes.last() else { - return false; - }; - let trailing = *trailing as char; - !std::path::is_separator(trailing) && path.file_name().is_some() -} - fn complete_custom_arg_value( value: &OsStr, completer: &ArgValueCandidates, diff --git a/clap_complete/src/engine/custom.rs b/clap_complete/src/engine/custom.rs index b01de7c72f9..249e40e380b 100644 --- a/clap_complete/src/engine/custom.rs +++ b/clap_complete/src/engine/custom.rs @@ -1,7 +1,9 @@ use std::any::type_name; +use std::ffi::OsStr; use std::sync::Arc; use clap::builder::ArgExt; +use clap_lex::OsStrExt as _; use super::CompletionCandidate; @@ -68,3 +70,75 @@ where self() } } + +pub(crate) fn complete_path( + value_os: &OsStr, + current_dir: Option<&std::path::Path>, + is_wanted: impl Fn(&std::path::Path) -> bool, +) -> Vec { + let mut completions = Vec::new(); + + let value_path = std::path::Path::new(value_os); + let (prefix, current) = split_file_name(value_path); + let current = current.to_string_lossy(); + let search_root = if prefix.is_absolute() { + prefix.to_owned() + } else { + let current_dir = match current_dir { + Some(current_dir) => current_dir, + None => { + // Can't complete without a `current_dir` + return Vec::new(); + } + }; + current_dir.join(prefix) + }; + debug!("complete_path: search_root={search_root:?}, prefix={prefix:?}"); + + for entry in std::fs::read_dir(&search_root) + .ok() + .into_iter() + .flatten() + .filter_map(Result::ok) + { + let raw_file_name = entry.file_name(); + if !raw_file_name.starts_with(¤t) { + continue; + } + + if entry.metadata().map(|m| m.is_dir()).unwrap_or(false) { + let mut suggestion = prefix.join(raw_file_name); + suggestion.push(""); // Ensure trailing `/` + completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); + } else { + let path = entry.path(); + if is_wanted(&path) { + let suggestion = prefix.join(raw_file_name); + completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); + } + } + } + + completions +} + +fn split_file_name(path: &std::path::Path) -> (&std::path::Path, &OsStr) { + // Workaround that `Path::new("name/").file_name()` reports `"name"` + if path_has_name(path) { + ( + path.parent().unwrap_or_else(|| std::path::Path::new("")), + path.file_name().expect("not called with `..`"), + ) + } else { + (path, Default::default()) + } +} + +fn path_has_name(path: &std::path::Path) -> bool { + let path_bytes = path.as_os_str().as_encoded_bytes(); + let Some(trailing) = path_bytes.last() else { + return false; + }; + let trailing = *trailing as char; + !std::path::is_separator(trailing) && path.file_name().is_some() +} From d7b8aafdc3d429506efba67e8743b5c53638e135 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 13:32:47 -0500 Subject: [PATCH 05/14] refactor(complete): Dont duplicate complete_path --- clap_complete/src/engine/complete.rs | 10 +++++----- clap_complete/src/engine/custom.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index ee93acef229..b2cdfa30e3c 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -290,17 +290,17 @@ fn complete_arg_value( // Should not complete } clap::ValueHint::Unknown | clap::ValueHint::AnyPath => { - values.extend(complete_path(value_os, current_dir, |_| true)); + values.extend(complete_path(value_os, current_dir, &|_| true)); } clap::ValueHint::FilePath => { - values.extend(complete_path(value_os, current_dir, |p| p.is_file())); + values.extend(complete_path(value_os, current_dir, &|p| p.is_file())); } clap::ValueHint::DirPath => { - values.extend(complete_path(value_os, current_dir, |p| p.is_dir())); + values.extend(complete_path(value_os, current_dir, &|p| p.is_dir())); } clap::ValueHint::ExecutablePath => { use is_executable::IsExecutable; - values.extend(complete_path(value_os, current_dir, |p| p.is_executable())); + values.extend(complete_path(value_os, current_dir, &|p| p.is_executable())); } clap::ValueHint::CommandName | clap::ValueHint::CommandString @@ -313,7 +313,7 @@ fn complete_arg_value( } _ => { // Safe-ish fallback - values.extend(complete_path(value_os, current_dir, |_| true)); + values.extend(complete_path(value_os, current_dir, &|_| true)); } } diff --git a/clap_complete/src/engine/custom.rs b/clap_complete/src/engine/custom.rs index 249e40e380b..1ce089fcb74 100644 --- a/clap_complete/src/engine/custom.rs +++ b/clap_complete/src/engine/custom.rs @@ -74,7 +74,7 @@ where pub(crate) fn complete_path( value_os: &OsStr, current_dir: Option<&std::path::Path>, - is_wanted: impl Fn(&std::path::Path) -> bool, + is_wanted: &dyn Fn(&std::path::Path) -> bool, ) -> Vec { let mut completions = Vec::new(); From 5f1eb968b4d2b772f18cece0f29eddca3d36c60c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 13:45:01 -0500 Subject: [PATCH 06/14] test(complete): Split ValueHint testing from value completion --- clap_complete/tests/testsuite/engine.rs | 122 ++++++++++-------------- 1 file changed, 53 insertions(+), 69 deletions(-) diff --git a/clap_complete/tests/testsuite/engine.rs b/clap_complete/tests/testsuite/engine.rs index 0fea4fb6a23..425e0af3cbe 100644 --- a/clap_complete/tests/testsuite/engine.rs +++ b/clap_complete/tests/testsuite/engine.rs @@ -293,18 +293,18 @@ goodbye-world #[test] fn suggest_argument_value() { let mut cmd = Command::new("dynamic") - .arg( - clap::Arg::new("input") - .long("input") - .short('i') - .value_hint(clap::ValueHint::FilePath), - ) .arg( clap::Arg::new("format") .long("format") .short('F') .value_parser(["json", "yaml", "toml"]), ) + .arg( + clap::Arg::new("stream") + .long("stream") + .short('S') + .value_parser(["stdout", "stderr"]), + ) .arg( clap::Arg::new("count") .long("count") @@ -314,44 +314,6 @@ fn suggest_argument_value() { .arg(clap::Arg::new("positional").value_parser(["pos_a", "pos_b", "pos_c"])) .args_conflicts_with_subcommands(true); - let testdir = snapbox::dir::DirRoot::mutable_temp().unwrap(); - let testdir_path = testdir.path().unwrap(); - - fs::write(testdir_path.join("a_file"), "").unwrap(); - fs::write(testdir_path.join("b_file"), "").unwrap(); - fs::create_dir_all(testdir_path.join("c_dir")).unwrap(); - fs::create_dir_all(testdir_path.join("d_dir")).unwrap(); - - assert_data_eq!( - complete!(cmd, "--input [TAB]", current_dir = Some(testdir_path)), - snapbox::str![[r#" -a_file -b_file -c_dir/ -d_dir/ -"#]], - ); - - assert_data_eq!( - complete!(cmd, "-i [TAB]", current_dir = Some(testdir_path)), - snapbox::str![[r#" -a_file -b_file -c_dir/ -d_dir/ -"#]], - ); - - assert_data_eq!( - complete!(cmd, "--input a[TAB]", current_dir = Some(testdir_path)), - snapbox::str!["a_file"], - ); - - assert_data_eq!( - complete!(cmd, "-i b[TAB]", current_dir = Some(testdir_path)), - snapbox::str!["b_file"], - ); - assert_data_eq!( complete!(cmd, "--format [TAB]"), snapbox::str![[r#" @@ -388,14 +350,14 @@ toml ); assert_data_eq!( - complete!(cmd, "--input a_file [TAB]"), + complete!(cmd, "--format toml [TAB]"), snapbox::str![[r#" ---input --format +--stream --count --help Print help --i -F +-S -c -h Print help pos_a @@ -405,39 +367,26 @@ pos_c ); assert_data_eq!( - complete!(cmd, "-ci[TAB]", current_dir = Some(testdir_path)), + complete!(cmd, "-cS[TAB]"), snapbox::str![[r#" --cia_file --cib_file --cic_dir/ --cid_dir/ +-cSstdout +-cSstderr "#]] ); assert_data_eq!( - complete!(cmd, "-ci=[TAB]", current_dir = Some(testdir_path)), + complete!(cmd, "-cS=[TAB]"), snapbox::str![[r#" --ci=a_file --ci=b_file --ci=c_dir/ --ci=d_dir/ +-cS=stdout +-cS=stderr "#]] ); - assert_data_eq!( - complete!(cmd, "-ci=a[TAB]", current_dir = Some(testdir_path)), - snapbox::str!["-ci=a_file"] - ); + assert_data_eq!(complete!(cmd, "-cS=stdo[TAB]"), snapbox::str!["-cS=stdout"]); - assert_data_eq!( - complete!(cmd, "-ciF[TAB]", current_dir = Some(testdir_path)), - snapbox::str![] - ); + assert_data_eq!(complete!(cmd, "-cSF[TAB]"), snapbox::str![]); - assert_data_eq!( - complete!(cmd, "-ciF=[TAB]", current_dir = Some(testdir_path)), - snapbox::str![] - ); + assert_data_eq!(complete!(cmd, "-cSF=[TAB]"), snapbox::str![]); } #[test] @@ -591,6 +540,41 @@ val3 ); } +#[test] +fn suggest_value_hint_file_path() { + let mut cmd = Command::new("dynamic") + .arg( + clap::Arg::new("input") + .long("input") + .short('i') + .value_hint(clap::ValueHint::FilePath), + ) + .args_conflicts_with_subcommands(true); + + let testdir = snapbox::dir::DirRoot::mutable_temp().unwrap(); + let testdir_path = testdir.path().unwrap(); + + fs::write(testdir_path.join("a_file"), "").unwrap(); + fs::write(testdir_path.join("b_file"), "").unwrap(); + fs::create_dir_all(testdir_path.join("c_dir")).unwrap(); + fs::create_dir_all(testdir_path.join("d_dir")).unwrap(); + + assert_data_eq!( + complete!(cmd, "--input [TAB]", current_dir = Some(testdir_path)), + snapbox::str![[r#" +a_file +b_file +c_dir/ +d_dir/ +"#]], + ); + + assert_data_eq!( + complete!(cmd, "--input a[TAB]", current_dir = Some(testdir_path)), + snapbox::str!["a_file"], + ); +} + #[test] fn suggest_custom_arg_value() { #[derive(Debug)] From 9bef07708d24f17c6f2b4c16ea46fd2a9a9e8a7e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 13:51:25 -0500 Subject: [PATCH 07/14] test(complete): Simplify ArgValueCandidates test --- clap_complete/tests/testsuite/engine.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/clap_complete/tests/testsuite/engine.rs b/clap_complete/tests/testsuite/engine.rs index 425e0af3cbe..5fc363b0c62 100644 --- a/clap_complete/tests/testsuite/engine.rs +++ b/clap_complete/tests/testsuite/engine.rs @@ -4,7 +4,7 @@ use std::fs; use std::path::Path; use clap::{builder::PossibleValue, Command}; -use clap_complete::engine::{ArgValueCandidates, CompletionCandidate, ValueCandidates}; +use clap_complete::engine::{ArgValueCandidates, CompletionCandidate}; use snapbox::assert_data_eq; macro_rules! complete { @@ -577,23 +577,18 @@ d_dir/ #[test] fn suggest_custom_arg_value() { - #[derive(Debug)] - struct MyCustomCompleter {} - - impl ValueCandidates for MyCustomCompleter { - fn candidates(&self) -> Vec { - vec![ - CompletionCandidate::new("custom1"), - CompletionCandidate::new("custom2"), - CompletionCandidate::new("custom3"), - ] - } + fn custom_completer() -> Vec { + vec![ + CompletionCandidate::new("custom1"), + CompletionCandidate::new("custom2"), + CompletionCandidate::new("custom3"), + ] } let mut cmd = Command::new("dynamic").arg( clap::Arg::new("custom") .long("custom") - .add::(ArgValueCandidates::new(MyCustomCompleter {})), + .add(ArgValueCandidates::new(custom_completer)), ); assert_data_eq!( From 431e2bc931b6721471fad18866f52fefe7d6572c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 13:52:13 -0500 Subject: [PATCH 08/14] test(complete): Ensure ArgValueCandidates get filtered --- clap_complete/tests/testsuite/engine.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/clap_complete/tests/testsuite/engine.rs b/clap_complete/tests/testsuite/engine.rs index 5fc363b0c62..ed70bd0931c 100644 --- a/clap_complete/tests/testsuite/engine.rs +++ b/clap_complete/tests/testsuite/engine.rs @@ -579,9 +579,9 @@ d_dir/ fn suggest_custom_arg_value() { fn custom_completer() -> Vec { vec![ - CompletionCandidate::new("custom1"), - CompletionCandidate::new("custom2"), - CompletionCandidate::new("custom3"), + CompletionCandidate::new("foo"), + CompletionCandidate::new("bar"), + CompletionCandidate::new("baz"), ] } @@ -594,9 +594,17 @@ fn suggest_custom_arg_value() { assert_data_eq!( complete!(cmd, "--custom [TAB]"), snapbox::str![[r#" -custom1 -custom2 -custom3 +foo +bar +baz +"#]], + ); + + assert_data_eq!( + complete!(cmd, "--custom b[TAB]"), + snapbox::str![[r#" +bar +baz "#]], ); } From 47aedc6906c0c35a89833331041a083630d777fb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 14:11:48 -0500 Subject: [PATCH 09/14] fix(complete): Ensure paths are sorted --- clap_complete/src/engine/custom.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/clap_complete/src/engine/custom.rs b/clap_complete/src/engine/custom.rs index 1ce089fcb74..240972cfb54 100644 --- a/clap_complete/src/engine/custom.rs +++ b/clap_complete/src/engine/custom.rs @@ -77,6 +77,7 @@ pub(crate) fn complete_path( is_wanted: &dyn Fn(&std::path::Path) -> bool, ) -> Vec { let mut completions = Vec::new(); + let mut potential = Vec::new(); let value_path = std::path::Path::new(value_os); let (prefix, current) = split_file_name(value_path); @@ -88,7 +89,7 @@ pub(crate) fn complete_path( Some(current_dir) => current_dir, None => { // Can't complete without a `current_dir` - return Vec::new(); + return completions; } }; current_dir.join(prefix) @@ -109,15 +110,24 @@ pub(crate) fn complete_path( if entry.metadata().map(|m| m.is_dir()).unwrap_or(false) { let mut suggestion = prefix.join(raw_file_name); suggestion.push(""); // Ensure trailing `/` - completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); + let candidate = CompletionCandidate::new(suggestion.as_os_str().to_owned()); + + if is_wanted(&entry.path()) { + completions.push(candidate); + } else { + potential.push(candidate); + } } else { - let path = entry.path(); - if is_wanted(&path) { + if is_wanted(&entry.path()) { let suggestion = prefix.join(raw_file_name); - completions.push(CompletionCandidate::new(suggestion.as_os_str().to_owned())); + let candidate = CompletionCandidate::new(suggestion.as_os_str().to_owned()); + completions.push(candidate); } } } + completions.sort(); + potential.sort(); + completions.extend(potential); completions } From 82a360aa545713e2c70b82d22f1be1e91e9611ff Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 13:39:56 -0500 Subject: [PATCH 10/14] feat(complete): Add ArgValueCompleter --- clap_complete/src/command/mod.rs | 2 + clap_complete/src/engine/complete.rs | 5 +- clap_complete/src/engine/custom.rs | 79 +++++++++++++++++++++++++ clap_complete/src/engine/mod.rs | 2 + clap_complete/src/env/mod.rs | 1 + clap_complete/src/lib.rs | 2 + clap_complete/tests/testsuite/engine.rs | 45 +++++++++++++- 7 files changed, 134 insertions(+), 2 deletions(-) diff --git a/clap_complete/src/command/mod.rs b/clap_complete/src/command/mod.rs index b7fbbf5b0fe..ed3ebeabd58 100644 --- a/clap_complete/src/command/mod.rs +++ b/clap_complete/src/command/mod.rs @@ -50,6 +50,7 @@ pub use shells::*; /// - [`ValueHint`][crate::ValueHint] /// - [`ValueEnum`][clap::ValueEnum] /// - [`ArgValueCandidates`][crate::ArgValueCandidates] +/// - [`ArgValueCompleter`][crate::ArgValueCompleter] /// /// **Warning:** `stdout` should not be written to before [`CompleteCommand::complete`] has had a /// chance to run. @@ -122,6 +123,7 @@ impl CompleteCommand { /// - [`ValueHint`][crate::ValueHint] /// - [`ValueEnum`][clap::ValueEnum] /// - [`ArgValueCandidates`][crate::ArgValueCandidates] +/// - [`ArgValueCompleter`][crate::ArgValueCompleter] /// /// **Warning:** `stdout` should not be written to before [`CompleteArgs::complete`] has had a /// chance to run. diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index b2cdfa30e3c..34110062bee 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -5,6 +5,7 @@ use clap_lex::OsStrExt as _; use super::custom::complete_path; use super::ArgValueCandidates; +use super::ArgValueCompleter; use super::CompletionCandidate; /// Complete the given command, shell-agnostic @@ -271,7 +272,9 @@ fn complete_arg_value( Err(value_os) => value_os, }; - if let Some(completer) = arg.get::() { + if let Some(completer) = arg.get::() { + values.extend(completer.complete(value_os)); + } else if let Some(completer) = arg.get::() { values.extend(complete_custom_arg_value(value_os, completer)); } else if let Some(possible_values) = possible_values(arg) { if let Ok(value) = value { diff --git a/clap_complete/src/engine/custom.rs b/clap_complete/src/engine/custom.rs index 240972cfb54..6879de3f3b1 100644 --- a/clap_complete/src/engine/custom.rs +++ b/clap_complete/src/engine/custom.rs @@ -71,6 +71,85 @@ where } } +/// Extend [`Arg`][clap::Arg] with a completer +/// +/// # Example +/// +/// ```rust +/// use clap::Parser; +/// use clap_complete::engine::{ArgValueCompleter, CompletionCandidate}; +/// +/// fn custom_completer(current: &std::ffi::OsStr) -> Vec { +/// let mut completions = vec![]; +/// let Some(current) = current.to_str() else { +/// return completions; +/// }; +/// +/// if "foo".starts_with(current) { +/// completions.push(CompletionCandidate::new("foo")); +/// } +/// if "bar".starts_with(current) { +/// completions.push(CompletionCandidate::new("bar")); +/// } +/// if "baz".starts_with(current) { +/// completions.push(CompletionCandidate::new("baz")); +/// } +/// completions +/// } +/// +/// #[derive(Debug, Parser)] +/// struct Cli { +/// #[arg(long, add = ArgValueCompleter::new(custom_completer))] +/// custom: Option, +/// } +/// ``` +#[derive(Clone)] +pub struct ArgValueCompleter(Arc); + +impl ArgValueCompleter { + /// Create a new `ArgValueCompleter` with a custom completer + pub fn new(completer: C) -> Self + where + C: ValueCompleter + 'static, + { + Self(Arc::new(completer)) + } + + /// Candidates that match `current` + /// + /// See [`CompletionCandidate`] for more information. + pub fn complete(&self, current: &OsStr) -> Vec { + self.0.complete(current) + } +} + +impl std::fmt::Debug for ArgValueCompleter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(type_name::()) + } +} + +impl ArgExt for ArgValueCompleter {} + +/// User-provided completion candidates for an [`Arg`][clap::Arg], see [`ArgValueCompleter`] +/// +/// This is useful when predefined value hints are not enough. +pub trait ValueCompleter: Send + Sync { + /// All potential candidates for an argument. + /// + /// See [`CompletionCandidate`] for more information. + fn complete(&self, current: &OsStr) -> Vec; +} + +impl ValueCompleter for F +where + F: Fn(&OsStr) -> Vec + Send + Sync, +{ + fn complete(&self, current: &OsStr) -> Vec { + self(current) + } +} + pub(crate) fn complete_path( value_os: &OsStr, current_dir: Option<&std::path::Path>, diff --git a/clap_complete/src/engine/mod.rs b/clap_complete/src/engine/mod.rs index 937577ebf3e..2e7e25d22c6 100644 --- a/clap_complete/src/engine/mod.rs +++ b/clap_complete/src/engine/mod.rs @@ -9,4 +9,6 @@ mod custom; pub use candidate::CompletionCandidate; pub use complete::complete; pub use custom::ArgValueCandidates; +pub use custom::ArgValueCompleter; pub use custom::ValueCandidates; +pub use custom::ValueCompleter; diff --git a/clap_complete/src/env/mod.rs b/clap_complete/src/env/mod.rs index 746549ab2f1..69e0b1d7233 100644 --- a/clap_complete/src/env/mod.rs +++ b/clap_complete/src/env/mod.rs @@ -20,6 +20,7 @@ //! - [`ValueHint`][crate::ValueHint] //! - [`ValueEnum`][clap::ValueEnum] //! - [`ArgValueCandidates`][crate::ArgValueCandidates] +//! - [`ArgValueCompleter`][crate::ArgValueCompleter] //! //! To source your completions: //! diff --git a/clap_complete/src/lib.rs b/clap_complete/src/lib.rs index 5e20c8a3483..f09b8fd1098 100644 --- a/clap_complete/src/lib.rs +++ b/clap_complete/src/lib.rs @@ -81,6 +81,8 @@ pub use command::CompleteCommand; #[doc(inline)] #[cfg(feature = "unstable-dynamic")] pub use engine::ArgValueCandidates; +#[cfg(feature = "unstable-dynamic")] +pub use engine::ArgValueCompleter; #[doc(inline)] #[cfg(feature = "unstable-dynamic")] pub use engine::CompletionCandidate; diff --git a/clap_complete/tests/testsuite/engine.rs b/clap_complete/tests/testsuite/engine.rs index ed70bd0931c..996e394d7ac 100644 --- a/clap_complete/tests/testsuite/engine.rs +++ b/clap_complete/tests/testsuite/engine.rs @@ -4,7 +4,7 @@ use std::fs; use std::path::Path; use clap::{builder::PossibleValue, Command}; -use clap_complete::engine::{ArgValueCandidates, CompletionCandidate}; +use clap_complete::engine::{ArgValueCandidates, ArgValueCompleter, CompletionCandidate}; use snapbox::assert_data_eq; macro_rules! complete { @@ -609,6 +609,49 @@ baz ); } +#[test] +fn suggest_custom_arg_completer() { + fn custom_completer(current: &std::ffi::OsStr) -> Vec { + let mut completions = vec![]; + let Some(current) = current.to_str() else { + return completions; + }; + + if "foo".starts_with(current) { + completions.push(CompletionCandidate::new("foo")); + } + if "bar".starts_with(current) { + completions.push(CompletionCandidate::new("bar")); + } + if "baz".starts_with(current) { + completions.push(CompletionCandidate::new("baz")); + } + completions + } + + let mut cmd = Command::new("dynamic").arg( + clap::Arg::new("custom") + .long("custom") + .add(ArgValueCompleter::new(custom_completer)), + ); + + assert_data_eq!( + complete!(cmd, "--custom [TAB]"), + snapbox::str![[r#" +foo +bar +baz +"#]] + ); + assert_data_eq!( + complete!(cmd, "--custom b[TAB]"), + snapbox::str![[r#" +bar +baz +"#]] + ); +} + #[test] fn suggest_multi_positional() { let mut cmd = Command::new("dynamic") From 49b8108f8c8dfd8bd2d7907c60bed9d2155a42cb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 13:40:07 -0500 Subject: [PATCH 11/14] feat(complete): Add PathCompleter --- clap_complete/src/engine/custom.rs | 72 +++++++++++++++++++++++++ clap_complete/src/engine/mod.rs | 1 + clap_complete/src/lib.rs | 2 + clap_complete/tests/testsuite/engine.rs | 40 +++++++++++++- 4 files changed, 114 insertions(+), 1 deletion(-) diff --git a/clap_complete/src/engine/custom.rs b/clap_complete/src/engine/custom.rs index 6879de3f3b1..86276fbf0ea 100644 --- a/clap_complete/src/engine/custom.rs +++ b/clap_complete/src/engine/custom.rs @@ -150,6 +150,78 @@ where } } +/// Complete a value as a [`std::path::Path`] +/// +/// # Example +/// +/// ```rust +/// use clap::Parser; +/// use clap_complete::engine::{ArgValueCompleter, PathCompleter}; +/// +/// #[derive(Debug, Parser)] +/// struct Cli { +/// #[arg(long, add = ArgValueCompleter::new(PathCompleter::file()))] +/// custom: Option, +/// } +/// ``` +pub struct PathCompleter { + current_dir: Option, + filter: Option bool + Send + Sync>>, +} + +impl PathCompleter { + /// Any path is allowed + pub fn any() -> Self { + Self { + filter: None, + current_dir: None, + } + } + + /// Complete only files + pub fn file() -> Self { + Self::any().filter(|p| p.is_file()) + } + + /// Complete only directories + pub fn dir() -> Self { + Self::any().filter(|p| p.is_dir()) + } + + /// Select which paths should be completed + pub fn filter( + mut self, + filter: impl Fn(&std::path::Path) -> bool + Send + Sync + 'static, + ) -> Self { + self.filter = Some(Box::new(filter)); + self + } + + /// Override [`std::env::current_dir`] + pub fn current_dir(mut self, path: impl Into) -> Self { + self.current_dir = Some(path.into()); + self + } +} + +impl Default for PathCompleter { + fn default() -> Self { + Self::any() + } +} + +impl ValueCompleter for PathCompleter { + fn complete(&self, current: &OsStr) -> Vec { + let filter = self.filter.as_deref().unwrap_or(&|_| true); + let mut current_dir_actual = None; + let current_dir = self.current_dir.as_deref().or_else(|| { + current_dir_actual = std::env::current_dir().ok(); + current_dir_actual.as_deref() + }); + complete_path(current, current_dir, filter) + } +} + pub(crate) fn complete_path( value_os: &OsStr, current_dir: Option<&std::path::Path>, diff --git a/clap_complete/src/engine/mod.rs b/clap_complete/src/engine/mod.rs index 2e7e25d22c6..43a1f9f0c10 100644 --- a/clap_complete/src/engine/mod.rs +++ b/clap_complete/src/engine/mod.rs @@ -10,5 +10,6 @@ pub use candidate::CompletionCandidate; pub use complete::complete; pub use custom::ArgValueCandidates; pub use custom::ArgValueCompleter; +pub use custom::PathCompleter; pub use custom::ValueCandidates; pub use custom::ValueCompleter; diff --git a/clap_complete/src/lib.rs b/clap_complete/src/lib.rs index f09b8fd1098..099ed6929f2 100644 --- a/clap_complete/src/lib.rs +++ b/clap_complete/src/lib.rs @@ -87,6 +87,8 @@ pub use engine::ArgValueCompleter; #[cfg(feature = "unstable-dynamic")] pub use engine::CompletionCandidate; #[cfg(feature = "unstable-dynamic")] +pub use engine::PathCompleter; +#[cfg(feature = "unstable-dynamic")] pub use env::CompleteEnv; /// Deprecated, see [`aot`] diff --git a/clap_complete/tests/testsuite/engine.rs b/clap_complete/tests/testsuite/engine.rs index 996e394d7ac..dc4ab13e556 100644 --- a/clap_complete/tests/testsuite/engine.rs +++ b/clap_complete/tests/testsuite/engine.rs @@ -4,7 +4,9 @@ use std::fs; use std::path::Path; use clap::{builder::PossibleValue, Command}; -use clap_complete::engine::{ArgValueCandidates, ArgValueCompleter, CompletionCandidate}; +use clap_complete::engine::{ + ArgValueCandidates, ArgValueCompleter, CompletionCandidate, PathCompleter, +}; use snapbox::assert_data_eq; macro_rules! complete { @@ -575,6 +577,42 @@ d_dir/ ); } +#[test] +fn suggest_value_path_file() { + let testdir = snapbox::dir::DirRoot::mutable_temp().unwrap(); + let testdir_path = testdir.path().unwrap(); + fs::write(testdir_path.join("a_file"), "").unwrap(); + fs::write(testdir_path.join("b_file"), "").unwrap(); + fs::create_dir_all(testdir_path.join("c_dir")).unwrap(); + fs::create_dir_all(testdir_path.join("d_dir")).unwrap(); + + let mut cmd = Command::new("dynamic") + .arg( + clap::Arg::new("input") + .long("input") + .short('i') + .add(ArgValueCompleter::new( + PathCompleter::file().current_dir(testdir_path.to_owned()), + )), + ) + .args_conflicts_with_subcommands(true); + + assert_data_eq!( + complete!(cmd, "--input [TAB]", current_dir = Some(testdir_path)), + snapbox::str![[r#" +a_file +b_file +c_dir/ +d_dir/ +"#]], + ); + + assert_data_eq!( + complete!(cmd, "--input a[TAB]", current_dir = Some(testdir_path)), + snapbox::str!["a_file"], + ); +} + #[test] fn suggest_custom_arg_value() { fn custom_completer() -> Vec { From 27b348dbcbd647d24b506ed70c61ebe23c2b087e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 14:16:52 -0500 Subject: [PATCH 12/14] refactor(complete): Simplify ArgValueCandidates code --- clap_complete/src/engine/complete.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index 34110062bee..24fc9cac291 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -351,12 +351,8 @@ fn complete_custom_arg_value( ) -> Vec { debug!("complete_custom_arg_value: completer={completer:?}, value={value:?}"); - let mut values = Vec::new(); - let custom_arg_values = completer.candidates(); - values.extend(custom_arg_values); - + let mut values = completer.candidates(); values.retain(|comp| comp.get_content().starts_with(&value.to_string_lossy())); - values } From bb6493e890a1fb0d954d64916650688b276c44f1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 14:24:16 -0500 Subject: [PATCH 13/14] feat(complete): Offer - as a path option --- clap_complete/src/engine/custom.rs | 14 +++++++++++++- clap_complete/tests/testsuite/engine.rs | 5 ++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/clap_complete/src/engine/custom.rs b/clap_complete/src/engine/custom.rs index 86276fbf0ea..10c15f3806c 100644 --- a/clap_complete/src/engine/custom.rs +++ b/clap_complete/src/engine/custom.rs @@ -167,6 +167,7 @@ where pub struct PathCompleter { current_dir: Option, filter: Option bool + Send + Sync>>, + stdio: bool, } impl PathCompleter { @@ -175,6 +176,7 @@ impl PathCompleter { Self { filter: None, current_dir: None, + stdio: false, } } @@ -188,6 +190,12 @@ impl PathCompleter { Self::any().filter(|p| p.is_dir()) } + /// Include stdio (`-`) + pub fn stdio(mut self) -> Self { + self.stdio = true; + self + } + /// Select which paths should be completed pub fn filter( mut self, @@ -218,7 +226,11 @@ impl ValueCompleter for PathCompleter { current_dir_actual = std::env::current_dir().ok(); current_dir_actual.as_deref() }); - complete_path(current, current_dir, filter) + let mut candidates = complete_path(current, current_dir, filter); + if self.stdio && current.is_empty() { + candidates.push(CompletionCandidate::new("-").help(Some("stdio".into()))); + } + candidates } } diff --git a/clap_complete/tests/testsuite/engine.rs b/clap_complete/tests/testsuite/engine.rs index dc4ab13e556..56410a22aff 100644 --- a/clap_complete/tests/testsuite/engine.rs +++ b/clap_complete/tests/testsuite/engine.rs @@ -592,7 +592,9 @@ fn suggest_value_path_file() { .long("input") .short('i') .add(ArgValueCompleter::new( - PathCompleter::file().current_dir(testdir_path.to_owned()), + PathCompleter::file() + .stdio() + .current_dir(testdir_path.to_owned()), )), ) .args_conflicts_with_subcommands(true); @@ -604,6 +606,7 @@ a_file b_file c_dir/ d_dir/ +- stdio "#]], ); From 951762db57c81e09954031ef3beac6e9f622cc6f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 21 Aug 2024 14:25:40 -0500 Subject: [PATCH 14/14] feat(complete): Allow any OsString-compatible type to be a CompletionCandidate --- clap_complete/src/engine/candidate.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clap_complete/src/engine/candidate.rs b/clap_complete/src/engine/candidate.rs index 50f8b33a4aa..ed0d0df1b77 100644 --- a/clap_complete/src/engine/candidate.rs +++ b/clap_complete/src/engine/candidate.rs @@ -65,3 +65,9 @@ impl CompletionCandidate { self.hidden } } + +impl> From for CompletionCandidate { + fn from(s: S) -> Self { + CompletionCandidate::new(s.into()) + } +}