Subcommands for bash and fish complete properly

argh_complete/src/bash.rs

@@ -49,14 +49,7 @@ impl Generator for Bash {
 
         // Now dispatch based on the determined `cmd`
         writeln!(&mut out, "    case \"${{cmd}}\" in").unwrap();
-        generate_bash_dispatch(&mut out, cmd_name, cmd);
-        for subcmd in &cmd.commands {
-            generate_bash_dispatch(
-                &mut out,
-                &format!("{}_{}", cmd_name, subcmd.name),
-                &subcmd.command,
-            );
-        }
+        generate_all_dispatch(&mut out, cmd_name, cmd);
         writeln!(&mut out, "    esac").unwrap();
         writeln!(&mut out, "}}").unwrap();
         writeln!(&mut out).unwrap();
@@ -130,3 +123,10 @@ fn generate_bash_dispatch(out: &mut String, full_name: &str, cmd: &CommandInfoWi
 
     writeln!(out, "            ;;").unwrap();
 }
+fn generate_all_dispatch(out: &mut String, full_name: &str, cmd: &CommandInfoWithArgs<'_>) {
+    generate_bash_dispatch(out, full_name, cmd);
+    for subcmd in &cmd.commands {
+        let next_full_name = format!("{}_{}", full_name, subcmd.name);
+        generate_all_dispatch(out, &next_full_name, &subcmd.command);
+    }
+}

argh_complete/src/fish.rs

@@ -14,20 +14,7 @@ pub struct Fish;
 impl Generator for Fish {
     fn generate(cmd_name: &str, cmd: &CommandInfoWithArgs<'_>) -> String {
         let mut out = String::new();
-
-        // In Fish, `complete -c cmd_name ...` is used to register completions.
-        // We can do this recursively for subcommands, but we need to track the parent command tree.
-        // For simplicity, we'll start with a flat generation for the root command and its immediate children.
-
-        generate_fish_cmd(&mut out, cmd_name, cmd, "");
-
-        for subcmd in &cmd.commands {
-            // Fish subcommand completions usually require checking if the subcommand is the current token
-            // using `-n '__fish_seen_subcommand_from <subcmd>'` or similar.
-            let subcmd_condition = format!("-n \"__fish_seen_subcommand_from {}\"", subcmd.name);
-            generate_fish_cmd(&mut out, cmd_name, &subcmd.command, &subcmd_condition);
-        }
-
+        generate_fish_cmd(&mut out, cmd_name, cmd, &[]);
         out
     }
 }
@@ -36,16 +23,67 @@ fn generate_fish_cmd(
     out: &mut String,
     base_cmd: &str,
     cmd: &CommandInfoWithArgs<'_>,
-    condition: &str,
+    parent_subcommands: &[&str],
 ) {
+    // Condition for the current command's flags and immediate subcommands.
+    // It must be active (all parents seen) AND no children seen yet.
+    let mut conditions = Vec::new();
+
+    // 1. Requirement: All parent subcommands must be effectively "seen" (in order).
+    // actually `__fish_seen_subcommand_from` handles the check if *any* of them are seen,
+    // but for specific nesting, we usually want to say "we have seen parent X" and "we have NOT seen any child of X yet".
+
+    // For the root command, we don't need `__fish_seen_subcommand_from`.
+    // For subcommand `A`, we need `__fish_seen_subcommand_from A`.
+    // For nested `A B`, we need `__fish_seen_subcommand_from B`.
+    // The issue with `__fish_seen_subcommand_from` is it returns true if the subcommand is present *anywhere*.
+    // However, typical fish completion scripts use this pattern:
+    // `complete -c cmd -n '__fish_seen_subcommand_from sub' ...`
+    // This implies we are "in" the subcommand.
+
+    // If we have parents, the last parent must be seen.
+    if let Some(last_parent) = parent_subcommands.last() {
+        conditions.push(format!("__fish_seen_subcommand_from {}", last_parent));
+    } else {
+        // Root command: ensure NO subcommands are seen (if we want to prevent root flags from showing up in subcommands).
+        // BUT strict `not __fish_seen_subcommand_from ...` for ALL descendants is hard.
+        // Usually, we just check immediate children to distinguish "root context" from "subcommand context".
+        conditions.push("not __fish_seen_subcommand_from".to_string());
+        for sub in &cmd.commands {
+            conditions[0].push_str(&format!(" {}", sub.name));
+        }
+    }
+
+    // 2. Requirement: No immediate child of THIS command must be seen.
+    if !cmd.commands.is_empty() && !parent_subcommands.is_empty() {
+        let mut not_seen_child = String::from("not __fish_seen_subcommand_from");
+        for sub in &cmd.commands {
+            not_seen_child.push_str(&format!(" {}", sub.name));
+        }
+        conditions.push(not_seen_child);
+    }
+
+    let joined_condition = if conditions.is_empty() {
+        String::new()
+    } else {
+        let parts: Vec<String> = conditions.iter().map(|c| format!("-n '{}'", c)).collect();
+        parts.join(" ")
+    };
+
+    // If the command has no positional arguments, disable file completion.
+    let no_files = if cmd.positionals.is_empty() { " -f" } else { "" };
+
+    // Generate flags for this command
     for flag in cmd.flags {
         let mut line = format!("complete -c {}", base_cmd);
-        if !condition.is_empty() {
+        if !joined_condition.is_empty() {
             line.push(' ');
-            line.push_str(condition);
+            line.push_str(&joined_condition);
         }
 
-        // Use long syntax:
+        // Add -f if applicable
+        line.push_str(no_files);
+
         if !flag.long.is_empty() {
             let stripped_long = flag.long.trim_start_matches('-');
             if !stripped_long.is_empty() {
@@ -57,45 +95,38 @@ fn generate_fish_cmd(
             line.push_str(&format!(" -s {}", short));
         }
 
-        if let FlagInfoKind::Option { arg_name } = flag.kind {
-            // Options usually take arguments, so we add `-r` (requires argument) and possibly `-d` (description)
+        if let FlagInfoKind::Option { .. } = flag.kind {
             line.push_str(" -r");
-            let _ = arg_name; // Maybe use argument name in description
         }
 
         if !flag.description.is_empty() {
-            // escape single quotes in description
             let description = flag.description.replace("'", "\\'");
             line.push_str(&format!(" -d '{}'", description));
         }
 
         writeln!(out, "{}", line).unwrap();
     }
 
-    // Subcommands themselves are completions.
+    // Generate immediate subcommands (as arguments to this command)
     for subcmd in &cmd.commands {
         let mut line = format!("complete -c {}", base_cmd);
-        // Note: Fish typically handles subcommands by defining them as arguments that can be taken without a flag
-        // `-f` means no file completion, `-a` means an argument.
-        if !condition.is_empty() {
-            line.push_str(&format!(" {} -f -a '{}'", condition, subcmd.name));
-        } else {
-            // Check if we are at the top level, we might want to ensure no subcommand has been seen yet.
-            line.push_str(&format!(
-                " -n \"not __fish_seen_subcommand_from ...\" -f -a '{}'",
-                subcmd.name
-            ));
-            // A more robust way in Fish is custom conditions, but here's a simpler one:
-            // Just complete the subcommand if it's not starting with a dash.
-            // Often we define a fish function `__fish_cmd_needs_command` and use it.
-            // For now, let's just register it as a top level argument `-a subcmd.name`
-            line = format!(
-                "complete -c {} -f -a '{}' -d '{}'",
-                base_cmd,
-                subcmd.name,
-                subcmd.command.description.replace("'", "\\'")
-            );
+        if !joined_condition.is_empty() {
+            line.push(' ');
+            line.push_str(&joined_condition);
         }
+        // Subcommands are just arguments that don't take files
+        line.push_str(&format!(
+            " -f -a '{}' -d '{}'",
+            subcmd.name,
+            subcmd.command.description.replace("'", "\\'")
+        ));
         writeln!(out, "{}", line).unwrap();
     }
+
+    // Recurse
+    for subcmd in &cmd.commands {
+        let mut new_parents = parent_subcommands.to_vec();
+        new_parents.push(subcmd.name);
+        generate_fish_cmd(out, base_cmd, &subcmd.command, &new_parents);
+    }
 }

argh_complete/src/tests.rs

@@ -52,6 +52,6 @@ fn test_fish_generator() {
     let cmd = make_mock_command();
     let fish_out = crate::fish::Fish::generate("mycmd", &cmd);
 
-    assert!(fish_out.contains("complete -c mycmd -l verbose -s v -d 'verbose output'"));
-    assert!(fish_out.contains("complete -c mycmd -f -a 'subcmd' -d 'a sub command'"));
+    assert!(fish_out.contains("complete -c mycmd -n 'not __fish_seen_subcommand_from subcmd' -f -l verbose -s v -d 'verbose output'"));
+    assert!(fish_out.contains("complete -c mycmd -n 'not __fish_seen_subcommand_from subcmd' -f -a 'subcmd' -d 'a sub command'"));
 }