Clap 3 → Clap 4 Upgrade (#3431)

* Upgrade to Clap 4

Signed-off-by: Brian Hardock <brian.hardock@fermyon.com>

* Remove vendored clap_markdown

Signed-off-by: Brian Hardock <brian.hardock@fermyon.com>

* Reintroduce cmd argument for external

Signed-off-by: Brian Hardock <brian.hardock@fermyon.com>

---------

Signed-off-by: Brian Hardock <brian.hardock@fermyon.com>

Author: fibonacci1729

Cargo.lock

@@ -18,8 +18,9 @@ rust-version = "1.86"
 anyhow = { workspace = true }
 async-trait = { workspace = true }
 bytes = { workspace = true }
-# 'deprecated' enables deprecation warnings
-clap = { workspace = true, features = ["deprecated", "derive", "env"] }
+clap = { workspace = true, features = ["derive", "env", "string", "wrap_help"] }
+clap-markdown = "0.1.5"
+clap_lex = "0.7.5"
 clearscreen = "4"
 comfy-table = "7"
 command-group = { version = "5", features = ["with-tokio"] }
@@ -133,7 +134,7 @@ async-trait = "0.1"
 base64 = "0.22"
 bytes = "1.11.1"
 chrono = "0.4"
-clap = "3.2"
+clap = "4.6.0"
 conformance-tests = { git = "https://github.com/fermyon/conformance-tests", rev = "ffc2c879cdcb3c619c8f95ae8e0b847af8c8247e" }
 ctrlc = { version = "3.4", features = ["termination"] }
 dialoguer = "0.11"

Cargo.toml

@@ -6,6 +6,7 @@ edition = { workspace = true }
 
 [dependencies]
 anyhow = { workspace = true }
+clap = { workspace = true }
 dirs = { workspace = true }
 sha2 = { workspace = true }
 tempfile = { workspace = true }

crates/common/Cargo.toml

@@ -0,0 +1,10 @@
+//! Common CLI code and constants
+
+use clap::builder::{styling::AnsiColor, Styles};
+
+/// Clap [`Styles`] for Spin CLI and plugins.
+pub const CLAP_STYLES: Styles = Styles::styled()
+    .header(AnsiColor::Yellow.on_default())
+    .usage(AnsiColor::Green.on_default())
+    .literal(AnsiColor::Green.on_default())
+    .placeholder(AnsiColor::Green.on_default());

crates/common/src/cli.rs

@@ -10,6 +10,7 @@
 
 pub mod arg_parser;
 pub mod assert;
+pub mod cli;
 pub mod data_dir;
 pub mod paths;
 pub mod sha256;

crates/common/src/lib.rs

@@ -98,10 +98,7 @@ mod tests {
             let template = Template::new(tmpl).unwrap();
             assert!(
                 template.parts().eq(&expected),
-                "{:?} -> {:?} != {:?}",
-                tmpl,
-                template,
-                expected,
+                "{tmpl:?} -> {template:?} != {expected:?}",
             );
         }
     }

crates/expressions/src/template.rs

@@ -57,11 +57,11 @@ pub struct CliArgs {
     pub address: SocketAddr,
 
     /// The path to the certificate to use for https, if this is not set, normal http will be used. The cert should be in PEM format
-    #[clap(long, env = "SPIN_TLS_CERT", requires = "tls-key")]
+    #[clap(long, env = "SPIN_TLS_CERT", requires = "tls_key")]
     pub tls_cert: Option<PathBuf>,
 
     /// The path to the certificate key to use for https, if this is not set, normal http will be used. The key should be in PKCS#8 format
-    #[clap(long, env = "SPIN_TLS_KEY", requires = "tls-cert")]
+    #[clap(long, env = "SPIN_TLS_KEY", requires = "tls_cert")]
     pub tls_key: Option<PathBuf>,
 
     /// Sets the maximum buffer size (in bytes) for the HTTP connection. The minimum value allowed is 8192.
@@ -300,6 +300,10 @@ impl<F: RuntimeFactors> Trigger<F> for HttpTrigger {
     fn supported_host_requirements() -> Vec<&'static str> {
         vec![spin_app::locked::SERVICE_CHAINING_KEY]
     }
+
+    fn display_name() -> String {
+        "HTTP".to_string()
+    }
 }
 
 impl HttpTrigger {

crates/trigger-http/src/lib.rs

@@ -18,9 +18,10 @@ experimental-wasm-features = []
 
 [dependencies]
 anyhow = { workspace = true }
-clap = { workspace = true, features = ["derive", "env"] }
+clap = { workspace = true, features = ["derive", "env", "wrap_help"] }
 ctrlc = { workspace = true }
 futures = { workspace = true }
+heck = { workspace = true }
 sanitize-filename = "0.5"
 serde = { workspace = true }
 serde_json = { workspace = true }

crates/trigger/Cargo.toml

@@ -12,7 +12,7 @@ use std::{future::Future, sync::Arc};
 use anyhow::{Context, Result};
 #[cfg(feature = "experimental-wasm-features")]
 use clap::ValueEnum;
-use clap::{Args, IntoApp, Parser};
+use clap::{Args, CommandFactory, Parser};
 use spin_app::App;
 use spin_common::sloth;
 use spin_common::ui::quoted_path;
@@ -45,6 +45,7 @@ pub const SPIN_WORKING_DIR: &str = "SPIN_WORKING_DIR";
 /// A command that runs a TriggerExecutor.
 #[derive(Parser, Debug)]
 #[clap(
+    styles = spin_common::cli::CLAP_STYLES,
     override_usage = "spin [COMMAND] [OPTIONS]",
     next_help_heading = help_heading::<T, B::Factors>()
 )]
@@ -72,7 +73,6 @@ pub struct FactorsTriggerCommand<T: Trigger<B::Factors>, B: RuntimeFactorsBuilde
         long = "disable-cache",
         env = DISABLE_WASMTIME_CACHE,
         conflicts_with = WASMTIME_CACHE_FILE,
-        takes_value = false,
     )]
     pub disable_cache: bool,
 
@@ -91,7 +91,7 @@ pub struct FactorsTriggerCommand<T: Trigger<B::Factors>, B: RuntimeFactorsBuilde
 
     /// Enable Wasmtime's debug info for Wasm guests, allowing debugging
     /// with gdb or lldb.
-    #[clap(long = "debug-info", takes_value = false)]
+    #[clap(long = "debug-info")]
     pub debug_info: bool,
 
     /// Print output to stdout/stderr only for given component(s)
@@ -330,9 +330,9 @@ fn warn_if_wasm_build_slothful() -> sloth::SlothGuard {
 
 fn help_heading<T: Trigger<F>, F: RuntimeFactors>() -> Option<&'static str> {
     if T::TYPE == <help::HelpArgsOnlyTrigger as Trigger<F>>::TYPE {
-        Some("TRIGGER OPTIONS")
+        Some("Trigger Options")
     } else {
-        let heading = format!("{} TRIGGER OPTIONS", T::TYPE.to_uppercase());
+        let heading = format!("{} Trigger Options", T::display_name());
         let as_str = Box::new(heading).leak();
         Some(as_str)
     }

crates/trigger/src/cli.rs

@@ -1,6 +1,7 @@
 pub mod cli;
 pub mod loader;
 
+use heck::ToTitleCase;
 use std::future::Future;
 
 use clap::Args;
@@ -71,4 +72,9 @@ pub trait Trigger<F: RuntimeFactors>: Sized + Send {
     fn supported_host_requirements() -> Vec<&'static str> {
         Vec::new()
     }
+
+    /// Returns the display name for the type of this trigger. Defaults to title case.
+    fn display_name() -> String {
+        Self::TYPE.to_title_case()
+    }
 }