From 2aec66f9ee9ceabdfbb2e17c5071b7d1e5be7bdb Mon Sep 17 00:00:00 2001
From: Mykhailo Chalyi <mike@chaliy.name>
Date: Thu, 2 Apr 2026 05:58:29 +0000
Subject: [PATCH] fix(builtins): support long options in tree builtin

Add --noreport long option parsing before the short-flag character loop.
Previously, --noreport was parsed as combined short flags, causing an
invalid option error on the second '-'.

Closes #949
---
 crates/bashkit/src/builtins/tree.rs           | 80 ++++++++++++++-----
 .../tests/spec_cases/bash/tree.test.sh        | 11 +++
 2 files changed, 71 insertions(+), 20 deletions(-)
 create mode 100644 crates/bashkit/tests/spec_cases/bash/tree.test.sh

diff --git a/crates/bashkit/src/builtins/tree.rs b/crates/bashkit/src/builtins/tree.rs
index 7b188129..1f607f4f 100644
--- a/crates/bashkit/src/builtins/tree.rs
+++ b/crates/bashkit/src/builtins/tree.rs
@@ -9,13 +9,14 @@ use crate::interpreter::ExecResult;
 
 /// The tree builtin command.
 ///
-/// Usage: tree [-a] [-d] [-L level] [-I pattern] [PATH...]
+/// Usage: tree [-a] [-d] [-L level] [-I pattern] [--noreport] [PATH...]
 ///
 /// Options:
-///   -a          Show hidden files
-///   -d          Directories only
-///   -L level    Limit depth to level
-///   -I pattern  Exclude files matching pattern
+///   -a            Show hidden files
+///   -d            Directories only
+///   -L level      Limit depth to level
+///   -I pattern    Exclude files matching pattern
+///   --noreport    Suppress directory/file count report
 pub struct Tree;
 
 struct TreeOptions {
@@ -23,6 +24,7 @@ struct TreeOptions {
     dirs_only: bool,
     max_depth: Option<usize>,
     exclude_pattern: Option<String>,
+    noreport: bool,
 }
 
 struct TreeCounts {
@@ -38,6 +40,7 @@ impl Builtin for Tree {
             dirs_only: false,
             max_depth: None,
             exclude_pattern: None,
+            noreport: false,
         };
 
         let mut paths: Vec<&str> = Vec::new();
@@ -60,11 +63,25 @@ impl Builtin for Tree {
             } else if let Some(val) = p.flag_value_opt("-I") {
                 opts.exclude_pattern = Some(val.to_string());
             } else if p.is_flag() {
-                // Handle combined flags like -ad
                 let Some(s) = p.current() else {
                     p.advance();
                     continue;
                 };
+                // Handle long options (--foo) before short-flag loop
+                if s.starts_with("--") {
+                    match s {
+                        "--noreport" => opts.noreport = true,
+                        _ => {
+                            return Ok(ExecResult::err(
+                                format!("tree: unrecognized option '{}'\n", s),
+                                1,
+                            ));
+                        }
+                    }
+                    p.advance();
+                    continue;
+                }
+                // Handle combined short flags like -ad
                 for ch in s[1..].chars() {
                     match ch {
                         'a' => opts.show_hidden = true,
@@ -108,20 +125,22 @@ impl Builtin for Tree {
             let mut counts = TreeCounts { dirs: 0, files: 0 };
             build_tree(&ctx, &root, "", &opts, 0, &mut counts, &mut output).await;
 
-            if opts.dirs_only {
-                output.push_str(&format!(
-                    "\n{} director{}\n",
-                    counts.dirs,
-                    if counts.dirs == 1 { "y" } else { "ies" }
-                ));
-            } else {
-                output.push_str(&format!(
-                    "\n{} director{}, {} file{}\n",
-                    counts.dirs,
-                    if counts.dirs == 1 { "y" } else { "ies" },
-                    counts.files,
-                    if counts.files == 1 { "" } else { "s" }
-                ));
+            if !opts.noreport {
+                if opts.dirs_only {
+                    output.push_str(&format!(
+                        "\n{} director{}\n",
+                        counts.dirs,
+                        if counts.dirs == 1 { "y" } else { "ies" }
+                    ));
+                } else {
+                    output.push_str(&format!(
+                        "\n{} director{}, {} file{}\n",
+                        counts.dirs,
+                        if counts.dirs == 1 { "y" } else { "ies" },
+                        counts.files,
+                        if counts.files == 1 { "" } else { "s" }
+                    ));
+                }
             }
         }
 
@@ -381,4 +400,25 @@ mod tests {
         assert_eq!(result.exit_code, 1);
         assert!(result.stderr.contains("invalid option"));
     }
+
+    #[tokio::test]
+    async fn test_tree_noreport() {
+        let fs = setup_fs().await;
+        let result = run_tree(&["--noreport", "/project"], fs).await;
+        assert_eq!(result.exit_code, 0);
+        assert!(result.stdout.contains("/project"));
+        assert!(result.stdout.contains("src"));
+        assert!(result.stdout.contains("Cargo.toml"));
+        // --noreport should suppress the summary line
+        assert!(!result.stdout.contains("director"));
+        assert!(!result.stdout.contains("file"));
+    }
+
+    #[tokio::test]
+    async fn test_tree_unknown_long_option() {
+        let fs = Arc::new(InMemoryFs::new()) as Arc<dyn FileSystem>;
+        let result = run_tree(&["--bogus"], fs).await;
+        assert_eq!(result.exit_code, 1);
+        assert!(result.stderr.contains("unrecognized option"));
+    }
 }
diff --git a/crates/bashkit/tests/spec_cases/bash/tree.test.sh b/crates/bashkit/tests/spec_cases/bash/tree.test.sh
new file mode 100644
index 00000000..ab2f5451
--- /dev/null
+++ b/crates/bashkit/tests/spec_cases/bash/tree.test.sh
@@ -0,0 +1,11 @@
+### tree_noreport
+### bash_diff: tree --noreport is bashkit builtin
+# tree --noreport should suppress the report line
+mkdir -p /tmp/tree_nr/a
+touch /tmp/tree_nr/a/f.txt
+tree --noreport /tmp/tree_nr
+### expect
+/tmp/tree_nr
+└── a
+    └── f.txt
+### end