diff --git a/src/highlight.cpp b/src/highlight.cpp index 3019db942..7d2fffaaf 100644 --- a/src/highlight.cpp +++ b/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; diff --git a/src/highlight.h b/src/highlight.h index e708733de..a63cb28dd 100644 --- a/src/highlight.h +++ b/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); diff --git a/src/history.cpp b/src/history.cpp index aa27c7366..9579c8b27 100644 --- a/src/history.cpp +++ b/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. diff --git a/src/history.h b/src/history.h index 1cb9e0bf7..11aece2c0 100644 --- a/src/history.h +++ b/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 diff --git a/src/reader.cpp b/src/reader.cpp index 0298ad17e..59b02d530 100644 --- a/src/reader.cpp +++ b/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}; }