feat: helper functions to deal with clap's limitation handling --

- Added functions `try_parse_known` and `parse_known` for splitting arguments into `before hyphen` and `hyphen and after hyphen`
- This approach allows manually deciding what to do with `--` and the remaining arguments

Fixes:
- main CommandLineArgs
- EchoCommand

Clap issue: clap-rs/clap#5055

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<String>,
+///    }
+///
+///    let (mut parsed_args, raw_args) =
+///        brush_core::builtins::parse_known::<CommandLineArgs, _>(std::env::args());
+///    if raw_args.is_some() {
+///        parsed_args.script_args = raw_args.unwrap().collect();
+///    }
+/// ```
+pub fn parse_known<T: Parser, S>(
+    args: impl IntoIterator<Item = S>,
+) -> (T, Option<impl Iterator<Item = S>>)
+where
+    S: Into<std::ffi::OsString> + 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<T: Parser>(
+    args: impl IntoIterator<Item = String>,
+) -> Result<(T, Option<impl Iterator<Item = String>>), 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))
+}

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<I>(args: I) -> Result<Self, clap::Error>
+    where
+        I: IntoIterator<Item = String>,
+    {
+        let (mut this, rest_args) = crate::builtins::try_parse_known::<EchoCommand>(args)?;
+        rest_args.map(|args| {
+            this.args.extend(args);
+        });
+        Ok(this)
+    }
+
     async fn execute(
         &self,
         context: commands::ExecutionContext<'_>,

brush-shell/src/main.rs

@@ -121,6 +121,32 @@ impl CommandLineArgs {
     }
 }
 
+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<Item = &'a String>) -> Self {
+        let (mut this, script_args) = brush_core::builtins::parse_known::<CommandLineArgs, _>(itr);
+        // if we have `--` and unparsed raw args than
+        script_args.map(|mut 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
+    }
+}
+
 lazy_static::lazy_static! {
     static ref TRACE_EVENT_CONFIG: Arc<tokio::sync::Mutex<Option<events::TraceEventConfig>>> =
         Arc::new(tokio::sync::Mutex::new(None));
@@ -140,33 +166,7 @@ fn main() {
         }
     }
 
-    // clap always interprets `--` as a separator and not as an argument
-    // See: clap-rs/clap/clap_builder/src/parser/parser.rs `if arg_os.is_escape() { ...; continue } // means if arg_os == '--' {}`
-    // but sh can handle `--` just fine
-    // so we split [..., 'arg1', '--', 'arg2', ...] to [..., 'arg1'] for processing in CommandLineArgs::parse_from
-    // and raw script_args ['--', 'arg2', ...]
-    let parsed_args = {
-        // simulate take_until and split the iter by "--"
-        let hyphen_pos = args.iter().position(|arg| arg == "--");
-        let mut iter = args.iter();
-        let mut parsed_args =
-            CommandLineArgs::parse_from(iter.by_ref().take(hyphen_pos.unwrap_or(args.len())));
-        // if the iter was split
-        if hyphen_pos.is_some() {
-            // if script_path has not been parsed yet
-            // use the first script_args[0] as script_path (`command_name` in sh) $0
-            // Posix: sh -c command_string [**command_name** [argument...]]
-            // See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sh.html
-            let first = iter.next().cloned();
-            if parsed_args.script_path.is_none() {
-                parsed_args.script_path = first;
-                parsed_args.script_args = iter.cloned().collect();
-            } else {
-                parsed_args.script_args = first.into_iter().chain(iter.cloned()).collect();
-            }
-        }
-        parsed_args
-    };
+    let parsed_args = CommandLineArgs::parse_from(&args);
 
     //
     // Run.

brush-shell/tests/cases/arguments.yaml

@@ -40,11 +40,17 @@ cases:
       - --
       - --
       - -&-1
-      - --
-      - -2!
-      - --
+      - --!
+      - "\"-2\""
+      - "''--''"
       - 3--*
 
+  - name: "-c modeonly one --"
+    args:
+      - "-c"
+      - 'echo \"$0\" \"$1\" \"$2\" \"$@\"'
+      - --
+
   - name: "script arguments without --"
     args:
       - script.sh
@@ -67,6 +73,11 @@ cases:
       - "--"
       - --
       - -!-1*
-      - -2
+      - "\"-2\""
       - --
       - 3--
+
+  - name: "script only one --"
+    args:
+      - script.sh
+      - --

brush-shell/tests/cases/builtins/echo.yaml

@@ -0,0 +1,4 @@
+name: "Builtins: echo"
+cases:
+  - name: "echo with --"
+    stdin: echo --