Overhaul completions, redo #6693 (#8131)

As I complained in
https://github.com/NixOS/nix/pull/6784#issuecomment-1421777030 (a
comment on the wrong PR, sorry again!), #6693 introduced a second
completions mechanism to fix a bug. Having two completion mechanisms
isn't so nice.

As @thufschmitt also pointed out, it was a bummer to go from `FlakeRef`
to `std::string` when collecting flake refs. Now it is `FlakeRefs`
again.

The underlying issue that sought to work around was that completion of
arguments not at the end can still benefit from the information from
latter arguments.

To fix this better, we rip out that change and simply defer all
completion processing until after all the (regular, already-complete)
arguments have been passed.

In addition, I noticed the original completion logic used some global
variables. I do not like global variables, because even if they save
lines of code, they also obfuscate the architecture of the code.

I got rid of them  moved them to a new `RootArgs` class, which now has
`parseCmdline` instead of `Args`. The idea is that we have many argument
parsers from subcommands and what-not, but only one root args that owns
the other per actual parsing invocation. The state that was global is
now part of the root args instead.

This did, admittedly, add a bunch of new code. And I do feel bad about
that. So I went and added a lot of API docs to try to at least make the
current state of things clear to the next person.

--

This is needed for RFC 134 (tracking issue #7868). It was very hard to
modularize `Installable` parsing when there were two completion
arguments. I wouldn't go as far as to say it is *easy* now, but at least
it is less hard (and the completions test finally passed).

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Change-Id: If18cd5be78da4a70635e3fdcac6326dbfeea71a5
(cherry picked from commit 67eb37c1d0de28160cd25376e51d1ec1b1c8305b)

1 files changed, 69 insertions, 36 deletions

diff --git a/src/libutil/args.hh b/src/libutil/args.hh
index b55f1d238..c90a48ad4 100644
--- a/src/libutil/args.hh
+++ b/src/libutil/args.hh
@@ -15,17 +15,15 @@ enum HashType : char;
 
 class MultiCommand;
 
+class RootArgs;
+
+class AddCompletions;
+
 class Args
 {
 public:
 
     /**
-     * Parse the command line, throwing a UsageError if something goes
-     * wrong.
-     */
-    void parseCmdline(const Strings & cmdline);
-
-    /**
      * Return a short one-line description of the command.
      */
     virtual std::string description() { return ""; }
@@ -124,6 +122,25 @@ protected:
     };
 
     /**
+     * The basic function type of the completion callback.
+     *
+     * Used to define `CompleterClosure` and some common case completers
+     * that individual flags/arguments can use.
+     *
+     * The `AddCompletions` that is passed is an interface to the state
+     * stored as part of the root command
+     */
+    typedef void CompleterFun(AddCompletions &, size_t, std::string_view);
+
+    /**
+     * The closure type of the completion callback.
+     *
+     * This is what is actually stored as part of each Flag / Expected
+     * Arg.
+     */
+    typedef std::function<CompleterFun> CompleterClosure;
+
+    /**
      * Description of flags / options
      *
      * These are arguments like `-s` or `--long` that can (mostly)
@@ -140,7 +157,7 @@ protected:
         std::string category;
         Strings labels;
         Handler handler;
-        std::function<void(size_t, std::string_view)> completer;
+        CompleterClosure completer;
 
         std::optional<ExperimentalFeature> experimentalFeature;
 
@@ -177,7 +194,7 @@ protected:
         std::string label;
         bool optional = false;
         Handler handler;
-        std::function<void(size_t, std::string_view)> completer;
+        CompleterClosure completer;
     };
 
     /**
@@ -211,13 +228,6 @@ protected:
      */
     virtual void initialFlagsProcessed() {}
 
-    /**
-     * Called after the command line has been processed if we need to generate
-     * completions. Useful for commands that need to know the whole command line
-     * in order to know what completions to generate.
-     */
-    virtual void completionHook() { }
-
 public:
 
     void addFlag(Flag && flag);
@@ -252,24 +262,30 @@ public:
         });
     }
 
+    static CompleterFun completePath;
+
+    static CompleterFun completeDir;
+
     virtual nlohmann::json toJSON();
 
     friend class MultiCommand;
 
     /**
      * The parent command, used if this is a subcommand.
+     *
+     * Invariant: An Args with a null parent must also be a RootArgs
+     *
+     * \todo this would probably be better in the CommandClass.
+     * getRoot() could be an abstract method that peels off at most one
+     * layer before recuring.
      */
     MultiCommand * parent = nullptr;
 
-private:
-
     /**
-     * Experimental features needed when parsing args. These are checked
-     * after flag parsing is completed in order to support enabling
-     * experimental features coming after the flag that needs the
-     * experimental feature.
+     * Traverse parent pointers until we find the \ref RootArgs "root
+     * arguments" object.
      */
-    std::set<ExperimentalFeature> flagExperimentalFeatures;
+    RootArgs & getRoot();
 };
 
 /**
@@ -320,8 +336,6 @@ public:
 
     bool processArgs(const Strings & args, bool finish) override;
 
-    void completionHook() override;
-
     nlohmann::json toJSON() override;
 };
 
@@ -333,21 +347,40 @@ struct Completion {
 
     bool operator<(const Completion & other) const;
 };
-class Completions : public std::set<Completion> {
+
+/**
+ * The abstract interface for completions callbacks
+ *
+ * The idea is to restrict the callback so it can only add additional
+ * completions to the collection, or set the completion type. By making
+ * it go through this interface, the callback cannot make any other
+ * changes, or even view the completions / completion type that have
+ * been set so far.
+ */
+class AddCompletions
+{
 public:
-    void add(std::string completion, std::string description = "");
-};
-extern std::shared_ptr<Completions> completions;
 
-enum CompletionType {
-    ctNormal,
-    ctFilenames,
-    ctAttrs
-};
-extern CompletionType completionType;
+    /**
+     * The type of completion we are collecting.
+     */
+    enum class Type {
+        Normal,
+        Filenames,
+        Attrs,
+    };
 
-void completePath(size_t, std::string_view prefix);
+    /**
+     * Set the type of the completions being collected
+     *
+     * \todo it should not be possible to change the type after it has been set.
+     */
+    virtual void setType(Type type) = 0;
 
-void completeDir(size_t, std::string_view prefix);
+    /**
+     * Add a single completion to the collection
+     */
+    virtual void add(std::string completion, std::string description = "") = 0;
+};
 
 }