Change clients of file_detection_context_t to use lambdas

Allows for eliminating file_detection_context_t

src/highlight.cpp

@@ -327,7 +327,6 @@ static bool autosuggest_parse_command(const wcstring &buff, wcstring *out_expand
 }
 
 bool autosuggest_validate_from_history(const history_item_t &item,
-                                       file_detection_context_t &detector,
                                        const wcstring &working_directory,
                                        const env_vars_snapshot_t &vars) {
     ASSERT_IS_BACKGROUND_THREAD();
@@ -372,11 +371,7 @@ bool autosuggest_validate_from_history(const history_item_t &item,
 
     if (cmd_ok) {
         const path_list_t &paths = item.get_required_paths();
-        if (paths.empty()) {
-            suggestionOK = true;
-        } else {
-            suggestionOK = detector.paths_are_valid(paths);
-        }
+        suggestionOK = all_paths_are_valid(paths, working_directory);
     }
 
     return suggestionOK;

src/highlight.h

@@ -108,7 +108,6 @@ rgb_color_t highlight_get_color(highlight_spec_t highlight, bool is_background);
 /// specially recognizing the command. Returns true if we validated the command. If so, returns by
 /// reference whether the suggestion is valid or not.
 bool autosuggest_validate_from_history(const history_item_t &item,
-                                       file_detection_context_t &detector,
                                        const wcstring &working_directory,
                                        const env_vars_snapshot_t &vars);
 

src/history.cpp

@@ -1661,53 +1661,25 @@ void history_sanity_check() {
     // No sanity checking implemented yet...
 }
 
-int file_detection_context_t::perform_file_detection(bool test_all) {
+path_list_t valid_paths(const path_list_t &paths, const wcstring &working_directory) {
     ASSERT_IS_BACKGROUND_THREAD();
-    valid_paths.clear();
-    // TODO: Figure out why this bothers to return a variable result since the only consumer,
-    // perform_file_detection_done(), ignores the result. It seems like either this should always
-    // return a constant or perform_file_detection_done() should use our return value.
-    int result = 1;
-    for (path_list_t::const_iterator iter = potential_paths.begin(); iter != potential_paths.end();
-         ++iter) {
-        if (path_is_valid(*iter, working_directory)) {
-            // Push the original (possibly relative) path.
-            valid_paths.push_back(*iter);
-        } else {
-            // Not a valid path.
-            result = 0;
-            if (!test_all) break;
+    wcstring_list_t result;
+    for (const wcstring &path : paths) {
+        if (path_is_valid(path, working_directory)) {
+            result.push_back(path);
         }
     }
     return result;
 }
 
-bool file_detection_context_t::paths_are_valid(const path_list_t &paths) {
-    this->potential_paths = paths;
-    return perform_file_detection(false) > 0;
-}
-
-file_detection_context_t::file_detection_context_t(history_t *hist, history_identifier_t ident)
-    : history(hist), working_directory(env_get_pwd_slash()), history_item_identifier(ident) {}
-
-static int threaded_perform_file_detection(file_detection_context_t *ctx) {
+bool all_paths_are_valid(const path_list_t &paths, const wcstring &working_directory) {
     ASSERT_IS_BACKGROUND_THREAD();
-    assert(ctx != NULL);
-    return ctx->perform_file_detection(true /* test all */);
-}
-
-static void perform_file_detection_done(file_detection_context_t *ctx, int success) {
-    UNUSED(success);
-    ASSERT_IS_MAIN_THREAD();
-
-    // Now that file detection is done, update the history item with the valid file paths.
-    ctx->history->set_valid_file_paths(ctx->valid_paths, ctx->history_item_identifier);
-
-    // Allow saving again.
-    ctx->history->enable_automatic_saving();
-
-    // Done with the context.
-    delete ctx;
+    for (const wcstring &path : paths) {
+        if (! path_is_valid(path, working_directory)) {
+            return false;
+        }
+    }
+    return true;
 }
 
 static bool string_could_be_path(const wcstring &potential_path) {
@@ -1720,7 +1692,6 @@ static bool string_could_be_path(const wcstring &potential_path) {
 
 void history_t::add_pending_with_file_detection(const wcstring &str) {
     ASSERT_IS_MAIN_THREAD();
-    path_list_t potential_paths;
 
     // Find all arguments that look like they could be file paths.
     bool impending_exit = false;
@@ -1728,6 +1699,7 @@ void history_t::add_pending_with_file_detection(const wcstring &str) {
     parse_tree_from_string(str, parse_flag_none, &tree, NULL);
     size_t count = tree.size();
 
+    path_list_t potential_paths;
     for (size_t i = 0; i < count; i++) {
         const parse_node_t &node = tree.at(i);
         if (!node.has_source()) {
@@ -1764,16 +1736,18 @@ void history_t::add_pending_with_file_detection(const wcstring &str) {
         static history_identifier_t sLastIdentifier = 0;
         identifier = ++sLastIdentifier;
 
-        // Create a new detection context.
-        file_detection_context_t *context = new file_detection_context_t(this, identifier);
-        context->potential_paths.swap(potential_paths);
-
         // Prevent saving until we're done, so we have time to get the paths.
         this->disable_automatic_saving();
 
-        // Kick it off. Even though we haven't added the item yet, it updates the item on the main
-        // thread, so we can't race.
-        iothread_perform(&threaded_perform_file_detection, &perform_file_detection_done, context);
+        // Check for which paths are valid on a background thread,
+        // then on the main thread update our history item
+        const wcstring wd = env_get_pwd_slash();
+        iothread_perform([=](){
+            return valid_paths(potential_paths, wd);
+        }, [=](path_list_t validated_paths){
+            this->set_valid_file_paths(validated_paths, identifier);
+            this->enable_automatic_saving();
+        });
     }
 
     // Actually add the item to the history.

src/history.h

@@ -341,34 +341,12 @@ void history_destroy();
 // Perform sanity checks.
 void history_sanity_check();
 
-// A helper class for threaded detection of paths.
-struct file_detection_context_t {
-    // Constructor.
-    explicit file_detection_context_t(history_t *hist, history_identifier_t ident = 0);
+// Given a list of paths and a working directory, return the paths that are valid
+// This does disk I/O and may only be called in a background thread
+path_list_t valid_paths(const path_list_t &paths, const wcstring &working_directory);
 
-    // Determine which of potential_paths are valid, and put them in valid_paths.
-    int perform_file_detection();
-
-    // The history associated with this context.
-    history_t *const history;
-
-    // The working directory at the time the command was issued.
-    wcstring working_directory;
-
-    // Paths to test.
-    path_list_t potential_paths;
-
-    // Paths that were found to be valid.
-    path_list_t valid_paths;
-
-    // Identifier of the history item to which we are associated.
-    const history_identifier_t history_item_identifier;
-
-    // Performs file detection. Returns 1 if every path in potential_paths is valid, 0 otherwise. If
-    // test_all is true, tests every path; otherwise stops as soon as it reaches an invalid path.
-    int perform_file_detection(bool test_all);
-
-    // Determine whether the given paths are all valid.
-    bool paths_are_valid(const path_list_t &paths);
-};
+// Given a list of paths and a working directory,
+// return true if all paths in the list are valid
+// Returns true for if paths is empty
+bool all_paths_are_valid(const path_list_t &paths, const wcstring &working_directory);
 #endif

src/reader.cpp

@@ -1127,6 +1127,8 @@ get_autosuggestion_performer(const wcstring &search_string, size_t cursor_pos, h
     // This is safe because histories are immortal, but perhaps
     // this should use shared_ptr
     return [=]() -> autosuggestion_result_t {
+        ASSERT_IS_BACKGROUND_THREAD();
+
         const autosuggestion_result_t nothing = {};
         // If the main thread has moved on, skip all the work.
         if (generation_count != s_generation_count) {
@@ -1142,14 +1144,13 @@ get_autosuggestion_performer(const wcstring &search_string, size_t cursor_pos, h
         }
 
         history_search_t searcher(*history, search_string, HISTORY_SEARCH_TYPE_PREFIX);
-        file_detection_context_t detector(history);
         while (!reader_thread_job_is_stale() && searcher.go_backwards()) {
             history_item_t item = searcher.current_item();
 
             // Skip items with newlines because they make terrible autosuggestions.
             if (item.str().find('\n') != wcstring::npos) continue;
 
-            if (autosuggest_validate_from_history(item, detector, working_directory, vars)) {
+            if (autosuggest_validate_from_history(item, working_directory, vars)) {
                 // The command autosuggestion was handled specially, so we're done.
                 return {searcher.current_string(), search_string};
             }