Make complete() return the completion list directly

Returning it through a pointer was a remnant of pre C++-11 days.

src/builtin_complete.cpp

@@ -347,9 +347,8 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             if (!have_do_complete_param)
                 parser.libdata().builtin_complete_current_commandline = true;
 
-            completion_list_t comp;
+            completion_list_t comp = complete(do_complete_param, completion_request_t::fuzzy_match,
-            complete(do_complete_param, &comp, completion_request_t::fuzzy_match, parser.vars(),
+                                              parser.vars(), parser.shared());
-                     parser.shared());
 
             for (const auto &next : comp) {
                 // Make a fake commandline, and then apply the completion to it.

src/complete.cpp

@@ -1595,9 +1595,9 @@ void completer_t::perform() {
                     parser->libdata().transient_commandlines.push_back(unaliased_cmd);
                     cleanup_t remove_transient(
                         [&] { parser->libdata().transient_commandlines.pop_back(); });
-                    completion_list_t comp;
+                    completion_list_t comp =
-                    complete(unaliased_cmd, &comp, completion_request_t::fuzzy_match,
+                        complete(unaliased_cmd, completion_request_t::fuzzy_match, parser->vars(),
-                             parser->vars(), parser->shared());
+                                 parser->shared());
                     this->completions.insert(completions.end(), comp.begin(), comp.end());
                     do_file = false;
                 } else if (!complete_param(
@@ -1635,9 +1635,8 @@ void completer_t::perform() {
     mark_completions_duplicating_arguments(current_token, tokens);
 }
 
-void complete(const wcstring &cmd_with_subcmds, completion_list_t *out_comps,
+completion_list_t complete(const wcstring &cmd_with_subcmds, completion_request_flags_t flags,
-              completion_request_flags_t flags, const environment_t &vars,
+                           const environment_t &vars, const std::shared_ptr<parser_t> &parser) {
-              const std::shared_ptr<parser_t> &parser) {
     // Determine the innermost subcommand.
     const wchar_t *cmdsubst_begin, *cmdsubst_end;
     parse_util_cmdsubst_extent(cmd_with_subcmds.c_str(), cmd_with_subcmds.size(), &cmdsubst_begin,
@@ -1646,7 +1645,7 @@ void complete(const wcstring &cmd_with_subcmds, completion_list_t *out_comps,
     wcstring cmd = wcstring(cmdsubst_begin, cmdsubst_end - cmdsubst_begin);
     completer_t completer(vars, parser, std::move(cmd), flags);
     completer.perform();
-    *out_comps = completer.acquire_completions();
+    return completer.acquire_completions();
 }
 
 /// Print the short switch \c opt, and the argument \c arg to the specified

src/complete.h

@@ -171,9 +171,9 @@ void complete_remove(const wcstring &cmd, bool cmd_is_path, const wcstring &opti
 /// Removes all completions for a given command.
 void complete_remove_all(const wcstring &cmd, bool cmd_is_path);
 
-/// Find all completions of the command cmd, insert them into out.
+/// \return all completions of the command cmd.
 class parser_t;
-void complete(const wcstring &cmd, completion_list_t *out_comps, completion_request_flags_t flags,
+completion_list_t complete(const wcstring &cmd, completion_request_flags_t flags,
                            const environment_t &vars, const std::shared_ptr<parser_t> &parser);
 
 /// Return a list of all current completions.

src/fish_tests.cpp

@@ -2612,8 +2612,13 @@ static void test_complete() {
 
     auto parser = parser_t::principal_parser().shared();
 
+    auto do_complete = [&](const wcstring &cmd, completion_request_flags_t flags) {
+        return complete(cmd, flags, vars, parser);
+    };
+
     completion_list_t completions;
-    complete(L"$", &completions, {}, vars, parser);
+
+    completions = do_complete(L"$", {});
     completions_sort_and_prioritize(&completions);
     do_test(completions.size() == 6);
     do_test(completions.at(0).completion == L"Bar1");
@@ -2623,21 +2628,18 @@ static void test_complete() {
     do_test(completions.at(4).completion == L"Foo2");
     do_test(completions.at(5).completion == L"Foo3");
 
-    completions.clear();
+    completions = do_complete(L"$F", {});
-    complete(L"$F", &completions, {}, vars, parser);
     completions_sort_and_prioritize(&completions);
     do_test(completions.size() == 3);
     do_test(completions.at(0).completion == L"oo1");
     do_test(completions.at(1).completion == L"oo2");
     do_test(completions.at(2).completion == L"oo3");
 
-    completions.clear();
+    completions = do_complete(L"$1", {});
-    complete(L"$1", &completions, {}, vars, parser);
     completions_sort_and_prioritize(&completions);
     do_test(completions.empty());
 
-    completions.clear();
+    completions = do_complete(L"$1", completion_request_t::fuzzy_match);
-    complete(L"$1", &completions, completion_request_t::fuzzy_match, vars, parser);
     completions_sort_and_prioritize(&completions);
     do_test(completions.size() == 2);
     do_test(completions.at(0).completion == L"$Bar1");
@@ -2650,36 +2652,30 @@ static void test_complete() {
     if (system("touch 'test/complete_test/testfile'")) err(L"touch failed");
     if (system("chmod 700 'test/complete_test/testfile'")) err(L"chmod failed");
 
-    completions.clear();
+    completions = do_complete(L"echo (test/complete_test/testfil", {});
-    complete(L"echo (test/complete_test/testfil", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"e");
 
-    completions.clear();
+    completions = do_complete(L"echo (ls test/complete_test/testfil", {});
-    complete(L"echo (ls test/complete_test/testfil", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"e");
 
-    completions.clear();
+    completions = do_complete(L"echo (command ls test/complete_test/testfil", {});
-    complete(L"echo (command ls test/complete_test/testfil", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"e");
 
     // Completing after spaces - see #2447
-    completions.clear();
+    completions = do_complete(L"echo (ls test/complete_test/has\\ ", {});
-    complete(L"echo (ls test/complete_test/has\\ ", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"space");
 
     // Brackets - see #5831
-    completions.clear();
+    completions = do_complete(L"echo (ls test/complete_test/bracket[", {});
-    complete(L"echo (ls test/complete_test/bracket[", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"test/complete_test/bracket[abc]");
 
     wcstring cmdline = L"touch test/complete_test/bracket[";
-    completions.clear();
+    completions = do_complete(cmdline, {});
-    complete(cmdline, &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.front().completion == L"test/complete_test/bracket[abc]");
     size_t where = cmdline.size();
@@ -2687,9 +2683,8 @@ static void test_complete() {
         completions.front().completion, completions.front().flags, cmdline, &where, false);
     do_test(newcmdline == L"touch test/complete_test/bracket\\[abc\\] ");
 
-    completions.clear();
     cmdline = LR"(touch test/complete_test/gnarlybracket\\[)";
-    complete(cmdline, &completions, {}, vars, parser);
+    completions = do_complete(cmdline, {});
     do_test(completions.size() == 1);
     do_test(completions.front().completion == LR"(test/complete_test/gnarlybracket\[abc])");
     where = cmdline.size();
@@ -2703,24 +2698,20 @@ static void test_complete() {
     function_add(L"scuttlebutt", {}, nullptr, {});
 
     // Complete a function name.
-    completions.clear();
+    completions = do_complete(L"echo (scuttlebut", {});
-    complete(L"echo (scuttlebut", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"t");
 
     // But not with the command prefix.
-    completions.clear();
+    completions = do_complete(L"echo (command scuttlebut", {});
-    complete(L"echo (command scuttlebut", &completions, {}, vars, parser);
     do_test(completions.size() == 0);
 
     // Not with the builtin prefix.
-    completions.clear();
+    completions = do_complete(L"echo (builtin scuttlebut", {});
-    complete(L"echo (builtin scuttlebut", &completions, {}, vars, parser);
     do_test(completions.size() == 0);
 
     // Not after a redirection.
-    completions.clear();
+    completions = do_complete(L"echo hi > scuttlebut", {});
-    complete(L"echo hi > scuttlebut", &completions, {}, vars, parser);
     do_test(completions.size() == 0);
 
     // Trailing spaces (#1261).
@@ -2728,83 +2719,65 @@ static void test_complete() {
     no_files.no_files = true;
     complete_add(L"foobarbaz", false, wcstring(), option_type_args_only, no_files, NULL, L"qux",
                  NULL, COMPLETE_AUTO_SPACE);
-    completions.clear();
+    completions = do_complete(L"foobarbaz ", {});
-    complete(L"foobarbaz ", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"qux");
 
     // Don't complete variable names in single quotes (#1023).
-    completions.clear();
+    completions = do_complete(L"echo '$Foo", {});
-    complete(L"echo '$Foo", &completions, {}, vars, parser);
     do_test(completions.empty());
-    completions.clear();
+    completions = do_complete(L"echo \\$Foo", {});
-    complete(L"echo \\$Foo", &completions, {}, vars, parser);
     do_test(completions.empty());
 
     // File completions.
-    completions.clear();
+    completions = do_complete(L"cat test/complete_test/te", {});
-    complete(L"cat test/complete_test/te", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
-    completions.clear();
+    completions = do_complete(L"echo sup > test/complete_test/te", {});
-    complete(L"echo sup > test/complete_test/te", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
-    completions.clear();
+    completions = do_complete(L"echo sup > test/complete_test/te", {});
-    complete(L"echo sup > test/complete_test/te", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
 
     if (!pushd("test/complete_test")) return;
-    complete(L"cat te", &completions, {}, vars, parser);
+    completions = do_complete(L"cat te", {});
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
     do_test(!(completions.at(0).flags & COMPLETE_REPLACES_TOKEN));
     do_test(!(completions.at(0).flags & COMPLETE_DUPLICATES_ARGUMENT));
-    completions.clear();
+    completions = do_complete(L"cat testfile te", {});
-    complete(L"cat testfile te", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
     do_test(completions.at(0).flags & COMPLETE_DUPLICATES_ARGUMENT);
-    completions.clear();
+    completions = do_complete(L"cat testfile TE", {});
-    complete(L"cat testfile TE", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"testfile");
     do_test(completions.at(0).flags & COMPLETE_REPLACES_TOKEN);
     do_test(completions.at(0).flags & COMPLETE_DUPLICATES_ARGUMENT);
-    completions.clear();
+    completions = do_complete(L"something --abc=te", {});
-    complete(L"something --abc=te", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
-    completions.clear();
+    completions = do_complete(L"something -abc=te", {});
-    complete(L"something -abc=te", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
-    completions.clear();
+    completions = do_complete(L"something abc=te", {});
-    complete(L"something abc=te", &completions, {}, vars, parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"stfile");
-    completions.clear();
+    completions = do_complete(L"something abc=stfile", {});
-    complete(L"something abc=stfile", &completions, {}, vars, parser);
     do_test(completions.size() == 0);
-    completions.clear();
+    completions = do_complete(L"something abc=stfile", completion_request_t::fuzzy_match);
-    complete(L"something abc=stfile", &completions, completion_request_t::fuzzy_match, vars,
-             parser);
     do_test(completions.size() == 1);
     do_test(completions.at(0).completion == L"abc=testfile");
 
     // Zero escapes can cause problems. See issue #1631.
-    completions.clear();
+    completions = do_complete(L"cat foo\\0", {});
-    complete(L"cat foo\\0", &completions, {}, vars, parser);
     do_test(completions.empty());
-    completions.clear();
+    completions = do_complete(L"cat foo\\0bar", {});
-    complete(L"cat foo\\0bar", &completions, {}, vars, parser);
     do_test(completions.empty());
-    completions.clear();
+    completions = do_complete(L"cat \\0", {});
-    complete(L"cat \\0", &completions, {}, vars, parser);
     do_test(completions.empty());
-    completions.clear();
+    completions = do_complete(L"cat te\\0", {});
-    complete(L"cat te\\0", &completions, {}, vars, parser);
     do_test(completions.empty());
 
     popd();
@@ -2814,7 +2787,7 @@ static void test_complete() {
     auto &pvars = parser_t::principal_parser().vars();
     function_add(L"testabbrsonetwothreefour", {}, nullptr, {});
     int ret = pvars.set_one(L"_fish_abbr_testabbrsonetwothreezero", ENV_LOCAL, L"expansion");
-    complete(L"testabbrsonetwothree", &completions, {}, pvars, parser);
+    completions = complete(L"testabbrsonetwothree", {}, pvars, parser);
     do_test(ret == 0);
     do_test(completions.size() == 2);
     do_test(completions.at(0).completion == L"four");
@@ -2898,8 +2871,8 @@ static void test_completion_insertions() {
 
 static void perform_one_autosuggestion_cd_test(const wcstring &command, const wcstring &expected,
                                                const environment_t &vars, long line) {
-    completion_list_t comps;
+    completion_list_t comps =
-    complete(command, &comps, completion_request_t::autosuggestion, vars, nullptr);
+        complete(command, completion_request_t::autosuggestion, vars, nullptr);
 
     bool expects_error = (expected == L"<error>");
 
@@ -2934,8 +2907,7 @@ static void perform_one_autosuggestion_cd_test(const wcstring &command, const wc
 
 static void perform_one_completion_cd_test(const wcstring &command, const wcstring &expected,
                                            const environment_t &vars, long line) {
-    completion_list_t comps;
+    completion_list_t comps = complete(command, {}, vars, nullptr);
-    complete(command, &comps, {}, vars, nullptr);
 
     bool expects_error = (expected == L"<error>");
 
@@ -3074,8 +3046,8 @@ static void test_autosuggest_suggest_special() {
 }
 
 static void perform_one_autosuggestion_should_ignore_test(const wcstring &command, long line) {
-    completion_list_t comps;
+    completion_list_t comps =
-    complete(command, &comps, completion_request_t::autosuggestion, null_environment_t{}, nullptr);
+        complete(command, completion_request_t::autosuggestion, null_environment_t{}, nullptr);
     do_test(comps.empty());
     if (!comps.empty()) {
         const wcstring &suggestion = comps.front().completion;

src/reader.cpp

@@ -1325,8 +1325,7 @@ static std::function<autosuggestion_result_t(void)> get_autosuggestion_performer
 
         // Try normal completions.
         completion_request_flags_t complete_flags = completion_request_t::autosuggestion;
-        completion_list_t completions;
+        completion_list_t completions = complete(search_string, complete_flags, *vars, nullptr);
-        complete(search_string, &completions, complete_flags, *vars, nullptr);
         completions_sort_and_prioritize(&completions, complete_flags);
         if (!completions.empty()) {
             const completion_t &comp = completions.at(0);
@@ -2567,7 +2566,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
                 // std::fwprintf(stderr, L"Complete (%ls)\n", buffcpy.c_str());
                 completion_request_flags_t complete_flags = {completion_request_t::descriptions,
                                                              completion_request_t::fuzzy_match};
-                complete_func(buffcpy, &rls.comp, complete_flags, vars, parser_ref);
+                rls.comp = complete_func(buffcpy, complete_flags, vars, parser_ref);
 
                 // User-supplied completions may have changed the commandline - prevent buffer
                 // overflow.

src/reader.h

@@ -147,9 +147,10 @@ void reader_push(parser_t &parser, const wcstring &name);
 void reader_pop();
 
 /// Specify function to use for finding possible tab completions.
-typedef void (*complete_function_t)(const wcstring &, completion_list_t *,
+typedef completion_list_t (*complete_function_t)(const wcstring &, completion_request_flags_t,
-                                    completion_request_flags_t, const environment_t &,
+                                                 const environment_t &,
                                                  const std::shared_ptr<parser_t> &parser);
+
 void reader_set_complete_function(complete_function_t);
 
 /// The type of a highlight function.