Wrapping completions to allow injecting arguments

This enables some limited use of arguments for wrapping completions. The
simplest example is that complete gco -w 'git checkout' now works like
you would want: `gco <tab>` now invokes git's completions with the
`checkout` argument prepended.

Fixes #1976

CHANGELOG.md

@@ -41,6 +41,7 @@ This section is for changes merged to the `major` branch that are not also merge
 - Tildes in file names are now properly escaped in completions (#2274)
 - A pipe at the end of a line now allows the job to continue on the next line (#1285)
 - The names `argparse`, `read`, `set`, `status`, `test` and `[` are now reserved and not allowed as function names. This prevents users unintentionally breaking stuff (#3000).
+- Wrapping completions (from `complete -w` or `function -w`) can now inject arguments. For example, `complete gco -w 'git checkout'` now works properly (#1976).
 
 ## Other significant changes
 - Command substitution output is now limited to 10 MB by default (#3822).

src/complete.cpp

@@ -1220,6 +1220,45 @@ bool completer_t::try_complete_user(const wcstring &str) {
 #endif
 }
 
+// The callback type for walk_wrap_chain
+using wrap_chain_visitor_t = std::function<void(const wcstring &, const wcstring &, size_t depth)>;
+
+// Helper to complete a parameter for a command and its transitive wrap chain.
+// Given a command line \p command_line and the range of the command itself within the command line
+// as \p command_range, invoke the \p receiver with the command and the command line. Then, for each
+// target wrapped by the given command, update the command line with that target and invoke this
+// recursively.
+static void walk_wrap_chain(const wcstring &command_line, source_range_t command_range,
+                            const wrap_chain_visitor_t &visitor, size_t depth = 0) {
+    // Limit our recursion depth. This prevents cycles in the wrap chain graph from overflowing.
+    if (depth > 24) return;
+
+    // Extract command from the command line and invoke the receiver with it.
+    wcstring command(command_line, command_range.start, command_range.length);
+    visitor(command, command_line, depth);
+
+    wcstring_list_t targets = complete_get_wrap_targets(command);
+    for (const wcstring &wt : targets) {
+        // Construct a fake command line containing the wrap target.
+        wcstring faux_commandline = command_line;
+        faux_commandline.replace(command_range.start, command_range.length, wt);
+
+        // Try to extract the command from the faux commandline.
+        // We do this by simply getting the first token. This is a hack; for example one might
+        // imagine the first token being 'builtin' or similar. Nevertheless that is simpler than
+        // re-parsing everything.
+        wcstring wrapped_command = tok_first(wt);
+        if (!wrapped_command.empty()) {
+            size_t where = faux_commandline.find(wrapped_command, command_range.start);
+            if (where != wcstring::npos) {
+                // Recurse with our new command and command line.
+                source_range_t faux_source_range{uint32_t(where), uint32_t(wrapped_command.size())};
+                walk_wrap_chain(faux_commandline, faux_source_range, visitor, depth + 1);
+            }
+        }
+    }
+}
+
 void complete(const wcstring &cmd_with_subcmds, std::vector<completion_t> *out_comps,
               completion_request_flags_t flags) {
     // Determine the innermost subcommand.
@@ -1417,31 +1456,22 @@ void complete(const wcstring &cmd_with_subcmds, std::vector<completion_t> *out_c
                         // Have to walk over the command and its entire wrap chain. If any command
                         // disables do_file, then they all do.
                         do_file = true;
-                        const wcstring_list_t wrap_chain =
-                            complete_get_wrap_chain(current_command_unescape);
-                        for (size_t i = 0; i < wrap_chain.size(); i++) {
-                            // Hackish, this. The first command in the chain is always the given
-                            // command. For every command past the first, we need to create a
-                            // transient commandline for builtin_commandline. But not for
-                            // COMPLETION_REQUEST_AUTOSUGGESTION, which may occur on background
-                            // threads.
-                            std::unique_ptr<builtin_commandline_scoped_transient_t> transient_cmd;
-                            if (i == 0) {
-                                assert(wrap_chain.at(i) == current_command_unescape);
-                            } else if (!(flags & COMPLETION_REQUEST_AUTOSUGGESTION)) {
-                                wcstring faux_cmdline = cmd;
-                                faux_cmdline.replace(cmd_node.source_range()->start,
-                                                     cmd_node.source_range()->length,
-                                                     wrap_chain.at(i));
-                                transient_cmd = make_unique<builtin_commandline_scoped_transient_t>(
-                                    faux_cmdline);
+                        auto receiver = [&](const wcstring &cmd, const wcstring &cmdline,
+                                            size_t depth) {
+                            // Perhaps set a transient commandline so that custom completions
+                            // buitin_commandline will refer to the wrapped command. But not if
+                            // we're doing autosuggestions.
+                            std::unique_ptr<builtin_commandline_scoped_transient_t> bcst;
+                            if (depth > 0 && !(flags & COMPLETION_REQUEST_AUTOSUGGESTION)) {
+                                bcst = make_unique<builtin_commandline_scoped_transient_t>(cmdline);
                             }
-                            if (!completer.complete_param(wrap_chain.at(i),
-                                                          previous_argument_unescape,
+                            // Now invoke any custom completions for this command.
+                            if (!completer.complete_param(cmd, previous_argument_unescape,
                                                           current_argument_unescape, !had_ddash)) {
                                 do_file = false;
                             }
-                        }
+                        };
+                        walk_wrap_chain(cmd, *cmd_node.source_range(), receiver);
                     }
 
                     // Hack. If we're cd, handle it specially (issue #1059, others).
@@ -1590,42 +1620,15 @@ bool complete_remove_wrapper(const wcstring &command, const wcstring &target_to_
     return result;
 }
 
-wcstring_list_t complete_get_wrap_chain(const wcstring &command) {
+wcstring_list_t complete_get_wrap_targets(const wcstring &command) {
     if (command.empty()) {
-        return wcstring_list_t();
+        return {};
     }
     scoped_lock locker(wrapper_lock);
     const wrapper_map_t &wraps = wrap_map();
-
-    wcstring_list_t result;
-    std::unordered_set<wcstring> visited;  // set of visited commands
-    wcstring_list_t to_visit(1, command);  // stack of remaining-to-visit commands
-
-    wcstring target;
-    while (!to_visit.empty()) {
-        // Grab the next command to visit, put it in target.
-        target = std::move(to_visit.back());
-        to_visit.pop_back();
-
-        // Try inserting into visited. If it was already present, we skip it; this is how we avoid
-        // loops.
-        if (!visited.insert(target).second) {
-            continue;
-        }
-
-        // Insert the target in the result. Note this is the command itself, if this is the first
-        // iteration of the loop.
-        result.push_back(target);
-
-        // Enqueue its children.
-        wrapper_map_t::const_iterator target_children_iter = wraps.find(target);
-        if (target_children_iter != wraps.end()) {
-            const wcstring_list_t &children = target_children_iter->second;
-            to_visit.insert(to_visit.end(), children.begin(), children.end());
-        }
-    }
-
-    return result;
+    auto iter = wraps.find(command);
+    if (iter == wraps.end()) return {};
+    return iter->second;
 }
 
 wcstring_list_t complete_get_wrap_pairs() {

src/complete.h

@@ -186,12 +186,12 @@ void append_completion(std::vector<completion_t> *completions, wcstring comp,
 /// Function used for testing.
 void complete_set_variable_names(const wcstring_list_t *names);
 
-/// Support for "wrap targets." A wrap target is a command that completes liek another command. The
-/// target chain is the sequence of wraps (A wraps B wraps C...). Any loops in the chain are
-/// silently ignored.
+/// Support for "wrap targets." A wrap target is a command that completes like another command.
 bool complete_add_wrapper(const wcstring &command, const wcstring &wrap_target);
 bool complete_remove_wrapper(const wcstring &command, const wcstring &wrap_target);
-wcstring_list_t complete_get_wrap_chain(const wcstring &command);
+
+/// Returns a list of wrap targets for a given command.
+wcstring_list_t complete_get_wrap_targets(const wcstring &command);
 
 // Wonky interface: returns all wraps. Even-values are the commands, odd values are the targets.
 wcstring_list_t complete_get_wrap_pairs();

src/fish_tests.cpp

@@ -2348,16 +2348,20 @@ static void test_complete() {
     complete_set_variable_names(NULL);
 
     // Test wraps.
-    do_test(comma_join(complete_get_wrap_chain(L"wrapper1")) == L"wrapper1");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper1")) == L"");
     complete_add_wrapper(L"wrapper1", L"wrapper2");
-    do_test(comma_join(complete_get_wrap_chain(L"wrapper1")) == L"wrapper1,wrapper2");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper1")) == L"wrapper2");
     complete_add_wrapper(L"wrapper2", L"wrapper3");
-    do_test(comma_join(complete_get_wrap_chain(L"wrapper1")) == L"wrapper1,wrapper2,wrapper3");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper1")) == L"wrapper2");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper2")) == L"wrapper3");
     complete_add_wrapper(L"wrapper3", L"wrapper1");  // loop!
-    do_test(comma_join(complete_get_wrap_chain(L"wrapper1")) == L"wrapper1,wrapper2,wrapper3");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper1")) == L"wrapper2");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper2")) == L"wrapper3");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper3")) == L"wrapper1");
     complete_remove_wrapper(L"wrapper1", L"wrapper2");
-    do_test(comma_join(complete_get_wrap_chain(L"wrapper1")) == L"wrapper1");
-    do_test(comma_join(complete_get_wrap_chain(L"wrapper2")) == L"wrapper2,wrapper3,wrapper1");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper1")) == L"");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper2")) == L"wrapper3");
+    do_test(comma_join(complete_get_wrap_targets(L"wrapper3")) == L"wrapper1");
 }
 
 static void test_1_completion(wcstring line, const wcstring &completion, complete_flags_t flags,

tests/complete.err

@@ -0,0 +1,3 @@
+
+####################
+# Completion Wrappers

tests/complete.in

@@ -0,0 +1,9 @@
+logmsg Completion Wrappers
+
+complete -c complete_test_alpha1 --no-files -a '(commandline)'
+complete -c complete_test_alpha2 --no-files -w 'complete_test_alpha1 extra1'
+complete -c complete_test_alpha3 --no-files -w 'complete_test_alpha2 extra2'
+
+complete -C'complete_test_alpha1 arg1 '
+complete -C'complete_test_alpha2 arg2 '
+complete -C'complete_test_alpha3 arg3 '

tests/complete.out

@@ -0,0 +1,6 @@
+
+####################
+# Completion Wrappers
+complete_test_alpha1 arg1 
+complete_test_alpha1 extra1 arg2 
+complete_test_alpha1 extra1 extra2 arg3