From d28a6618a886427aea26b2d356680eb569b7a036 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Mar 2024 04:38:33 +0100 Subject: [PATCH] Merge pull request #9167 from obsidiansystems/pre-overhaul-completions Improve tests and docs prior to refactoring completions (cherry picked from commit 5442d9b47298389918d1f38d20f768a80ffc2369) Change-Id: Ief99ac2cd9c92981a9a522d15b9c3daf99182c9d --- src/libcmd/command.hh | 78 ++++++++++++++++++++++++++++----- src/libutil/args.hh | 59 ++++++++++++++++++++++++- tests/functional/completions.sh | 2 + 3 files changed, 125 insertions(+), 14 deletions(-) diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index 96236b987..5c4569001 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -34,21 +34,28 @@ struct NixMultiCommand : virtual MultiCommand, virtual Command // For the overloaded run methods #pragma GCC diagnostic ignored "-Woverloaded-virtual" -/* A command that requires a Nix store. */ +/** + * A command that requires a \ref Store "Nix store". + */ struct StoreCommand : virtual Command { StoreCommand(); void run() override; ref getStore(); virtual ref createStore(); + /** + * Main entry point, with a `Store` provided + */ virtual void run(ref) = 0; private: std::shared_ptr _store; }; -/* A command that copies something between `--from` and `--to` - stores. */ +/** + * A command that copies something between `--from` and `--to` \ref + * Store stores. + */ struct CopyCommand : virtual StoreCommand { std::string srcUri, dstUri; @@ -60,6 +67,9 @@ struct CopyCommand : virtual StoreCommand ref getDstStore(); }; +/** + * A command that needs to evaluate Nix language expressions. + */ struct EvalCommand : virtual StoreCommand, MixEvalArgs { bool startReplOnEvalErrors = false; @@ -79,6 +89,10 @@ private: std::shared_ptr evalState; }; +/** + * A mixin class for commands that process flakes, adding a few standard + * flake-related options/flags. + */ struct MixFlakeOptions : virtual Args, EvalCommand { flake::LockFlags lockFlags; @@ -87,6 +101,14 @@ struct MixFlakeOptions : virtual Args, EvalCommand MixFlakeOptions(); + /** + * The completion for some of these flags depends on the flake(s) in + * question. + * + * This method should be implemented to gather all flakerefs the + * command is operating with (presumably specified via some other + * arguments) so that the completions for these flags can use them. + */ virtual std::vector getFlakesForCompletion() { return {}; } @@ -112,15 +134,29 @@ struct SourceExprCommand : virtual Args, MixFlakeOptions virtual Strings getDefaultFlakeAttrPathPrefixes(); + /** + * Complete an installable from the given prefix. + */ void completeInstallable(std::string_view prefix); }; +/** + * A mixin class for commands that need a read-only flag. + * + * What exactly is "read-only" is unspecified, but it will usually be + * the \ref Store "Nix store". + */ struct MixReadOnlyOption : virtual Args { MixReadOnlyOption(); }; -/* Like InstallablesCommand but the installables are not loaded */ +/** + * Like InstallablesCommand but the installables are not loaded. + * + * This is needed by `CmdRepl` which wants to load (and reload) the + * installables itself. + */ struct RawInstallablesCommand : virtual Args, SourceExprCommand { RawInstallablesCommand(); @@ -129,7 +165,7 @@ struct RawInstallablesCommand : virtual Args, SourceExprCommand void run(ref store) override; - // FIXME make const after CmdRepl's override is fixed up + // FIXME make const after `CmdRepl`'s override is fixed up virtual void applyDefaultInstallables(std::vector & rawInstallables); bool readFromStdIn = false; @@ -140,8 +176,11 @@ private: std::vector rawInstallables; }; -/* A command that operates on a list of "installables", which can be - store paths, attribute paths, Nix expressions, etc. */ + +/** + * A command that operates on a list of "installables", which can be + * store paths, attribute paths, Nix expressions, etc. + */ struct InstallablesCommand : RawInstallablesCommand { virtual void run(ref store, Installables && installables) = 0; @@ -149,7 +188,9 @@ struct InstallablesCommand : RawInstallablesCommand void run(ref store, std::vector && rawInstallables) override; }; -/* A command that operates on exactly one "installable" */ +/** + * A command that operates on exactly one "installable". + */ struct InstallableCommand : virtual Args, SourceExprCommand { InstallableCommand(); @@ -175,7 +216,12 @@ struct MixOperateOnOptions : virtual Args MixOperateOnOptions(); }; -/* A command that operates on zero or more store paths. */ +/** + * A command that operates on zero or more extant store paths. + * + * If the argument the user passes is a some sort of recipe for a path + * not yet built, it must be built first. + */ struct BuiltPathsCommand : InstallablesCommand, virtual MixOperateOnOptions { private: @@ -207,7 +253,9 @@ struct StorePathsCommand : public BuiltPathsCommand void run(ref store, BuiltPaths && paths) override; }; -/* A command that operates on exactly one store path. */ +/** + * A command that operates on exactly one store path. + */ struct StorePathCommand : public StorePathsCommand { virtual void run(ref store, const StorePath & storePath) = 0; @@ -215,7 +263,9 @@ struct StorePathCommand : public StorePathsCommand void run(ref store, StorePaths && storePaths) override; }; -/* A helper class for registering commands globally. */ +/** + * A helper class for registering \ref Command commands globally. + */ struct RegisterCommand { typedef std::map, std::function()>> Commands; @@ -271,7 +321,11 @@ struct MixEnvironment : virtual Args { MixEnvironment(); - /* Modify global environ based on ignoreEnvironment, keep, and unset. It's expected that exec will be called before this class goes out of scope, otherwise environ will become invalid. */ + /*** + * Modify global environ based on `ignoreEnvironment`, `keep`, and + * `unset`. It's expected that exec will be called before this class + * goes out of scope, otherwise `environ` will become invalid. + */ void setEnviron(); }; diff --git a/src/libutil/args.hh b/src/libutil/args.hh index d90129796..b55f1d238 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -39,8 +39,21 @@ public: protected: + /** + * The largest `size_t` is used to indicate the "any" arity, for + * handlers/flags/arguments that accept an arbitrary number of + * arguments. + */ static const size_t ArityAny = std::numeric_limits::max(); + /** + * Arguments (flags/options and positional) have a "handler" which is + * caused when the argument is parsed. The handler has an arbitrary side + * effect, including possible affect further command-line parsing. + * + * There are many constructors in order to support many shorthand + * initializations, and this is used a lot. + */ struct Handler { std::function)> fun; @@ -110,7 +123,12 @@ protected: { } }; - /* Options. */ + /** + * Description of flags / options + * + * These are arguments like `-s` or `--long` that can (mostly) + * appear in any order. + */ struct Flag { typedef std::shared_ptr ptr; @@ -130,12 +148,30 @@ protected: static Flag mkHashTypeOptFlag(std::string && longName, std::optional * oht); }; + /** + * Index of all registered "long" flag descriptions (flags like + * `--long`). + */ std::map longFlags; + + /** + * Index of all registered "short" flag descriptions (flags like + * `-s`). + */ std::map shortFlags; + /** + * Process a single flag and its arguments, pulling from an iterator + * of raw CLI args as needed. + */ virtual bool processFlag(Strings::iterator & pos, Strings::iterator end); - /* Positional arguments. */ + /** + * Description of positional arguments + * + * These are arguments that do not start with a `-`, and for which + * the order does matter. + */ struct ExpectedArg { std::string label; @@ -144,8 +180,24 @@ protected: std::function completer; }; + /** + * Queue of expected positional argument forms. + * + * Positional arugment descriptions are inserted on the back. + * + * As positional arguments are passed, these are popped from the + * front, until there are hopefully none left as all args that were + * expected in fact were passed. + */ std::list expectedArgs; + /** + * Process some positional arugments + * + * @param finish: We have parsed everything else, and these are the only + * arguments left. Used because we accumulate some "pending args" we might + * have left over. + */ virtual bool processArgs(const Strings & args, bool finish); virtual Strings::iterator rewriteArgs(Strings & args, Strings::iterator pos) @@ -204,6 +256,9 @@ public: friend class MultiCommand; + /** + * The parent command, used if this is a subcommand. + */ MultiCommand * parent = nullptr; private: diff --git a/tests/functional/completions.sh b/tests/functional/completions.sh index 19dc61098..7c1e4b287 100644 --- a/tests/functional/completions.sh +++ b/tests/functional/completions.sh @@ -48,6 +48,8 @@ EOF [[ "$(NIX_GET_COMPLETIONS=5 nix build ./foo ./bar --override-input '')" == $'normal\na\t\nb\t' ]] ## With tilde expansion [[ "$(HOME=$PWD NIX_GET_COMPLETIONS=4 nix build '~/foo' --override-input '')" == $'normal\na\t' ]] +[[ "$(HOME=$PWD NIX_GET_COMPLETIONS=5 nix flake show '~/foo' --update-input '')" == $'normal\na\t' ]] +[[ "$(HOME=$PWD NIX_GET_COMPLETIONS=4 nix run '~/foo' --update-input '')" == $'normal\na\t' ]] ## Out of order [[ "$(NIX_GET_COMPLETIONS=3 nix build --update-input '' ./foo)" == $'normal\na\t' ]] [[ "$(NIX_GET_COMPLETIONS=4 nix build ./foo --update-input '' ./bar)" == $'normal\na\t\nb\t' ]]