Fix generated shell completion script handling of repeating & non-repeating positional arguments, flags & options

Sources/ArgumentParser/Completions/BashCompletionsGenerator.swift

@@ -48,15 +48,17 @@ extension CommandInfoV0 {
     #
     # required variables:
     #
-    # - flags: the flags that the current (sub)command can accept
-    # - options: the options that the current (sub)command can accept
+    # - repeating_flags: the repeating flags that the current (sub)command can accept
+    # - non_repeating_flags: the non-repeating flags that the current (sub)command can accept
+    # - repeating_options: the repeating options that the current (sub)command can accept
+    # - non_repeating_options: the non-repeating options that the current (sub)command can accept
     # - positional_number: value ignored
     # - unparsed_words: unparsed words from the current command line
     #
     # modified variables:
     #
-    # - flags: remove flags for this (sub)command that are already on the command line
-    # - options: remove options for this (sub)command that are already on the command line
+    # - non_repeating_flags: remove flags for this (sub)command that are already on the command line
+    # - non_repeating_options: remove options for this (sub)command that are already on the command line
     # - positional_number: set to the current positional number
     # - unparsed_words: remove all flags, options, and option values for this (sub)command
     \(offerFlagsOptionsFunctionName)() {
@@ -93,26 +95,26 @@ extension CommandInfoV0 {
                     # ${word} is a flag or an option
                     # If ${word} is an option, mark that the next word to be parsed is an option value
                     local option
-                    for option in "${options[@]}"; do
+                    for option in "${repeating_options[@]}" "${non_repeating_options[@]}"; do
                         [[ "${word}" = "${option}" ]] && is_parsing_option_value=true && break
                     done
 
-                    # Remove ${word} from ${flags} or ${options} so it isn't offered again
+                    # Remove ${word} from ${non_repeating_flags} or ${non_repeating_options} so it isn't offered again
                     local not_found=true
                     local -i index
-                    for index in "${!flags[@]}"; do
-                        if [[ "${flags[${index}]}" = "${word}" ]]; then
-                            unset "flags[${index}]"
-                            flags=("${flags[@]}")
+                    for index in "${!non_repeating_flags[@]}"; do
+                        if [[ "${non_repeating_flags[${index}]}" = "${word}" ]]; then
+                            unset "non_repeating_flags[${index}]"
+                            non_repeating_flags=("${non_repeating_flags[@]}")
                             not_found=false
                             break
                         fi
                     done
                     if "${not_found}"; then
-                        for index in "${!options[@]}"; do
-                            if [[ "${options[${index}]}" = "${word}" ]]; then
-                                unset "options[${index}]"
-                                options=("${options[@]}")
+                        for index in "${!non_repeating_flags[@]}"; do
+                            if [[ "${non_repeating_flags[${index}]}" = "${word}" ]]; then
+                                unset "non_repeating_flags[${index}]"
+                                non_repeating_flags=("${non_repeating_flags[@]}")
                                 break
                             fi
                         done
@@ -124,7 +126,7 @@ extension CommandInfoV0 {
             fi
 
             # ${word} is neither a flag, nor an option, nor an option value
-            if [[ "${positional_number}" -lt "${positional_count}" ]]; then
+            if [[ "${positional_number}" -lt "${positional_count}" || "${positional_count}" -lt 0 ]]; then
                 # ${word} is a positional
                 ((positional_number++))
                 unset "unparsed_words[${word_index}]"
@@ -147,7 +149,7 @@ extension CommandInfoV0 {
             && ! "${is_parsing_option_value}"\\
             && [[ ("${cur}" = -* && "${positional_number}" -ge 0) || "${positional_number}" -eq -1 ]]
         then
-            COMPREPLY+=($(compgen -W "${flags[*]} ${options[*]}" -- "${cur}"))
+            COMPREPLY+=($(compgen -W "${repeating_flags[*]} ${non_repeating_flags[*]} ${repeating_options[*]} ${non_repeating_options[*]}" -- "${cur}"))
         fi
     }
 
@@ -211,26 +213,33 @@ extension CommandInfoV0 {
 
     let positionalArguments = positionalArguments
 
-    let flagCompletions = flagCompletions
-    let optionCompletions = optionCompletions
-    if !flagCompletions.isEmpty || !optionCompletions.isEmpty {
+    let arguments = arguments ?? []
+
+    let flags = arguments.filter { $0.kind == .flag }
+    let options = arguments.filter { $0.kind == .option }
+    if !flags.flatMap(\.completionWords).isEmpty
+      || !options.flatMap(\.completionWords).isEmpty
+    {
       result += """
-            \(declareTopLevelArray)flags=(\(flagCompletions.joined(separator: " ")))
-            \(declareTopLevelArray)options=(\(optionCompletions.joined(separator: " ")))
-            \(offerFlagsOptionsFunctionName) \(positionalArguments.count)
+            \(declareTopLevelArray)repeating_flags=(\(flags.filter(\.isRepeating).flatMap(\.completionWords).joined(separator: " ")))
+            \(declareTopLevelArray)non_repeating_flags=(\(flags.filter { !$0.isRepeating }.flatMap(\.completionWords).joined(separator: " ")))
+            \(declareTopLevelArray)repeating_options=(\(options.filter(\.isRepeating).flatMap(\.completionWords).joined(separator: " ")))
+            \(declareTopLevelArray)non_repeating_options=(\(options.filter { !$0.isRepeating }.flatMap(\.completionWords).joined(separator: " ")))
+            \(offerFlagsOptionsFunctionName) \
+        \(positionalArguments.contains { $0.isRepeating } ? -1 : positionalArguments.count)
 
         """
     }
 
     // Generate the case pattern-matching statements for option values.
     // If there aren't any, skip the case block altogether.
     let optionHandlers =
-      (arguments ?? []).compactMap { arg in
+      options.compactMap { arg in
         guard arg.kind != .flag else { return nil }
         let words = arg.completionWords
         guard !words.isEmpty else { return nil }
         return """
-              \(arg.completionWords.map { "'\($0.shellEscapeForSingleQuotedString())'" }.joined(separator: "|")))
+              \(words.map { "'\($0.shellEscapeForSingleQuotedString())'" }.joined(separator: "|")))
           \(valueCompletion(arg).indentingEachLine(by: 8))\
                   return
                   ;;
@@ -248,14 +257,23 @@ extension CommandInfoV0 {
         """
     }
 
+    var encounteredRepeatingPositional = false
     let positionalCases =
       zip(1..., positionalArguments)
       .compactMap { position, arg in
+        guard !encounteredRepeatingPositional else {
+          return nil as String?
+        }
+
+        if arg.isRepeating {
+          encounteredRepeatingPositional = true
+        }
+
         let completion = valueCompletion(arg)
         return completion.isEmpty
           ? nil
           : """
-              \(position))
+              \(encounteredRepeatingPositional ? "*" : position.description))
           \(completion.indentingEachLine(by: 8))\
                   return
                   ;;
@@ -310,30 +328,6 @@ extension CommandInfoV0 {
       """
   }
 
-  /// Returns flag completions.
-  private var flagCompletions: [String] {
-    (arguments ?? []).flatMap {
-      switch $0.kind {
-      case .flag:
-        return $0.completionWords
-      default:
-        return []
-      }
-    }
-  }
-
-  /// Returns option completions.
-  private var optionCompletions: [String] {
-    (arguments ?? []).flatMap {
-      switch $0.kind {
-      case .option:
-        return $0.completionWords
-      default:
-        return []
-      }
-    }
-  }
-
   /// Returns the completions that can follow the given argument's `--name`.
   private func valueCompletion(_ arg: ArgumentInfoV0) -> String {
     switch arg.completionKind {
@@ -375,7 +369,6 @@ extension CommandInfoV0 {
         """
 
     case .custom, .customAsync:
-      // Generate a call back into the command to retrieve a completions list
       return """
         \(addCompletionsFunctionName) -W\
          "$(\(customCompleteFunctionName) \(arg.commonCustomCompletionCall(command: self))\
@@ -385,7 +378,6 @@ extension CommandInfoV0 {
         """
 
     case .customDeprecated:
-      // Generate a call back into the command to retrieve a completions list
       return """
         \(addCompletionsFunctionName) -W\
          "$(\(customCompleteFunctionName) \(arg.commonCustomCompletionCall(command: self)))"

Sources/ArgumentParser/Completions/FishCompletionsGenerator.swift

@@ -24,16 +24,36 @@ extension ToolInfoV0 {
 extension CommandInfoV0 {
   fileprivate var fishCompletionScript: String {
     """
-    function \(shouldOfferCompletionsForFunctionName) -a expected_commands -a expected_positional_index
-        set -l unparsed_tokens (\(tokensFunctionName) -pc)
+    function \(shouldOfferCompletionsForFlagsOrOptionsFunctionName) -a expected_commands
+        set -l non_repeating_flags_or_options $argv[2..]
+
+        set -l non_repeating_flags_or_options_absent 0
         set -l positional_index 0
         set -l commands
+        \(parseTokensFunctionName)
+        test "$commands" = "$expected_commands"; and return $non_repeating_flags_or_options_absent
+    end
+
+    function \(shouldOfferCompletionsForPositionalFunctionName) -a expected_commands expected_positional_index positional_index_comparison
+        if test -z $positional_index_comparison
+            set positional_index_comparison -eq
+        end
+
+        set -l non_repeating_flags_or_options
+        set -l non_repeating_flags_or_options_absent 0
+        set -l positional_index 0
+        set -l commands
+        \(parseTokensFunctionName)
+        test "$commands" = "$expected_commands" -a \\( "$positional_index" "$positional_index_comparison" "$expected_positional_index" \\)
+    end
+
+    function \(parseTokensFunctionName) -S
+        set -l unparsed_tokens (\(tokensFunctionName) -pc)
+        set -l present_flags_and_options
 
         switch $unparsed_tokens[1]
     \(commandCases)
         end
-
-        test "$commands" = "$expected_commands" -a \\( -z "$expected_positional_index" -o "$expected_positional_index" -eq "$positional_index" \\)
     end
 
     function \(tokensFunctionName)
@@ -44,9 +64,8 @@ extension CommandInfoV0 {
         end
     end
 
-    function \(parseSubcommandFunctionName) -S
+    function \(parseSubcommandFunctionName) -S -a positional_count
         argparse -s r -- $argv
-        set -l positional_count $argv[1]
         set -l option_specs $argv[2..]
 
         set -a commands $unparsed_tokens[1]
@@ -58,8 +77,16 @@ extension CommandInfoV0 {
             argparse -sn "$commands" $option_specs -- $unparsed_tokens 2> /dev/null
             set unparsed_tokens $argv
             set positional_index (math $positional_index + 1)
+
+            for non_repeating_flag_or_option in $non_repeating_flags_or_options
+                if set -ql _flag_$non_repeating_flag_or_option
+                    set non_repeating_flags_or_options_absent 1
+                    break
+                end
+            end
+
             if test (count $unparsed_tokens) -eq 0 -o \\( -z "$_flag_r" -a "$positional_index" -gt "$positional_count" \\)
-                return 0
+                break
             end
             set -e unparsed_tokens[1]
         end
@@ -113,39 +140,52 @@ extension CommandInfoV0 {
   }
 
   private var completions: [String] {
-    let prefix = """
-      complete -c '\(initialCommand)'\
-       -n '\(shouldOfferCompletionsForFunctionName)\
-       "\(commandContext.joined(separator: separator))"
-      """
+    let prefix = "complete -c '\(initialCommand)' -n '"
 
     let subcommands = (subcommands ?? []).filter(\.shouldDisplay)
 
     var positionalIndex = 0
 
+    var repeatingPositionalComparison = ""
     let argumentCompletions =
       completableArguments
-      .map { arg in
-        """
-        \(prefix)\(
-          arg.kind == .positional
-          ? """
-          \({
-            positionalIndex += 1
-            return " \(positionalIndex)"
-          }())
+      .compactMap { arg in
+        if arg.kind == .positional {
+          guard repeatingPositionalComparison.isEmpty else {
+            return nil as String?
+          }
+
+          if arg.isRepeating {
+            repeatingPositionalComparison = " -ge"
+          }
+        }
+
+        return """
+          \(prefix)\(
+            arg.kind == .positional
+            ? """
+            \(shouldOfferCompletionsForPositionalFunctionName) "\(commandContext.joined(separator: separator))" \({
+              positionalIndex += 1
+              return "\(positionalIndex)\(repeatingPositionalComparison)"
+            }())
+            """
+            : """
+              \(shouldOfferCompletionsForFlagsOrOptionsFunctionName) "\(commandContext.joined(separator: separator))"\
+              \((arg.isRepeating ? [] : arg.names ?? []).map { " \($0.name)" }.sorted().joined())
+              """
+          )' \(argumentSegments(arg).joined(separator: separator))
           """
-          : ""
-        )' \(argumentSegments(arg).joined(separator: separator))
-        """
       }
 
     positionalIndex += 1
 
     return
       argumentCompletions
       + subcommands.map {
-        "\(prefix) \(positionalIndex)' -fa '\($0.commandName)' -d '\($0.abstract?.fishEscapeForSingleQuotedString() ?? "")'"
+        """
+        \(prefix)\(shouldOfferCompletionsForPositionalFunctionName) "\(commandContext.joined(separator: separator))"\
+         \(positionalIndex)' -fa '\($0.commandName)' -d '\($0.abstract?.fishEscapeForSingleQuotedString() ?? "")'
+        """
       }
       + subcommands.flatMap(\.completions)
   }
@@ -247,8 +287,16 @@ extension CommandInfoV0 {
       """
   }
 
-  private var shouldOfferCompletionsForFunctionName: String {
-    "\(completionFunctionPrefix)_should_offer_completions_for"
+  private var shouldOfferCompletionsForFlagsOrOptionsFunctionName: String {
+    "\(completionFunctionPrefix)_should_offer_completions_for_flags_or_options"
+  }
+
+  private var shouldOfferCompletionsForPositionalFunctionName: String {
+    "\(completionFunctionPrefix)_should_offer_completions_for_positional"
+  }
+
+  private var parseTokensFunctionName: String {
+    "\(completionFunctionPrefix)_parse_tokens"
   }
 
   private var tokensFunctionName: String {
@@ -289,7 +337,7 @@ extension ArgumentInfoV0 {
   private func optionSpecRequiresValue(_ optionSpec: String) -> String {
     switch kind {
     case .option:
-      return "\(optionSpec)="
+      return "\(optionSpec)=\(isRepeating ? "+" : "")"
     default:
       return optionSpec
     }