From 1a780dee6084067b5aca18a1dd0d7fee0918dc32 Mon Sep 17 00:00:00 2001
From: reuben olinsky <reubeno@users.noreply.github.com>
Date: Wed, 22 Jan 2025 12:59:00 -0800
Subject: [PATCH] fix(builtins): fix set builtin handling of - and -- (#354)

---
 brush-core/src/builtins/set.rs            | 56 +++++++++++++++++++----
 brush-shell/tests/cases/builtins/set.yaml | 35 ++++++++++++++
 2 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/brush-core/src/builtins/set.rs b/brush-core/src/builtins/set.rs
index c28c758f..5aa76cee 100644
--- a/brush-core/src/builtins/set.rs
+++ b/brush-core/src/builtins/set.rs
@@ -129,6 +129,7 @@ pub(crate) struct SetCommand {
     #[clap(flatten)]
     set_option: SetOption,
 
+    #[arg(trailing_var_arg = true, allow_hyphen_values = true)]
     positional_args: Vec<String>,
 }
 
@@ -137,6 +138,35 @@ impl builtins::Command for SetCommand {
         true
     }
 
+    /// Override the default [`builtins::Command::new`] function to handle clap's limitation related
+    /// to `--`. See [`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>,
+    {
+        //
+        // TODO: This is getting pretty messy; we need to see how to avoid this -- handling from
+        // leaking into too many commands' custom parsing.
+        //
+
+        // Apply the same workaround from the default implementation of Command::new to handle +
+        // args.
+        let args = args.into_iter().map(|arg| {
+            if arg.starts_with('+') {
+                format!("--{arg}")
+            } else {
+                arg
+            }
+        });
+
+        let (mut this, rest_args) = crate::builtins::try_parse_known::<SetCommand>(args)?;
+        if let Some(args) = rest_args {
+            this.positional_args.extend(args);
+        }
+        Ok(this)
+    }
+
     #[allow(clippy::too_many_lines)]
     async fn execute(
         &self,
@@ -308,16 +338,26 @@ impl builtins::Command for SetCommand {
             }
         }
 
-        for (i, arg) in self.positional_args.iter().enumerate() {
-            if arg == "-" && i == 0 {
-                continue;
+        let skip = match self.positional_args.first() {
+            Some(x) if x == "-" => {
+                if self.positional_args.len() > 1 {
+                    context.shell.positional_parameters.clear();
+                }
+                1
             }
-
-            if i < context.shell.positional_parameters.len() {
-                arg.clone_into(&mut context.shell.positional_parameters[i]);
-            } else {
-                context.shell.positional_parameters.push(arg.to_owned());
+            Some(x) if x == "--" => {
+                context.shell.positional_parameters.clear();
+                1
             }
+            Some(_) => {
+                context.shell.positional_parameters.clear();
+                0
+            }
+            None => 0,
+        };
+
+        for arg in self.positional_args.iter().skip(skip) {
+            context.shell.positional_parameters.push(arg.to_owned());
         }
 
         saw_option = saw_option || !self.positional_args.is_empty();
diff --git a/brush-shell/tests/cases/builtins/set.yaml b/brush-shell/tests/cases/builtins/set.yaml
index 74a54e12..9668b6f2 100644
--- a/brush-shell/tests/cases/builtins/set.yaml
+++ b/brush-shell/tests/cases/builtins/set.yaml
@@ -1,6 +1,41 @@
 name: "Builtins: set"
 cases:
+  - name: "set with no args"
+    stdin: |
+      MYVARIABLE=VALUE
+      set > set-output.txt
+      grep MYVARIABLE set-output.txt
+
+      # Remove set-output.txt to avoid it being byte-for-byte compared.
+      rm set-output.txt
+
   - name: "Basic set usage"
     stdin: |
       set a b c d
       echo ${*}
+
+  - name: "set clearing args"
+    stdin: |
+      set a b c
+      echo ${*}
+      set a
+      echo ${*}
+
+  - name: "set with -"
+    stdin: |
+      set - a b c
+      echo "args: " ${*}
+      set -
+      echo "args: " ${*}
+
+  - name: "set with --"
+    stdin: |
+      set -- a b c
+      echo "args: " ${*}
+      set --
+      echo "args: " ${*}
+
+  - name: "set with option-looking tokens"
+    stdin: |
+      set a -v
+      echo ${*}