Allow autosuggestions to do job expansion. Fixes

https://github.com/fish-shell/fish-shell/issues/1152

6 files changed, 142 insertions, 114 deletions

diff --git a/complete.cpp b/complete.cpp
index de822a6b..2b7423b5 100644
--- a/complete.cpp
+++ b/complete.cpp
@@ -430,7 +430,7 @@ public:
         /* Never do command substitution in autosuggestions. Sadly, we also can't yet do job expansion because it's not thread safe. */
         expand_flags_t result = 0;
         if (this->type() == COMPLETE_AUTOSUGGEST)
-            result |= EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_JOBS;
+            result |= EXPAND_SKIP_CMDSUBST;
 
         /* Allow fuzzy matching */
         if (this->fuzzy())
diff --git a/expand.cpp b/expand.cpp
index 2a3a1fe8..10e3cbf4 100644
--- a/expand.cpp
+++ b/expand.cpp
@@ -49,6 +49,7 @@ parameter expansion.
 #include "signal.h"
 #include "tokenizer.h"
 #include "complete.h"
+#include "iothread.h"
 
 #include "parse_util.h"
 
@@ -560,106 +561,100 @@ std::vector<wcstring> expand_get_all_process_names(void)
     return result;
 }
 
-/**
-   Searches for a job with the specified job id, or a job or process
-   which has the string \c proc as a prefix of its commandline.
+/* Helper function to do a job search. */
+struct find_job_data_t
+{
+    const wchar_t *proc; /* The process to search for - possibly numeric, possibly a name */
+    expand_flags_t flags;
+    std::vector<completion_t> *completions;
+};
 
-   If the ACCEPT_INCOMPLETE flag is set, the remaining string for any matches
-   are inserted.
+/* The following function is invoked on the main thread, because the job list is not thread safe. It should search the job list for something matching the given proc, and then return 1 to stop the search, 0 to continue it  */
+static int find_job(const struct find_job_data_t *info)
+{
+    ASSERT_IS_MAIN_THREAD();
 
-   Otherwise, any job matching the specified string is matched, and
-   the job pgid is returned. If no job matches, all child processes
-   are searched. If no child processes match, and <tt>fish</tt> can
-   understand the contents of the /proc filesystem, all the users
-   processes are searched for matches.
-*/
+    const wchar_t * const proc = info->proc;
+    const expand_flags_t flags = info->flags;
+    std::vector<completion_t> &completions = *info->completions;
 
-static int find_process(const wchar_t *proc,
-                        expand_flags_t flags,
-                        std::vector<completion_t> &out)
-{
+    const job_t *j;
     int found = 0;
-
-    if (!(flags & EXPAND_SKIP_JOBS))
+    // do the empty param check first, because an empty string passes our 'numeric' check
+    if (wcslen(proc)==0)
+    {
+        /*
+          This is an empty job expansion: '%'
+          It expands to the last job backgrounded.
+        */
+        job_iterator_t jobs;
+        while ((j = jobs.next()))
+        {
+            if (!j->command_is_empty())
+            {
+                append_completion(completions, to_string<long>(j->pgid));
+                break;
+            }
+        }
+        /*
+          You don't *really* want to flip a coin between killing
+          the last process backgrounded and all processes, do you?
+          Let's not try other match methods with the solo '%' syntax.
+        */
+        found = 1;
+    }
+    else if (iswnumeric(proc))
     {
-        ASSERT_IS_MAIN_THREAD();
-        const job_t *j;
+        /*
+          This is a numeric job string, like '%2'
+        */
 
-        // do the empty param check first, because an empty string passes our 'numeric' check
-        if (wcslen(proc)==0)
+        if (flags & ACCEPT_INCOMPLETE)
         {
-            /*
-              This is an empty job expansion: '%'
-              It expands to the last job backgrounded.
-            */
             job_iterator_t jobs;
             while ((j = jobs.next()))
             {
-                if (!j->command_is_empty())
+                wchar_t jid[16];
+                if (j->command_is_empty())
+                    continue;
+
+                swprintf(jid, 16, L"%d", j->job_id);
+
+                if (wcsncmp(proc, jid, wcslen(proc))==0)
                 {
-                    append_completion(out, to_string<long>(j->pgid));
-                    break;
+                    wcstring desc_buff = format_string(COMPLETE_JOB_DESC_VAL, j->command_wcstr());
+                    append_completion(completions,
+                                      jid+wcslen(proc),
+                                      desc_buff,
+                                      0);
                 }
             }
-            /*
-              You don't *really* want to flip a coin between killing
-              the last process backgrounded and all processes, do you?
-              Let's not try other match methods with the solo '%' syntax.
-            */
-            found = 1;
         }
-        else if (iswnumeric(proc))
+        else
         {
-            /*
-              This is a numeric job string, like '%2'
-            */
+            int jid;
+            wchar_t *end;
 
-            if (flags & ACCEPT_INCOMPLETE)
+            errno = 0;
+            jid = fish_wcstoi(proc, &end, 10);
+            if (jid > 0 && !errno && !*end)
             {
-                job_iterator_t jobs;
-                while ((j = jobs.next()))
+                j = job_get(jid);
+                if ((j != 0) && (j->command_wcstr() != 0) && (!j->command_is_empty()))
                 {
-                    wchar_t jid[16];
-                    if (j->command_is_empty())
-                        continue;
-
-                    swprintf(jid, 16, L"%d", j->job_id);
-
-                    if (wcsncmp(proc, jid, wcslen(proc))==0)
-                    {
-                        wcstring desc_buff = format_string(COMPLETE_JOB_DESC_VAL, j->command_wcstr());
-                        append_completion(out,
-                                          jid+wcslen(proc),
-                                          desc_buff,
-                                          0);
-                    }
-                }
-            }
-            else
-            {
-                int jid;
-                wchar_t *end;
-
-                errno = 0;
-                jid = fish_wcstoi(proc, &end, 10);
-                if (jid > 0 && !errno && !*end)
-                {
-                    j = job_get(jid);
-                    if ((j != 0) && (j->command_wcstr() != 0) && (!j->command_is_empty()))
-                    {
-                        append_completion(out, to_string<long>(j->pgid));
-                    }
+                    append_completion(completions, to_string<long>(j->pgid));
                 }
             }
-            /*
-               Stop here so you can't match a random process name
-               when you're just trying to use job control.
-            */
-            found = 1;
         }
-        if (found)
-            return 1;
+        /*
+           Stop here so you can't match a random process name
+           when you're just trying to use job control.
+        */
+        found = 1;
+    }
 
+    if (! found)
+    {
         job_iterator_t jobs;
         while ((j = jobs.next()))
         {
@@ -672,56 +667,84 @@ static int find_process(const wchar_t *proc,
             {
                 if (flags & ACCEPT_INCOMPLETE)
                 {
-                    append_completion(out,
+                    append_completion(completions,
                                       j->command_wcstr() + offset + wcslen(proc),
                                       COMPLETE_JOB_DESC,
                                       0);
                 }
                 else
                 {
-                    append_completion(out, to_string<long>(j->pgid));
+                    append_completion(completions, to_string<long>(j->pgid));
                     found = 1;
                 }
             }
         }
 
-        if (found)
+        if (! found)
         {
-            return 1;
-        }
-
-        jobs.reset();
-        while ((j = jobs.next()))
-        {
-            process_t *p;
-            if (j->command_is_empty())
-                continue;
-            for (p=j->first_process; p; p=p->next)
+            jobs.reset();
+            while ((j = jobs.next()))
             {
-                if (p->actual_cmd.empty())
+                process_t *p;
+                if (j->command_is_empty())
                     continue;
-
-                size_t offset;
-                if (match_pid(p->actual_cmd, proc, flags, &offset))
+                for (p=j->first_process; p; p=p->next)
                 {
-                    if (flags & ACCEPT_INCOMPLETE)
-                    {
-                        append_completion(out,
-                                          wcstring(p->actual_cmd, offset + wcslen(proc)),
-                                          COMPLETE_CHILD_PROCESS_DESC,
-                                          0);
-                    }
-                    else
+                    if (p->actual_cmd.empty())
+                        continue;
+
+                    size_t offset;
+                    if (match_pid(p->actual_cmd, proc, flags, &offset))
                     {
-                        append_completion(out,
-                                          to_string<long>(p->pid),
-                                          L"",
-                                          0);
-                        found = 1;
+                        if (flags & ACCEPT_INCOMPLETE)
+                        {
+                            append_completion(completions,
+                                              wcstring(p->actual_cmd, offset + wcslen(proc)),
+                                              COMPLETE_CHILD_PROCESS_DESC,
+                                              0);
+                        }
+                        else
+                        {
+                            append_completion(completions,
+                                              to_string<long>(p->pid),
+                                              L"",
+                                              0);
+                            found = 1;
+                        }
                     }
                 }
             }
         }
+    }
+
+    return found;
+}
+
+
+/**
+   Searches for a job with the specified job id, or a job or process
+   which has the string \c proc as a prefix of its commandline.
+
+   If the ACCEPT_INCOMPLETE flag is set, the remaining string for any matches
+   are inserted.
+
+   Otherwise, any job matching the specified string is matched, and
+   the job pgid is returned. If no job matches, all child processes
+   are searched. If no child processes match, and <tt>fish</tt> can
+   understand the contents of the /proc filesystem, all the users
+   processes are searched for matches.
+*/
+
+static int find_process(const wchar_t *proc,
+                        expand_flags_t flags,
+                        std::vector<completion_t> &out)
+{
+    int found = 0;
+
+    if (!(flags & EXPAND_SKIP_JOBS))
+    {
+        const struct find_job_data_t data = {proc, flags, &out};
+        found = iothread_perform_on_main(find_job, &data);
 
         if (found)
         {
diff --git a/highlight.cpp b/highlight.cpp
index 2e154387..fd7aa481 100644
--- a/highlight.cpp
+++ b/highlight.cpp
@@ -873,7 +873,7 @@ bool autosuggest_validate_from_history(const history_item_t &item, file_detectio
     {
         /* We can possibly handle this specially */
         wcstring dir = parsed_arguments.back();
-        if (expand_one(dir, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_JOBS))
+        if (expand_one(dir, EXPAND_SKIP_CMDSUBST))
         {
             handled = true;
             bool is_help = string_prefixes_string(dir, L"--help") || string_prefixes_string(dir, L"-h");
@@ -997,7 +997,7 @@ static void tokenize(const wchar_t * const buff, std::vector<int> &color, const
                     if (cmd == L"cd")
                     {
                         wcstring dir = tok_last(&tok);
-                        if (expand_one(dir, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_JOBS))
+                        if (expand_one(dir, EXPAND_SKIP_CMDSUBST))
                         {
                             int is_help = string_prefixes_string(dir, L"--help") || string_prefixes_string(dir, L"-h");
                             if (!is_help && ! is_potential_cd_path(dir, working_directory, PATH_EXPAND_TILDE, NULL))
@@ -1182,7 +1182,7 @@ static void tokenize(const wchar_t * const buff, std::vector<int> &color, const
                     case TOK_STRING:
                     {
                         target_str = tok_last(&tok);
-                        if (expand_one(target_str, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_JOBS))
+                        if (expand_one(target_str, EXPAND_SKIP_CMDSUBST))
                         {
                             target = target_str.c_str();
                         }
diff --git a/iothread.cpp b/iothread.cpp
index 6f07aec0..73646792 100644
--- a/iothread.cpp
+++ b/iothread.cpp
@@ -305,7 +305,11 @@ static void iothread_service_main_thread_requests(void)
 
 int iothread_perform_on_main_base(int (*handler)(void *), void *context)
 {
-    ASSERT_IS_BACKGROUND_THREAD();
+    // If this is the main thread, just do it
+    if (is_main_thread())
+    {
+        return handler(context);
+    }
 
     // Make a new request. Note we are synchronous, so this can be stack allocated!
     MainThreadRequest_t req;
diff --git a/iothread.h b/iothread.h
index ac328ace..8ac3bd9d 100644
--- a/iothread.h
+++ b/iothread.h
@@ -42,7 +42,7 @@ int iothread_perform(int (*handler)(T *), void (*completionCallback)(T *, int),
 template<typename T>
 int iothread_perform_on_main(int (*handler)(T *), T *context)
 {
-    return iothread_perform_on_main_base((int (*)(void *))handler, static_cast<void *>(context));
+    return iothread_perform_on_main_base((int (*)(void *))handler, (void *)(context));
 }
 
 
diff --git a/proc.cpp b/proc.cpp
index 1ca70099..a774d359 100644
--- a/proc.cpp
+++ b/proc.cpp
@@ -106,6 +106,7 @@ job_iterator_t::job_iterator_t(job_list_t &jobs) : job_list(&jobs)
 
 job_iterator_t::job_iterator_t() : job_list(&parser_t::principal_parser().job_list())
 {
+    ASSERT_IS_MAIN_THREAD();
     this->reset();
 }