From d69e0dc777c10f1c5e952810845d520b102ecf9a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 10 Aug 2024 20:24:53 -0500 Subject: [PATCH 1/4] fix(complete)!: Rename ShwllCompleter to CommandCompleter This opens space for several completion strategies --- clap_complete/src/dynamic/shells/command.rs | 20 ++++++++++---------- clap_complete/src/dynamic/shells/mod.rs | 14 +++++++------- clap_complete/tests/testsuite/bash.rs | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/clap_complete/src/dynamic/shells/command.rs b/clap_complete/src/dynamic/shells/command.rs index 4aa1d0d33b0..a171802da64 100644 --- a/clap_complete/src/dynamic/shells/command.rs +++ b/clap_complete/src/dynamic/shells/command.rs @@ -229,15 +229,15 @@ impl CompleteArgs { /// This will generally be called by [`CompleteCommand`] or [`CompleteArgs`]. /// /// This handles adapting between the shell and [`completer`][crate::dynamic::complete()]. -/// A `ShellCompleter` can choose how much of that lives within the registration script and or -/// lives in [`ShellCompleter::write_complete`]. -pub trait ShellCompleter { +/// A `CommandCompleter` can choose how much of that lives within the registration script and or +/// lives in [`CommandCompleter::write_complete`]. +pub trait CommandCompleter { /// The recommended file name for the registration code fn file_name(&self, name: &str) -> String; /// Register for completions /// /// Write the `buf` the logic needed for calling into ` complete`, passing needed - /// arguments to [`ShellCompleter::write_complete`] through the environment. + /// arguments to [`CommandCompleter::write_complete`] through the environment. fn write_registration( &self, name: &str, @@ -247,7 +247,7 @@ pub trait ShellCompleter { ) -> Result<(), std::io::Error>; /// Complete the given command /// - /// Adapt information from arguments and [`ShellCompleter::write_registration`]-defined env + /// Adapt information from arguments and [`CommandCompleter::write_registration`]-defined env /// variables to what is needed for [`completer`][crate::dynamic::complete()]. /// /// Write out the [`CompletionCandidate`][crate::dynamic::CompletionCandidate]s in a way the shell will understand. @@ -260,7 +260,7 @@ pub trait ShellCompleter { ) -> Result<(), std::io::Error>; } -impl ShellCompleter for super::Bash { +impl CommandCompleter for super::Bash { fn file_name(&self, name: &str) -> String { format!("{name}.bash") } @@ -380,7 +380,7 @@ impl Default for CompType { Self::Normal } } -impl ShellCompleter for super::Elvish { +impl CommandCompleter for super::Elvish { fn file_name(&self, name: &str) -> String { format!("{name}.elv") } @@ -436,7 +436,7 @@ set edit:completion:arg-completer[BIN] = { |@words| } } -impl ShellCompleter for super::Fish { +impl CommandCompleter for super::Fish { fn file_name(&self, name: &str) -> String { format!("{name}.fish") } @@ -481,7 +481,7 @@ impl ShellCompleter for super::Fish { } } -impl ShellCompleter for super::Powershell { +impl CommandCompleter for super::Powershell { fn file_name(&self, name: &str) -> String { format!("{name}.ps1") } @@ -547,7 +547,7 @@ Register-ArgumentCompleter -Native -CommandName {bin} -ScriptBlock {{ } } -impl ShellCompleter for super::Zsh { +impl CommandCompleter for super::Zsh { fn file_name(&self, name: &str) -> String { format!("{name}.zsh") } diff --git a/clap_complete/src/dynamic/shells/mod.rs b/clap_complete/src/dynamic/shells/mod.rs index 74c259fed0b..bc4fd570920 100644 --- a/clap_complete/src/dynamic/shells/mod.rs +++ b/clap_complete/src/dynamic/shells/mod.rs @@ -133,7 +133,7 @@ impl ValueEnum for Shell { } impl Shell { - fn completer(&self) -> &dyn ShellCompleter { + fn completer(&self) -> &dyn CommandCompleter { match self { Self::Bash => &Bash, Self::Elvish => &Elvish, @@ -144,7 +144,7 @@ impl Shell { } } -impl ShellCompleter for Shell { +impl CommandCompleter for Shell { fn file_name(&self, name: &str) -> String { self.completer().file_name(name) } @@ -169,22 +169,22 @@ impl ShellCompleter for Shell { } } -/// A [`ShellCompleter`] for Bash +/// A [`CommandCompleter`] for Bash #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Bash; -/// A [`ShellCompleter`] for Elvish +/// A [`CommandCompleter`] for Elvish #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Elvish; -/// A [`ShellCompleter`] for Fish +/// A [`CommandCompleter`] for Fish #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Fish; -/// A [`ShellCompleter`] for Powershell +/// A [`CommandCompleter`] for Powershell #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Powershell; -/// A [`ShellCompleter`] for zsh +/// A [`CommandCompleter`] for zsh #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Zsh; diff --git a/clap_complete/tests/testsuite/bash.rs b/clap_complete/tests/testsuite/bash.rs index cc19243c0bd..70b8cf26388 100644 --- a/clap_complete/tests/testsuite/bash.rs +++ b/clap_complete/tests/testsuite/bash.rs @@ -114,7 +114,7 @@ fn value_terminator() { #[cfg(feature = "unstable-dynamic")] #[test] fn register_minimal() { - use clap_complete::dynamic::shells::ShellCompleter as _; + use clap_complete::dynamic::shells::CommandCompleter as _; let name = "my-app"; let bin = name; From d064946c60cbaae6e2563877a6fb0ee1dabf67e9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 10 Aug 2024 20:28:22 -0500 Subject: [PATCH 2/4] refactor(complete): Consolidate CommandCompleter's --- clap_complete/src/dynamic/shells/command.rs | 25 +++++++++++++++++++++ clap_complete/src/dynamic/shells/mod.rs | 25 --------------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/clap_complete/src/dynamic/shells/command.rs b/clap_complete/src/dynamic/shells/command.rs index a171802da64..10e264116cb 100644 --- a/clap_complete/src/dynamic/shells/command.rs +++ b/clap_complete/src/dynamic/shells/command.rs @@ -260,6 +260,31 @@ pub trait CommandCompleter { ) -> Result<(), std::io::Error>; } +impl CommandCompleter for Shell { + fn file_name(&self, name: &str) -> String { + self.completer().file_name(name) + } + fn write_registration( + &self, + name: &str, + bin: &str, + completer: &str, + buf: &mut dyn std::io::Write, + ) -> Result<(), std::io::Error> { + self.completer() + .write_registration(name, bin, completer, buf) + } + fn write_complete( + &self, + cmd: &mut clap::Command, + args: Vec, + current_dir: Option<&std::path::Path>, + buf: &mut dyn std::io::Write, + ) -> Result<(), std::io::Error> { + self.completer().write_complete(cmd, args, current_dir, buf) + } +} + impl CommandCompleter for super::Bash { fn file_name(&self, name: &str) -> String { format!("{name}.bash") diff --git a/clap_complete/src/dynamic/shells/mod.rs b/clap_complete/src/dynamic/shells/mod.rs index bc4fd570920..9a6f848ed59 100644 --- a/clap_complete/src/dynamic/shells/mod.rs +++ b/clap_complete/src/dynamic/shells/mod.rs @@ -144,31 +144,6 @@ impl Shell { } } -impl CommandCompleter for Shell { - fn file_name(&self, name: &str) -> String { - self.completer().file_name(name) - } - fn write_registration( - &self, - name: &str, - bin: &str, - completer: &str, - buf: &mut dyn std::io::Write, - ) -> Result<(), std::io::Error> { - self.completer() - .write_registration(name, bin, completer, buf) - } - fn write_complete( - &self, - cmd: &mut clap::Command, - args: Vec, - current_dir: Option<&std::path::Path>, - buf: &mut dyn std::io::Write, - ) -> Result<(), std::io::Error> { - self.completer().write_complete(cmd, args, current_dir, buf) - } -} - /// A [`CommandCompleter`] for Bash #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Bash; From 2b2b2be610c9397229f838116c9544cad2b1d4bd Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 10 Aug 2024 20:34:20 -0500 Subject: [PATCH 3/4] refactor(complete): Isolate all CommandCompleter knowledge --- clap_complete/src/dynamic/shells/command.rs | 17 +++++++++++++---- clap_complete/src/dynamic/shells/mod.rs | 12 ------------ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/clap_complete/src/dynamic/shells/command.rs b/clap_complete/src/dynamic/shells/command.rs index 10e264116cb..2efd3f63965 100644 --- a/clap_complete/src/dynamic/shells/command.rs +++ b/clap_complete/src/dynamic/shells/command.rs @@ -262,7 +262,7 @@ pub trait CommandCompleter { impl CommandCompleter for Shell { fn file_name(&self, name: &str) -> String { - self.completer().file_name(name) + shell_completer(self).file_name(name) } fn write_registration( &self, @@ -271,8 +271,7 @@ impl CommandCompleter for Shell { completer: &str, buf: &mut dyn std::io::Write, ) -> Result<(), std::io::Error> { - self.completer() - .write_registration(name, bin, completer, buf) + shell_completer(self).write_registration(name, bin, completer, buf) } fn write_complete( &self, @@ -281,7 +280,17 @@ impl CommandCompleter for Shell { current_dir: Option<&std::path::Path>, buf: &mut dyn std::io::Write, ) -> Result<(), std::io::Error> { - self.completer().write_complete(cmd, args, current_dir, buf) + shell_completer(self).write_complete(cmd, args, current_dir, buf) + } +} + +fn shell_completer(shell: &Shell) -> &dyn CommandCompleter { + match shell { + Shell::Bash => &super::Bash, + Shell::Elvish => &super::Elvish, + Shell::Fish => &super::Fish, + Shell::Powershell => &super::Powershell, + Shell::Zsh => &super::Zsh, } } diff --git a/clap_complete/src/dynamic/shells/mod.rs b/clap_complete/src/dynamic/shells/mod.rs index 9a6f848ed59..2a0f87223c8 100644 --- a/clap_complete/src/dynamic/shells/mod.rs +++ b/clap_complete/src/dynamic/shells/mod.rs @@ -132,18 +132,6 @@ impl ValueEnum for Shell { } } -impl Shell { - fn completer(&self) -> &dyn CommandCompleter { - match self { - Self::Bash => &Bash, - Self::Elvish => &Elvish, - Self::Fish => &Fish, - Self::Powershell => &Powershell, - Self::Zsh => &Zsh, - } - } -} - /// A [`CommandCompleter`] for Bash #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Bash; From b94fce32426dd6505c9e99faa8f51e0e163761f4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 10 Aug 2024 20:41:01 -0500 Subject: [PATCH 4/4] fix(complete)!: Put CompleteCommand behind unstable-command Avoid the cost of the deps when not needed. --- clap_complete/Cargo.toml | 7 ++++--- clap_complete/examples/exhaustive.rs | 6 +++--- clap_complete/src/dynamic/candidate.rs | 2 +- clap_complete/src/dynamic/complete.rs | 2 +- clap_complete/src/dynamic/mod.rs | 2 ++ clap_complete/src/dynamic/shells/command.rs | 3 +++ clap_complete/src/dynamic/shells/mod.rs | 12 +++++++----- clap_complete/tests/testsuite/bash.rs | 2 +- clap_complete/tests/testsuite/common.rs | 2 ++ clap_complete/tests/testsuite/elvish.rs | 4 ++-- clap_complete/tests/testsuite/fish.rs | 4 ++-- clap_complete/tests/testsuite/zsh.rs | 4 ++-- 12 files changed, 30 insertions(+), 20 deletions(-) diff --git a/clap_complete/Cargo.toml b/clap_complete/Cargo.toml index f20969ec3fc..9461771e6c5 100644 --- a/clap_complete/Cargo.toml +++ b/clap_complete/Cargo.toml @@ -52,12 +52,13 @@ automod = "1.0.14" [[example]] name = "dynamic" -required-features = ["unstable-dynamic"] +required-features = ["unstable-dynamic", "unstable-command"] [features] default = [] -unstable-doc = ["unstable-dynamic"] # for docs.rs -unstable-dynamic = ["dep:clap_lex", "dep:shlex", "dep:unicode-xid", "clap/derive", "dep:is_executable", "dep:pathdiff", "clap/unstable-ext"] +unstable-doc = ["unstable-dynamic", "unstable-command"] # for docs.rs +unstable-dynamic = ["dep:clap_lex", "dep:is_executable", "dep:pathdiff", "clap/unstable-ext"] +unstable-command = ["unstable-dynamic", "dep:shlex", "dep:unicode-xid", "clap/derive", "dep:is_executable", "dep:pathdiff", "clap/unstable-ext"] debug = ["clap/debug"] [lints] diff --git a/clap_complete/examples/exhaustive.rs b/clap_complete/examples/exhaustive.rs index 7a79c1088c5..ce7d313fd30 100644 --- a/clap_complete/examples/exhaustive.rs +++ b/clap_complete/examples/exhaustive.rs @@ -1,5 +1,5 @@ use clap::builder::PossibleValue; -#[cfg(feature = "unstable-dynamic")] +#[cfg(feature = "unstable-command")] use clap::{FromArgMatches, Subcommand}; use clap_complete::{generate, Generator, Shell}; @@ -12,7 +12,7 @@ fn main() { return; } - #[cfg(feature = "unstable-dynamic")] + #[cfg(feature = "unstable-command")] if let Ok(completions) = clap_complete::dynamic::CompleteCommand::from_arg_matches(&matches) { completions.complete(&mut cli()); return; @@ -195,7 +195,7 @@ fn cli() -> clap::Command { .value_hint(clap::ValueHint::EmailAddress), ]), ]); - #[cfg(feature = "unstable-dynamic")] + #[cfg(feature = "unstable-command")] let cli = clap_complete::dynamic::CompleteCommand::augment_subcommands(cli); cli } diff --git a/clap_complete/src/dynamic/candidate.rs b/clap_complete/src/dynamic/candidate.rs index 8add845d573..517e287918e 100644 --- a/clap_complete/src/dynamic/candidate.rs +++ b/clap_complete/src/dynamic/candidate.rs @@ -5,7 +5,7 @@ use clap::builder::StyledStr; /// A shell-agnostic completion candidate /// -/// [`ShellCompleter`][crate::dynamic::shells::ShellCompleter] will adapt what it can to the +/// [`Shell`][crate::dynamic::shells::Shell] will adapt what it can to the /// current shell. #[derive(Default, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct CompletionCandidate { diff --git a/clap_complete/src/dynamic/complete.rs b/clap_complete/src/dynamic/complete.rs index c80e71b420a..214e3d6594b 100644 --- a/clap_complete/src/dynamic/complete.rs +++ b/clap_complete/src/dynamic/complete.rs @@ -10,7 +10,7 @@ use super::CompletionCandidate; /// /// For integration with clap and shells, see [`CompleteCommand`][crate::dynamic::shells::CompleteCommand]. /// -/// This is generally called by a [`ShellCompleter`][crate::dynamic::shells::ShellCompleter] which +/// This is generally called by a [`Shell`][crate::dynamic::shells::Shell] which /// handles the shell-specific logic. pub fn complete( cmd: &mut clap::Command, diff --git a/clap_complete/src/dynamic/mod.rs b/clap_complete/src/dynamic/mod.rs index ef396d8d373..3ccb879e6c8 100644 --- a/clap_complete/src/dynamic/mod.rs +++ b/clap_complete/src/dynamic/mod.rs @@ -19,5 +19,7 @@ pub use custom::ArgValueCompleter; pub use custom::CustomCompleter; // These live in `shells` because they are tightly coupled with the `ShellCompleter`s +#[cfg(feature = "unstable-command")] pub use shells::CompleteArgs; +#[cfg(feature = "unstable-command")] pub use shells::CompleteCommand; diff --git a/clap_complete/src/dynamic/shells/command.rs b/clap_complete/src/dynamic/shells/command.rs index 2efd3f63965..4ed25c9c6f1 100644 --- a/clap_complete/src/dynamic/shells/command.rs +++ b/clap_complete/src/dynamic/shells/command.rs @@ -76,6 +76,7 @@ use super::Shell; #[allow(missing_docs)] #[derive(Clone, Debug)] #[command(about = None, long_about = None)] +#[cfg(feature = "unstable-command")] pub enum CompleteCommand { /// Register shell completions for this program #[command(hide = true)] @@ -173,6 +174,7 @@ impl CompleteCommand { /// ``` #[derive(clap::Args, Clone, Debug)] #[command(about = None, long_about = None)] +#[cfg(feature = "unstable-command")] pub struct CompleteArgs { /// Specify shell to complete for #[arg(value_name = "NAME")] @@ -231,6 +233,7 @@ impl CompleteArgs { /// This handles adapting between the shell and [`completer`][crate::dynamic::complete()]. /// A `CommandCompleter` can choose how much of that lives within the registration script and or /// lives in [`CommandCompleter::write_complete`]. +#[cfg(feature = "unstable-command")] pub trait CommandCompleter { /// The recommended file name for the registration code fn file_name(&self, name: &str) -> String; diff --git a/clap_complete/src/dynamic/shells/mod.rs b/clap_complete/src/dynamic/shells/mod.rs index 2a0f87223c8..147d95ae0b2 100644 --- a/clap_complete/src/dynamic/shells/mod.rs +++ b/clap_complete/src/dynamic/shells/mod.rs @@ -1,7 +1,9 @@ //! Shell completion support, see [`CompleteCommand`] for more details +#[cfg(feature = "unstable-command")] mod command; +#[cfg(feature = "unstable-command")] pub use command::*; use std::fmt::Display; @@ -132,22 +134,22 @@ impl ValueEnum for Shell { } } -/// A [`CommandCompleter`] for Bash +/// Bash completion adapter #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Bash; -/// A [`CommandCompleter`] for Elvish +/// Elvish completion adapter #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Elvish; -/// A [`CommandCompleter`] for Fish +/// Fish completion adapter #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Fish; -/// A [`CommandCompleter`] for Powershell +/// Powershell completion adapter #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Powershell; -/// A [`CommandCompleter`] for zsh +/// Zsh completion adapter #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Zsh; diff --git a/clap_complete/tests/testsuite/bash.rs b/clap_complete/tests/testsuite/bash.rs index 70b8cf26388..c5d9267f0ee 100644 --- a/clap_complete/tests/testsuite/bash.rs +++ b/clap_complete/tests/testsuite/bash.rs @@ -111,7 +111,7 @@ fn value_terminator() { ); } -#[cfg(feature = "unstable-dynamic")] +#[cfg(feature = "unstable-command")] #[test] fn register_minimal() { use clap_complete::dynamic::shells::CommandCompleter as _; diff --git a/clap_complete/tests/testsuite/common.rs b/clap_complete/tests/testsuite/common.rs index 29d0f6df72f..7ebfea6837a 100644 --- a/clap_complete/tests/testsuite/common.rs +++ b/clap_complete/tests/testsuite/common.rs @@ -321,6 +321,7 @@ pub(crate) fn register_example(context: &str, nam // Unconditionally include to avoid completion file tests failing based on the how // `cargo test` is invoked "--features=unstable-dynamic", + "--features=unstable-command", ], ) .unwrap(); @@ -379,6 +380,7 @@ where // Unconditionally include to avoid completion file tests failing based on the how // `cargo test` is invoked "--features=unstable-dynamic", + "--features=unstable-command", ], ) .unwrap(); diff --git a/clap_complete/tests/testsuite/elvish.rs b/clap_complete/tests/testsuite/elvish.rs index b013077311d..9feeb0b4145 100644 --- a/clap_complete/tests/testsuite/elvish.rs +++ b/clap_complete/tests/testsuite/elvish.rs @@ -175,14 +175,14 @@ value value assert_data_eq!(actual, expected); } -#[cfg(all(unix, feature = "unstable-dynamic"))] +#[cfg(all(unix, feature = "unstable-command"))] #[test] fn register_dynamic() { common::register_example::("dynamic", "exhaustive"); } #[test] -#[cfg(all(unix, feature = "unstable-dynamic"))] +#[cfg(all(unix, feature = "unstable-command"))] fn complete_dynamic() { if !common::has_command("elvish") { return; diff --git a/clap_complete/tests/testsuite/fish.rs b/clap_complete/tests/testsuite/fish.rs index b35458fd21e..f27db021934 100644 --- a/clap_complete/tests/testsuite/fish.rs +++ b/clap_complete/tests/testsuite/fish.rs @@ -165,14 +165,14 @@ bash (bash (shell)) fish (fish shell) zsh (zsh shell)"#; assert_data_eq!(actual, expected); } -#[cfg(all(unix, feature = "unstable-dynamic"))] +#[cfg(all(unix, feature = "unstable-command"))] #[test] fn register_dynamic() { common::register_example::("dynamic", "exhaustive"); } #[test] -#[cfg(all(unix, feature = "unstable-dynamic"))] +#[cfg(all(unix, feature = "unstable-command"))] fn complete_dynamic() { if !common::has_command("fish") { return; diff --git a/clap_complete/tests/testsuite/zsh.rs b/clap_complete/tests/testsuite/zsh.rs index fbfd1ac71a5..b5abb971010 100644 --- a/clap_complete/tests/testsuite/zsh.rs +++ b/clap_complete/tests/testsuite/zsh.rs @@ -163,14 +163,14 @@ pacman action alias value quote hint last -- assert_data_eq!(actual, expected); } -#[cfg(all(unix, feature = "unstable-dynamic"))] +#[cfg(all(unix, feature = "unstable-command"))] #[test] fn register_dynamic() { common::register_example::("dynamic", "exhaustive"); } #[test] -#[cfg(all(unix, feature = "unstable-dynamic"))] +#[cfg(all(unix, feature = "unstable-command"))] fn complete_dynamic() { if !common::has_command("zsh") { return;