From 90ac0ef66e3458dd022b78825637dd0d927e36ec Mon Sep 17 00:00:00 2001
From: 百地 希留耶 <65301509+KiruyaMomochi@users.noreply.github.com>
Date: Sat, 22 Jul 2023 21:55:12 +0800
Subject: Implement fish completion (#29549)

* commands: provide more information to Command

* fish: Add script to generate fish completion

* fish: auto prepend `spack` command to avoid duplication

* fish: impove completion generation code readability

* commands: replace match-case with if-else

* fish: fix optspec variable name prefix

* fish: fix return value in get_optspecs

* fish: fix return value in get_optspecs

* format: split long line and trim trailing space

* bugfix: replace f-string with interpolation

* fish: compete more specs and some fixes

* fish: complete hash spec starts with /

* fish: improve compatibility

* style: trim trailing whitespace

* commands: add fish to update args and update tests

* commands: add fish completion file

* style: merge imports

* fish: source completion in setup-env

* fish: caret only completes dependencies

* fish: make sure we always get same order of output

* fish: spack activate
only show installed packages that have extensions

* fish: update completion file

* fish: make dict keys sorted

* Blacken code

* Fix bad merge

* Undo style changes to setup-env.fish

* Fix unit tests

* Style fix

* Compatible with fish_indent

* Use list for stability of order

* Sort one more place

* Sort more things

* Sorting unneeded

* Unsort

* Print difference

* Style fix

* Help messages need quotes

* Arguments to -a must be quoted

* Update types

* Update types

* Update types

* Add type hints

* Change order of positionals

* Always expand help

* Remove shared base class

* Fix type hints

* Remove platform-specific choices

* First line of help only

* Remove unused maps

* Remove suppress

* Remove debugging comments

* Better quoting

* Fish completions have no double dash

* Remove test for deleted class

* Fix grammar in header file

* Use single quotes in most places

* Better support for remainder nargs

* No magic strings

* * and + can also complete multiple

* lower case, no period

---------

Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
---
 lib/spack/llnl/util/argparsewriter.py            | 167 +++-----
 lib/spack/spack/cmd/commands.py                  | 472 ++++++++++++++++++++++-
 lib/spack/spack/cmd/mirror.py                    |   4 +-
 lib/spack/spack/test/cmd/commands.py             |  75 +++-
 lib/spack/spack/test/llnl/util/argparsewriter.py |  10 -
 5 files changed, 563 insertions(+), 165 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/llnl/util/argparsewriter.py b/lib/spack/llnl/util/argparsewriter.py
index dfd602bb34..60a404abd4 100644
--- a/lib/spack/llnl/util/argparsewriter.py
+++ b/lib/spack/llnl/util/argparsewriter.py
@@ -9,7 +9,7 @@ import io
 import re
 import sys
 from argparse import ArgumentParser
-from typing import IO, Optional, Sequence, Tuple
+from typing import IO, Any, Iterable, List, Optional, Sequence, Tuple, Union
 
 
 class Command:
@@ -25,9 +25,9 @@ class Command:
         prog: str,
         description: Optional[str],
         usage: str,
-        positionals: Sequence[Tuple[str, str]],
-        optionals: Sequence[Tuple[Sequence[str], str, str]],
-        subcommands: Sequence[Tuple[ArgumentParser, str]],
+        positionals: List[Tuple[str, Optional[Iterable[Any]], Union[int, str, None], str]],
+        optionals: List[Tuple[Sequence[str], List[str], str, Union[int, str, None], str]],
+        subcommands: List[Tuple[ArgumentParser, str, str]],
     ) -> None:
         """Initialize a new Command instance.
 
@@ -96,13 +96,30 @@ class ArgparseWriter(argparse.HelpFormatter, abc.ABC):
             if action.option_strings:
                 flags = action.option_strings
                 dest_flags = fmt._format_action_invocation(action)
-                help = self._expand_help(action) if action.help else ""
-                help = help.replace("\n", " ")
-                optionals.append((flags, dest_flags, help))
+                nargs = action.nargs
+                help = (
+                    self._expand_help(action)
+                    if action.help and action.help != argparse.SUPPRESS
+                    else ""
+                )
+                help = help.split("\n")[0]
+
+                if action.choices is not None:
+                    dest = [str(choice) for choice in action.choices]
+                else:
+                    dest = [action.dest]
+
+                optionals.append((flags, dest, dest_flags, nargs, help))
             elif isinstance(action, argparse._SubParsersAction):
                 for subaction in action._choices_actions:
                     subparser = action._name_parser_map[subaction.dest]
-                    subcommands.append((subparser, subaction.dest))
+                    help = (
+                        self._expand_help(subaction)
+                        if subaction.help and action.help != argparse.SUPPRESS
+                        else ""
+                    )
+                    help = help.split("\n")[0]
+                    subcommands.append((subparser, subaction.dest, help))
 
                     # Look for aliases of the form 'name (alias, ...)'
                     if self.aliases and isinstance(subaction.metavar, str):
@@ -111,12 +128,22 @@ class ArgparseWriter(argparse.HelpFormatter, abc.ABC):
                             aliases = match.group(2).split(", ")
                             for alias in aliases:
                                 subparser = action._name_parser_map[alias]
-                                subcommands.append((subparser, alias))
+                                help = (
+                                    self._expand_help(subaction)
+                                    if subaction.help and action.help != argparse.SUPPRESS
+                                    else ""
+                                )
+                                help = help.split("\n")[0]
+                                subcommands.append((subparser, alias, help))
             else:
                 args = fmt._format_action_invocation(action)
-                help = self._expand_help(action) if action.help else ""
-                help = help.replace("\n", " ")
-                positionals.append((args, help))
+                help = (
+                    self._expand_help(action)
+                    if action.help and action.help != argparse.SUPPRESS
+                    else ""
+                )
+                help = help.split("\n")[0]
+                positionals.append((args, action.choices, action.nargs, help))
 
         return Command(prog, description, usage, positionals, optionals, subcommands)
 
@@ -146,7 +173,7 @@ class ArgparseWriter(argparse.HelpFormatter, abc.ABC):
         cmd = self.parse(parser, prog)
         self.out.write(self.format(cmd))
 
-        for subparser, prog in cmd.subcommands:
+        for subparser, prog, help in cmd.subcommands:
             self._write(subparser, prog, level=level + 1)
 
     def write(self, parser: ArgumentParser) -> None:
@@ -205,13 +232,13 @@ class ArgparseRstWriter(ArgparseWriter):
 
         if cmd.positionals:
             string.write(self.begin_positionals())
-            for args, help in cmd.positionals:
+            for args, choices, nargs, help in cmd.positionals:
                 string.write(self.positional(args, help))
             string.write(self.end_positionals())
 
         if cmd.optionals:
             string.write(self.begin_optionals())
-            for flags, dest_flags, help in cmd.optionals:
+            for flags, dest, dest_flags, nargs, help in cmd.optionals:
                 string.write(self.optional(dest_flags, help))
             string.write(self.end_optionals())
 
@@ -338,7 +365,7 @@ class ArgparseRstWriter(ArgparseWriter):
         """
         return ""
 
-    def begin_subcommands(self, subcommands: Sequence[Tuple[ArgumentParser, str]]) -> str:
+    def begin_subcommands(self, subcommands: List[Tuple[ArgumentParser, str, str]]) -> str:
         """Table with links to other subcommands.
 
         Arguments:
@@ -355,114 +382,8 @@ class ArgparseRstWriter(ArgparseWriter):
 
 """
 
-        for cmd, _ in subcommands:
+        for cmd, _, _ in subcommands:
             prog = re.sub(r"^[^ ]* ", "", cmd.prog)
             string += "   * :ref:`{0} <{1}>`\n".format(prog, cmd.prog.replace(" ", "-"))
 
         return string + "\n"
-
-
-class ArgparseCompletionWriter(ArgparseWriter):
-    """Write argparse output as shell programmable tab completion functions."""
-
-    def format(self, cmd: Command) -> str:
-        """Return the string representation of a single node in the parser tree.
-
-        Args:
-            cmd: Parsed information about a command or subcommand.
-
-        Returns:
-            String representation of this subcommand.
-        """
-
-        assert cmd.optionals  # we should always at least have -h, --help
-        assert not (cmd.positionals and cmd.subcommands)  # one or the other
-
-        # We only care about the arguments/flags, not the help messages
-        positionals: Tuple[str, ...] = ()
-        if cmd.positionals:
-            positionals, _ = zip(*cmd.positionals)
-        optionals, _, _ = zip(*cmd.optionals)
-        subcommands: Tuple[str, ...] = ()
-        if cmd.subcommands:
-            _, subcommands = zip(*cmd.subcommands)
-
-        # Flatten lists of lists
-        optionals = [x for xx in optionals for x in xx]
-
-        return (
-            self.start_function(cmd.prog)
-            + self.body(positionals, optionals, subcommands)
-            + self.end_function(cmd.prog)
-        )
-
-    def start_function(self, prog: str) -> str:
-        """Return the syntax needed to begin a function definition.
-
-        Args:
-            prog: Program name.
-
-        Returns:
-            Function definition beginning.
-        """
-        name = prog.replace("-", "_").replace(" ", "_")
-        return "\n_{0}() {{".format(name)
-
-    def end_function(self, prog: str) -> str:
-        """Return the syntax needed to end a function definition.
-
-        Args:
-            prog: Program name
-
-        Returns:
-            Function definition ending.
-        """
-        return "}\n"
-
-    def body(
-        self, positionals: Sequence[str], optionals: Sequence[str], subcommands: Sequence[str]
-    ) -> str:
-        """Return the body of the function.
-
-        Args:
-            positionals: List of positional arguments.
-            optionals: List of optional arguments.
-            subcommands: List of subcommand parsers.
-
-        Returns:
-            Function body.
-        """
-        return ""
-
-    def positionals(self, positionals: Sequence[str]) -> str:
-        """Return the syntax for reporting positional arguments.
-
-        Args:
-            positionals: List of positional arguments.
-
-        Returns:
-            Syntax for positional arguments.
-        """
-        return ""
-
-    def optionals(self, optionals: Sequence[str]) -> str:
-        """Return the syntax for reporting optional flags.
-
-        Args:
-            optionals: List of optional arguments.
-
-        Returns:
-            Syntax for optional flags.
-        """
-        return ""
-
-    def subcommands(self, subcommands: Sequence[str]) -> str:
-        """Return the syntax for reporting subcommands.
-
-        Args:
-            subcommands: List of subcommand parsers.
-
-        Returns:
-            Syntax for subcommand parsers
-        """
-        return ""
diff --git a/lib/spack/spack/cmd/commands.py b/lib/spack/spack/cmd/commands.py
index 6af7bb54e8..a65031387d 100644
--- a/lib/spack/spack/cmd/commands.py
+++ b/lib/spack/spack/cmd/commands.py
@@ -9,16 +9,11 @@ import os
 import re
 import sys
 from argparse import ArgumentParser, Namespace
-from typing import IO, Any, Callable, Dict, Sequence, Set
+from typing import IO, Any, Callable, Dict, Iterable, List, Optional, Sequence, Set, Tuple, Union
 
 import llnl.util.filesystem as fs
 import llnl.util.tty as tty
-from llnl.util.argparsewriter import (
-    ArgparseCompletionWriter,
-    ArgparseRstWriter,
-    ArgparseWriter,
-    Command,
-)
+from llnl.util.argparsewriter import ArgparseRstWriter, ArgparseWriter, Command
 from llnl.util.tty.colify import colify
 
 import spack.cmd
@@ -43,7 +38,13 @@ update_completion_args: Dict[str, Dict[str, Any]] = {
         "format": "bash",
         "header": os.path.join(spack.paths.share_path, "bash", "spack-completion.in"),
         "update": os.path.join(spack.paths.share_path, "spack-completion.bash"),
-    }
+    },
+    "fish": {
+        "aliases": True,
+        "format": "fish",
+        "header": os.path.join(spack.paths.share_path, "fish", "spack-completion.in"),
+        "update": os.path.join(spack.paths.share_path, "spack-completion.fish"),
+    },
 }
 
 
@@ -178,9 +179,63 @@ _positional_to_subroutine: Dict[str, str] = {
 }
 
 
-class BashCompletionWriter(ArgparseCompletionWriter):
+class BashCompletionWriter(ArgparseWriter):
     """Write argparse output as bash programmable tab completion."""
 
+    def format(self, cmd: Command) -> str:
+        """Return the string representation of a single node in the parser tree.
+
+        Args:
+            cmd: Parsed information about a command or subcommand.
+
+        Returns:
+            String representation of this subcommand.
+        """
+
+        assert cmd.optionals  # we should always at least have -h, --help
+        assert not (cmd.positionals and cmd.subcommands)  # one or the other
+
+        # We only care about the arguments/flags, not the help messages
+        positionals: Tuple[str, ...] = ()
+        if cmd.positionals:
+            positionals, _, _, _ = zip(*cmd.positionals)
+        optionals, _, _, _, _ = zip(*cmd.optionals)
+        subcommands: Tuple[str, ...] = ()
+        if cmd.subcommands:
+            _, subcommands, _ = zip(*cmd.subcommands)
+
+        # Flatten lists of lists
+        optionals = [x for xx in optionals for x in xx]
+
+        return (
+            self.start_function(cmd.prog)
+            + self.body(positionals, optionals, subcommands)
+            + self.end_function(cmd.prog)
+        )
+
+    def start_function(self, prog: str) -> str:
+        """Return the syntax needed to begin a function definition.
+
+        Args:
+            prog: Program name.
+
+        Returns:
+            Function definition beginning.
+        """
+        name = prog.replace("-", "_").replace(" ", "_")
+        return "\n_{0}() {{".format(name)
+
+    def end_function(self, prog: str) -> str:
+        """Return the syntax needed to end a function definition.
+
+        Args:
+            prog: Program name
+
+        Returns:
+            Function definition ending.
+        """
+        return "}\n"
+
     def body(
         self, positionals: Sequence[str], optionals: Sequence[str], subcommands: Sequence[str]
     ) -> str:
@@ -264,6 +319,396 @@ class BashCompletionWriter(ArgparseCompletionWriter):
         return 'SPACK_COMPREPLY="{0}"'.format(" ".join(subcommands))
 
 
+# Map argument destination names to their complete commands
+# Earlier items in the list have higher precedence
+_dest_to_fish_complete = {
+    ("activate", "view"): "-f -a '(__fish_complete_directories)'",
+    ("bootstrap root", "path"): "-f -a '(__fish_complete_directories)'",
+    ("mirror add", "mirror"): "-f",
+    ("repo add", "path"): "-f -a '(__fish_complete_directories)'",
+    ("test find", "filter"): "-f -a '(__fish_spack_tests)'",
+    ("bootstrap", "name"): "-f -a '(__fish_spack_bootstrap_names)'",
+    ("buildcache create", "key"): "-f -a '(__fish_spack_gpg_keys)'",
+    ("build-env", r"spec \[--\].*"): "-f -a '(__fish_spack_build_env_spec)'",
+    ("checksum", "package"): "-f -a '(__fish_spack_packages)'",
+    (
+        "checksum",
+        "versions",
+    ): "-f -a '(__fish_spack_package_versions $__fish_spack_argparse_argv[1])'",
+    ("config", "path"): "-f -a '(__fish_spack_colon_path)'",
+    ("config", "section"): "-f -a '(__fish_spack_config_sections)'",
+    ("develop", "specs?"): "-f -k -a '(__fish_spack_specs_or_id)'",
+    ("diff", "specs?"): "-f -a '(__fish_spack_installed_specs)'",
+    ("gpg sign", "output"): "-f -a '(__fish_complete_directories)'",
+    ("gpg", "keys?"): "-f -a '(__fish_spack_gpg_keys)'",
+    ("graph", "specs?"): "-f -k -a '(__fish_spack_specs_or_id)'",
+    ("help", "help_command"): "-f -a '(__fish_spack_commands)'",
+    ("list", "filter"): "-f -a '(__fish_spack_packages)'",
+    ("mirror", "mirror"): "-f -a '(__fish_spack_mirrors)'",
+    ("pkg", "package"): "-f -a '(__fish_spack_pkg_packages)'",
+    ("remove", "specs?"): "-f -a '(__fish_spack_installed_specs)'",
+    ("repo", "namespace_or_path"): "$__fish_spack_force_files -a '(__fish_spack_repos)'",
+    ("restage", "specs?"): "-f -k -a '(__fish_spack_specs_or_id)'",
+    ("rm", "specs?"): "-f -a '(__fish_spack_installed_specs)'",
+    ("solve", "specs?"): "-f -k -a '(__fish_spack_specs_or_id)'",
+    ("spec", "specs?"): "-f -k -a '(__fish_spack_specs_or_id)'",
+    ("stage", "specs?"): "-f -k -a '(__fish_spack_specs_or_id)'",
+    ("test-env", r"spec \[--\].*"): "-f -a '(__fish_spack_build_env_spec)'",
+    ("test", r"\[?name.*"): "-f -a '(__fish_spack_tests)'",
+    ("undevelop", "specs?"): "-f -k -a '(__fish_spack_specs_or_id)'",
+    ("verify", "specs_or_files"): "$__fish_spack_force_files -a '(__fish_spack_installed_specs)'",
+    ("view", "path"): "-f -a '(__fish_complete_directories)'",
+    ("", "comment"): "-f",
+    ("", "compiler_spec"): "-f -a '(__fish_spack_installed_compilers)'",
+    ("", "config_scopes"): "-f -a '(__fish_complete_directories)'",
+    ("", "extendable"): "-f -a '(__fish_spack_extensions)'",
+    ("", "installed_specs?"): "-f -a '(__fish_spack_installed_specs)'",
+    ("", "job_url"): "-f",
+    ("", "location_env"): "-f -a '(__fish_complete_directories)'",
+    ("", "pytest_args"): "-f -a '(__fish_spack_unit_tests)'",
+    ("", "package_or_file"): "$__fish_spack_force_files -a '(__fish_spack_packages)'",
+    ("", "package_or_user"): "-f -a '(__fish_spack_packages)'",
+    ("", "package"): "-f -a '(__fish_spack_packages)'",
+    ("", "PKG"): "-f -a '(__fish_spack_packages)'",
+    ("", "prefix"): "-f -a '(__fish_complete_directories)'",
+    ("", r"rev\d?"): "-f -a '(__fish_spack_git_rev)'",
+    ("", "specs?"): "-f -k -a '(__fish_spack_specs)'",
+    ("", "tags?"): "-f -a '(__fish_spack_tags)'",
+    ("", "virtual_package"): "-f -a '(__fish_spack_providers)'",
+    ("", "working_dir"): "-f -a '(__fish_complete_directories)'",
+    ("", r"(\w*_)?env"): "-f -a '(__fish_spack_environments)'",
+    ("", r"(\w*_)?dir(ectory)?"): "-f -a '(__fish_spack_environments)'",
+    ("", r"(\w*_)?mirror_name"): "-f -a '(__fish_spack_mirrors)'",
+}
+
+
+def _fish_dest_get_complete(prog: str, dest: str) -> Optional[str]:
+    """Map from subcommand to autocompletion argument.
+
+    Args:
+        prog: Program name.
+        dest: Destination.
+
+    Returns:
+        Autocompletion argument.
+    """
+    s = prog.split(None, 1)
+    subcmd = s[1] if len(s) == 2 else ""
+
+    for (prog_key, pos_key), value in _dest_to_fish_complete.items():
+        if subcmd.startswith(prog_key) and re.match("^" + pos_key + "$", dest):
+            return value
+    return None
+
+
+class FishCompletionWriter(ArgparseWriter):
+    """Write argparse output as bash programmable tab completion."""
+
+    def format(self, cmd: Command) -> str:
+        """Return the string representation of a single node in the parser tree.
+
+        Args:
+            cmd: Parsed information about a command or subcommand.
+
+        Returns:
+            String representation of a node.
+        """
+        assert cmd.optionals  # we should always at least have -h, --help
+        assert not (cmd.positionals and cmd.subcommands)  # one or the other
+
+        # We also need help messages and how arguments are used
+        # So we pass everything to completion writer
+        positionals = cmd.positionals
+        optionals = cmd.optionals
+        subcommands = cmd.subcommands
+
+        return (
+            self.prog_comment(cmd.prog)
+            + self.optspecs(cmd.prog, optionals)
+            + self.complete(cmd.prog, positionals, optionals, subcommands)
+        )
+
+    def _quote(self, string: str) -> str:
+        """Quote string and escape special characters if necessary.
+
+        Args:
+            string: Input string.
+
+        Returns:
+            Quoted string.
+        """
+        # Goal here is to match fish_indent behavior
+
+        # Strings without spaces (or other special characters) do not need to be escaped
+        if not any([sub in string for sub in [" ", "'", '"']]):
+            return string
+
+        string = string.replace("'", r"\'")
+        return f"'{string}'"
+
+    def optspecs(
+        self,
+        prog: str,
+        optionals: List[Tuple[Sequence[str], List[str], str, Union[int, str, None], str]],
+    ) -> str:
+        """Read the optionals and return the command to set optspec.
+
+        Args:
+            prog: Program name.
+            optionals: List of optional arguments.
+
+        Returns:
+            Command to set optspec variable.
+        """
+        # Variables of optspecs
+        optspec_var = "__fish_spack_optspecs_" + prog.replace(" ", "_").replace("-", "_")
+
+        if optionals is None:
+            return "set -g %s\n" % optspec_var
+
+        # Build optspec by iterating over options
+        args = []
+
+        for flags, dest, _, nargs, _ in optionals:
+            if len(flags) == 0:
+                continue
+
+            required = ""
+
+            # Because nargs '?' is treated differently in fish, we treat it as required.
+            # Because multi-argument options are not supported, we treat it like one argument.
+            required = "="
+            if nargs == 0:
+                required = ""
+
+            # Pair short options with long options
+
+            # We need to do this because fish doesn't support multiple short
+            # or long options.
+            # However, since we are paring options only, this is fine
+
+            short = [f[1:] for f in flags if f.startswith("-") and len(f) == 2]
+            long = [f[2:] for f in flags if f.startswith("--")]
+
+            while len(short) > 0 and len(long) > 0:
+                arg = "%s/%s%s" % (short.pop(), long.pop(), required)
+            while len(short) > 0:
+                arg = "%s/%s" % (short.pop(), required)
+            while len(long) > 0:
+                arg = "%s%s" % (long.pop(), required)
+
+            args.append(arg)
+
+        # Even if there is no option, we still set variable.
+        # In fish such variable is an empty array, we use it to
+        # indicate that such subcommand exists.
+        args = " ".join(args)
+
+        return "set -g %s %s\n" % (optspec_var, args)
+
+    @staticmethod
+    def complete_head(
+        prog: str, index: Optional[int] = None, nargs: Optional[Union[int, str]] = None
+    ) -> str:
+        """Return the head of the completion command.
+
+        Args:
+            prog: Program name.
+            index: Index of positional argument.
+            nargs: Number of arguments.
+
+        Returns:
+            Head of the completion command.
+        """
+        # Split command and subcommand
+        s = prog.split(None, 1)
+        subcmd = s[1] if len(s) == 2 else ""
+
+        if index is None:
+            return "complete -c %s -n '__fish_spack_using_command %s'" % (s[0], subcmd)
+        elif nargs in [argparse.ZERO_OR_MORE, argparse.ONE_OR_MORE, argparse.REMAINDER]:
+            head = "complete -c %s -n '__fish_spack_using_command_pos_remainder %d %s'"
+        else:
+            head = "complete -c %s -n '__fish_spack_using_command_pos %d %s'"
+        return head % (s[0], index, subcmd)
+
+    def complete(
+        self,
+        prog: str,
+        positionals: List[Tuple[str, Optional[Iterable[Any]], Union[int, str, None], str]],
+        optionals: List[Tuple[Sequence[str], List[str], str, Union[int, str, None], str]],
+        subcommands: List[Tuple[ArgumentParser, str, str]],
+    ) -> str:
+        """Return all the completion commands.
+
+        Args:
+            prog: Program name.
+            positionals: List of positional arguments.
+            optionals: List of optional arguments.
+            subcommands: List of subcommand parsers.
+
+        Returns:
+            Completion command.
+        """
+        commands = []
+
+        if positionals:
+            commands.append(self.positionals(prog, positionals))
+
+        if subcommands:
+            commands.append(self.subcommands(prog, subcommands))
+
+        if optionals:
+            commands.append(self.optionals(prog, optionals))
+
+        return "".join(commands)
+
+    def positionals(
+        self,
+        prog: str,
+        positionals: List[Tuple[str, Optional[Iterable[Any]], Union[int, str, None], str]],
+    ) -> str:
+        """Return the completion for positional arguments.
+
+        Args:
+            prog: Program name.
+            positionals: List of positional arguments.
+
+        Returns:
+            Completion command.
+        """
+        commands = []
+
+        for idx, (args, choices, nargs, help) in enumerate(positionals):
+            # Make sure we always get same order of output
+            if isinstance(choices, dict):
+                choices = sorted(choices.keys())
+            elif isinstance(choices, (set, frozenset)):
+                choices = sorted(choices)
+
+            # Remove platform-specific choices to avoid hard-coding the platform.
+            if choices is not None:
+                valid_choices = []
+                for choice in choices:
+                    if spack.platforms.host().name not in choice:
+                        valid_choices.append(choice)
+                choices = valid_choices
+
+            head = self.complete_head(prog, idx, nargs)
+
+            if choices is not None:
+                # If there are choices, we provide a completion for all possible values.
+                commands.append(head + " -f -a %s" % self._quote(" ".join(choices)))
+            else:
+                # Otherwise, we try to find a predefined completion for it
+                value = _fish_dest_get_complete(prog, args)
+                if value is not None:
+                    commands.append(head + " " + value)
+
+        return "\n".join(commands) + "\n"
+
+    def prog_comment(self, prog: str) -> str:
+        """Return a comment line for the command.
+
+        Args:
+            prog: Program name.
+
+        Returns:
+            Comment line.
+        """
+        return "\n# %s\n" % prog
+
+    def optionals(
+        self,
+        prog: str,
+        optionals: List[Tuple[Sequence[str], List[str], str, Union[int, str, None], str]],
+    ) -> str:
+        """Return the completion for optional arguments.
+
+        Args:
+            prog: Program name.
+            optionals: List of optional arguments.
+
+        Returns:
+            Completion command.
+        """
+        commands = []
+        head = self.complete_head(prog)
+
+        for flags, dest, _, nargs, help in optionals:
+            # Make sure we always get same order of output
+            if isinstance(dest, dict):
+                dest = sorted(dest.keys())
+            elif isinstance(dest, (set, frozenset)):
+                dest = sorted(dest)
+
+            # Remove platform-specific choices to avoid hard-coding the platform.
+            if dest is not None:
+                valid_choices = []
+                for choice in dest:
+                    if spack.platforms.host().name not in choice:
+                        valid_choices.append(choice)
+                dest = valid_choices
+
+            # To provide description for optionals, and also possible values,
+            # we need to use two split completion command.
+            # Otherwise, each option will have same description.
+            prefix = head
+
+            # Add all flags to the completion
+            for f in flags:
+                if f.startswith("--"):
+                    long = f[2:]
+                    prefix += " -l %s" % long
+                elif f.startswith("-"):
+                    short = f[1:]
+                    assert len(short) == 1
+                    prefix += " -s %s" % short
+
+            # Check if option require argument.
+            # Currently multi-argument options are not supported, so we treat it like one argument.
+            if nargs != 0:
+                prefix += " -r"
+
+            if dest is not None:
+                # If there are choices, we provide a completion for all possible values.
+                commands.append(prefix + " -f -a %s" % self._quote(" ".join(dest)))
+            else:
+                # Otherwise, we try to find a predefined completion for it
+                value = _fish_dest_get_complete(prog, dest)
+                if value is not None:
+                    commands.append(prefix + " " + value)
+
+            if help:
+                commands.append(prefix + " -d %s" % self._quote(help))
+
+        return "\n".join(commands) + "\n"
+
+    def subcommands(self, prog: str, subcommands: List[Tuple[ArgumentParser, str, str]]) -> str:
+        """Return the completion for subcommands.
+
+        Args:
+            prog: Program name.
+            subcommands: List of subcommand parsers.
+
+        Returns:
+            Completion command.
+        """
+        commands = []
+        head = self.complete_head(prog, 0)
+
+        for _, subcommand, help in subcommands:
+            command = head + " -f -a %s" % self._quote(subcommand)
+
+            if help is not None and len(help) > 0:
+                help = help.split("\n")[0]
+                command += " -d %s" % self._quote(help)
+
+            commands.append(command)
+
+        return "\n".join(commands) + "\n"
+
+
 @formatter
 def subcommands(args: Namespace, out: IO) -> None:
     """Hierarchical tree of subcommands.
@@ -371,6 +816,15 @@ def bash(args: Namespace, out: IO) -> None:
     writer.write(parser)
 
 
+@formatter
+def fish(args, out):
+    parser = spack.main.make_argument_parser()
+    spack.main.add_all_commands(parser)
+
+    writer = FishCompletionWriter(parser.prog, out, args.aliases)
+    writer.write(parser)
+
+
 def prepend_header(args: Namespace, out: IO) -> None:
     """Prepend header text at the beginning of a file.
 
diff --git a/lib/spack/spack/cmd/mirror.py b/lib/spack/spack/cmd/mirror.py
index 4b853c1bc0..15181d4ce6 100644
--- a/lib/spack/spack/cmd/mirror.py
+++ b/lib/spack/spack/cmd/mirror.py
@@ -253,12 +253,12 @@ def _configure_mirror(args):
 
 
 def mirror_set(args):
-    """Configure the connection details of a mirror"""
+    """configure the connection details of a mirror"""
     _configure_mirror(args)
 
 
 def mirror_set_url(args):
-    """Change the URL of a mirror."""
+    """change the URL of a mirror"""
     _configure_mirror(args)
 
 
diff --git a/lib/spack/spack/test/cmd/commands.py b/lib/spack/spack/test/cmd/commands.py
index 1477aa4859..7a531b4f91 100644
--- a/lib/spack/spack/test/cmd/commands.py
+++ b/lib/spack/spack/test/cmd/commands.py
@@ -14,7 +14,7 @@ import pytest
 import spack.cmd
 import spack.main
 import spack.paths
-from spack.cmd.commands import _positional_to_subroutine
+from spack.cmd.commands import _dest_to_fish_complete, _positional_to_subroutine
 
 commands = spack.main.SpackCommand("commands", subprocess=True)
 
@@ -185,26 +185,59 @@ def test_bash_completion():
     assert "_spack_compiler_add() {" in out2
 
 
-def test_update_completion_arg(tmpdir, monkeypatch):
+def test_fish_completion():
+    """Test the fish completion writer."""
+    out1 = commands("--format=fish")
+
+    # Make sure header not included
+    assert "function __fish_spack_argparse" not in out1
+    assert "complete -c spack --erase" not in out1
+
+    # Make sure subcommands appear
+    assert "__fish_spack_using_command remove" in out1
+    assert "__fish_spack_using_command compiler find" in out1
+
+    # Make sure aliases don't appear
+    assert "__fish_spack_using_command rm" not in out1
+    assert "__fish_spack_using_command compiler add" not in out1
+
+    # Make sure options appear
+    assert "-s h -l help" in out1
+
+    # Make sure subcommands are called
+    for complete_cmd in _dest_to_fish_complete.values():
+        assert complete_cmd in out1
+
+    out2 = commands("--aliases", "--format=fish")
+
+    # Make sure aliases appear
+    assert "__fish_spack_using_command rm" in out2
+    assert "__fish_spack_using_command compiler add" in out2
+
+
+@pytest.mark.parametrize("shell", ["bash", "fish"])
+def test_update_completion_arg(shell, tmpdir, monkeypatch):
+    """Test the update completion flag."""
+
     mock_infile = tmpdir.join("spack-completion.in")
-    mock_bashfile = tmpdir.join("spack-completion.bash")
+    mock_outfile = tmpdir.join(f"spack-completion.{shell}")
 
     mock_args = {
-        "bash": {
+        shell: {
             "aliases": True,
-            "format": "bash",
+            "format": shell,
             "header": str(mock_infile),
-            "update": str(mock_bashfile),
+            "update": str(mock_outfile),
         }
     }
 
     # make a mock completion file missing the --update-completion argument
     real_args = spack.cmd.commands.update_completion_args
-    shutil.copy(real_args["bash"]["header"], mock_args["bash"]["header"])
-    with open(real_args["bash"]["update"]) as old:
+    shutil.copy(real_args[shell]["header"], mock_args[shell]["header"])
+    with open(real_args[shell]["update"]) as old:
         old_file = old.read()
-        with open(mock_args["bash"]["update"], "w") as mock:
-            mock.write(old_file.replace("--update-completion", ""))
+        with open(mock_args[shell]["update"], "w") as mock:
+            mock.write(old_file.replace("update-completion", ""))
 
     monkeypatch.setattr(spack.cmd.commands, "update_completion_args", mock_args)
 
@@ -214,16 +247,17 @@ def test_update_completion_arg(tmpdir, monkeypatch):
         local_commands("--update-completion", "-a")
 
     # ensure arg is restored
-    assert "--update-completion" not in mock_bashfile.read()
+    assert "update-completion" not in mock_outfile.read()
     local_commands("--update-completion")
-    assert "--update-completion" in mock_bashfile.read()
+    assert "update-completion" in mock_outfile.read()
 
 
 # Note: this test is never expected to be supported on Windows
 @pytest.mark.skipif(
-    sys.platform == "win32", reason="bash completion script generator fails on windows"
+    sys.platform == "win32", reason="shell completion script generator fails on windows"
 )
-def test_updated_completion_scripts(tmpdir):
+@pytest.mark.parametrize("shell", ["bash", "fish"])
+def test_updated_completion_scripts(shell, tmpdir):
     """Make sure our shell tab completion scripts remain up-to-date."""
 
     msg = (
@@ -233,12 +267,11 @@ def test_updated_completion_scripts(tmpdir):
         "and adding the changed files to your pull request."
     )
 
-    for shell in ["bash"]:  # 'zsh', 'fish']:
-        header = os.path.join(spack.paths.share_path, shell, "spack-completion.in")
-        script = "spack-completion.{0}".format(shell)
-        old_script = os.path.join(spack.paths.share_path, script)
-        new_script = str(tmpdir.join(script))
+    header = os.path.join(spack.paths.share_path, shell, "spack-completion.in")
+    script = "spack-completion.{0}".format(shell)
+    old_script = os.path.join(spack.paths.share_path, script)
+    new_script = str(tmpdir.join(script))
 
-        commands("--aliases", "--format", shell, "--header", header, "--update", new_script)
+    commands("--aliases", "--format", shell, "--header", header, "--update", new_script)
 
-        assert filecmp.cmp(old_script, new_script), msg
+    assert filecmp.cmp(old_script, new_script), msg
diff --git a/lib/spack/spack/test/llnl/util/argparsewriter.py b/lib/spack/spack/test/llnl/util/argparsewriter.py
index a2455e0303..433833c6a2 100644
--- a/lib/spack/spack/test/llnl/util/argparsewriter.py
+++ b/lib/spack/spack/test/llnl/util/argparsewriter.py
@@ -22,13 +22,3 @@ spack.main.add_all_commands(parser)
 def test_format_not_overridden():
     with pytest.raises(TypeError):
         aw.ArgparseWriter("spack")
-
-
-def test_completion_format_not_overridden():
-    writer = aw.ArgparseCompletionWriter("spack")
-
-    assert writer.positionals([]) == ""
-    assert writer.optionals([]) == ""
-    assert writer.subcommands([]) == ""
-
-    writer.write(parser)
-- 
cgit v1.2.3-60-g2f50