diff --git a/brush-core/src/builtins.rs b/brush-core/src/builtins.rs index 3c5ab696..7115cda4 100644 --- a/brush-core/src/builtins.rs +++ b/brush-core/src/builtins.rs @@ -382,3 +382,75 @@ fn brush_help_styles() -> clap::builder::Styles { .literal(styling::AnsiColor::Magenta.on_default() | styling::Effects::BOLD) .placeholder(styling::AnsiColor::Cyan.on_default()) } + +/// This function and the [`try_parse_known`] exists to deal with +/// the Clap's limitation of treating `--` like a regular value +/// `https://github.com/clap-rs/clap/issues/5055` +/// +/// # Arguments +/// +/// * `args` - An Iterator from [`std::env::args`] +/// +/// # Returns +/// +/// * a parsed struct T from [`clap::Parser::parse_from`] +/// * the remain iterator `args` with `--` and the rest arguments if they present +/// othervise None +/// +/// # Examples +/// ``` +/// use clap::{builder::styling, Parser}; +/// #[derive(Parser)] +/// struct CommandLineArgs { +/// #[clap(allow_hyphen_values = true, num_args=1..)] +/// script_args: Vec, +/// } +/// +/// let (mut parsed_args, raw_args) = +/// brush_core::builtins::parse_known::(std::env::args()); +/// if raw_args.is_some() { +/// parsed_args.script_args = raw_args.unwrap().collect(); +/// } +/// ``` +pub fn parse_known( + args: impl IntoIterator, +) -> (T, Option>) +where + S: Into + Clone + PartialEq<&'static str>, +{ + let mut args = args.into_iter(); + // the best way to save `--` is to get it out with a side effect while `clap` iterates over the args + // this way we can be 100% sure that we have '--' and the remaining args + // and we will iterate only once + let mut hyphen = None; + let args_before_hyphen = args.by_ref().take_while(|a| { + let is_hyphen = *a == "--"; + if is_hyphen { + hyphen = Some(a.clone()); + } + !is_hyphen + }); + let parsed_args = T::parse_from(args_before_hyphen); + let raw_args = hyphen.map(|hyphen| std::iter::once(hyphen).chain(args)); + (parsed_args, raw_args) +} + +/// Similar to [`parse_known`] but with [`clap::Parser::try_parse_from`] +/// This function is used to parse arguments in builtins such as [`crate::builtins::echo::EchoCommand`] +pub fn try_parse_known( + args: impl IntoIterator, +) -> Result<(T, Option>), clap::Error> { + let mut args = args.into_iter(); + let mut hyphen = None; + let args_before_hyphen = args.by_ref().take_while(|a| { + let is_hyphen = a == "--"; + if is_hyphen { + hyphen = Some(a.clone()); + } + !is_hyphen + }); + let parsed_args = T::try_parse_from(args_before_hyphen)?; + + let raw_args = hyphen.map(|hyphen| std::iter::once(hyphen).chain(args)); + Ok((parsed_args, raw_args)) +} diff --git a/brush-core/src/builtins/echo.rs b/brush-core/src/builtins/echo.rs index 033f5ef1..3b7aaa1d 100644 --- a/brush-core/src/builtins/echo.rs +++ b/brush-core/src/builtins/echo.rs @@ -26,6 +26,20 @@ pub(crate) struct EchoCommand { #[async_trait::async_trait] impl builtins::Command for EchoCommand { + /// Override the default [builtins::Command::new] function to handle clap's limitation related to `--`. + /// See [crate::builtins::parse_known] for more information + /// TODO: we can safely remove this after the issue is resolved + fn new(args: I) -> Result + where + I: IntoIterator, + { + let (mut this, rest_args) = crate::builtins::try_parse_known::(args)?; + if let Some(args) = rest_args { + this.args.extend(args); + } + Ok(this) + } + async fn execute( &self, context: commands::ExecutionContext<'_>, diff --git a/brush-shell/src/args.rs b/brush-shell/src/args.rs index 98bf0d59..e5e0cd79 100644 --- a/brush-shell/src/args.rs +++ b/brush-shell/src/args.rs @@ -114,9 +114,14 @@ pub struct CommandLineArgs { pub enabled_log_events: Vec, /// Path to script to execute. + // allow any string as command_name similar to sh + #[clap(allow_hyphen_values = true)] pub script_path: Option, /// Arguments for script. + // `allow_hyphen_values`: do not strip `-` from flags + // `num_args=1..`: consume everything + #[clap(allow_hyphen_values = true, num_args=1..)] pub script_args: Vec, } diff --git a/brush-shell/src/main.rs b/brush-shell/src/main.rs index e5a4ebd7..49b9e56d 100644 --- a/brush-shell/src/main.rs +++ b/brush-shell/src/main.rs @@ -10,7 +10,6 @@ mod shell_factory; use crate::args::{CommandLineArgs, InputBackend}; use brush_interactive::InteractiveShell; -use clap::Parser; use std::{path::Path, sync::Arc}; lazy_static::lazy_static! { @@ -18,6 +17,34 @@ lazy_static::lazy_static! { Arc::new(tokio::sync::Mutex::new(None)); } +// WARN: this implementation shadows `clap::Parser::parse_from` one so it must be defined +// after the `use clap::Parser` +impl CommandLineArgs { + // Work around clap's limitation handling `--` like a regular value + // TODO: We can safely remove this `impl` after the issue is resolved + // https://github.com/clap-rs/clap/issues/5055 + // This function takes precedence over [`clap::Parser::parse_from`] + fn parse_from<'a>(itr: impl IntoIterator) -> Self { + let (mut this, script_args) = brush_core::builtins::parse_known::(itr); + // if we have `--` and unparsed raw args than + if let Some(mut args) = script_args { + // if script_path has not been parsed yet + // use the first script_args[0] (it is `--`) + // as script_path + let first = args.next(); + if this.script_path.is_none() { + this.script_path = first.cloned(); + this.script_args.extend(args.cloned()); + } else { + // if script_path already exists, everyone goes into script_args + this.script_args + .extend(first.into_iter().chain(args).cloned()); + } + } + this + } +} + /// Main entry point for the `brush` shell. fn main() { // diff --git a/brush-shell/tests/cases/arguments.yaml b/brush-shell/tests/cases/arguments.yaml new file mode 100644 index 00000000..81a86682 --- /dev/null +++ b/brush-shell/tests/cases/arguments.yaml @@ -0,0 +1,83 @@ +name: "Argument handling tests" +common_test_files: + - path: "script.sh" + contents: | + echo \"$0\" \"$1\" \"$2\" \"$@\" + +cases: + - name: "-c mode arguments without --" + args: + - "-c" + - 'echo \"$0\" \"$1\" \"$2\" \"$@\"' + - 1 + - "-2" + - "3" + + - name: "-c mode arguments with --" + args: + - "-c" + - 'echo \"$0\" \"$1\" \"$2\" \"$@\"' + - "--" + - 1 + - 2 + - 3 + + - name: "-c mode and arguments with +O" + args: + - "+O" + - "nullglob" + - "-c" + - 'echo \"$0\" \"$1\" \"$2\" \"$@\"' + - "--" + - 1 + - 2 + - 3 + + - name: "-c mode -- torture" + args: + - "-c" + - 'echo \"$0\" \"$1\" \"$2\" \"$@\"' + - -- + - -- + - -&-1 + - --! + - "\"-2\"" + - "''--''" + - 3--* + + - name: "-c modeonly one --" + args: + - "-c" + - 'echo \"$0\" \"$1\" \"$2\" \"$@\"' + - -- + + - name: "script arguments without --" + args: + - script.sh + - -1 + - -2 + - -3 + + - name: "script arguments with --" + args: + - script.sh + - -- + - --1 + - -2 + - 3 + + - name: "script -- torture" + args: + - script.sh + - -- + - "--" + - -- + - -!-1* + - "\"-2\"" + - -- + - 3-- + + - name: "script only one --" + args: + - script.sh + - -- diff --git a/brush-shell/tests/cases/builtins/echo.yaml b/brush-shell/tests/cases/builtins/echo.yaml new file mode 100644 index 00000000..6cd34152 --- /dev/null +++ b/brush-shell/tests/cases/builtins/echo.yaml @@ -0,0 +1,7 @@ +name: "Builtins: echo" +cases: + - name: "echo with only --" + stdin: echo -- + + - name: "echo with -- and args" + stdin: echo -- -1 --"aaa" ?^1as-