From 68a28106b2dd4bb962e945340579f2d64d3a4066 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 27 Apr 2019 13:05:12 -0700 Subject: [PATCH] Reimplement completion autoloading via autoloader_t This switches the completion autoloading machinery to autoloader_t. --- src/autoload.cpp | 12 ++++++++++++ src/autoload.h | 4 ++++ src/complete.cpp | 25 +++++++++++++++++++++---- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/autoload.cpp b/src/autoload.cpp index c91110db6..46588334b 100644 --- a/src/autoload.cpp +++ b/src/autoload.cpp @@ -151,12 +151,24 @@ maybe_t autoload_file_cache_t::check(const wcstring &cmd, b autoloader_t::autoloader_t(wcstring env_var_name) : env_var_name_(std::move(env_var_name)), cache_(make_unique()) {} +autoloader_t::autoloader_t(autoloader_t &&) = default; autoloader_t::~autoloader_t() = default; bool autoloader_t::can_autoload(const wcstring &cmd) { return cache_->check(cmd, true /* allow stale */).has_value(); } +wcstring_list_t autoloader_t::get_autoloaded_commands() const { + wcstring_list_t result; + result.reserve(autoloaded_files_.size()); + for (const auto &kv : autoloaded_files_) { + result.push_back(kv.first); + } + // Sort the output to make it easier to test. + std::sort(result.begin(), result.end()); + return result; +} + maybe_t autoloader_t::resolve_command(const wcstring &cmd, const environment_t &env) { // Are we currently in the process of autoloading this? if (current_autoloading_.count(cmd) > 0) return none(); diff --git a/src/autoload.h b/src/autoload.h index df5ea1575..0910923ee 100644 --- a/src/autoload.h +++ b/src/autoload.h @@ -42,6 +42,7 @@ class autoloader_t { /// Construct an autoloader that loads from the paths given by \p env_var_name. explicit autoloader_t(wcstring env_var_name); + autoloader_t(autoloader_t &&); ~autoloader_t(); /// Given a command, get a path to autoload. @@ -75,6 +76,9 @@ class autoloader_t { /// This does not actually mark the command as being autoloaded. bool can_autoload(const wcstring &cmd); + /// \return the names of all commands that have been autoloaded. + wcstring_list_t get_autoloaded_commands() const; + /// Mark that all autoloaded files have been forgotten. /// Future calls to path_to_autoload() will return previously-returned paths. void clear() { diff --git a/src/complete.cpp b/src/complete.cpp index 46d9f97d2..430463fe3 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -380,8 +380,8 @@ class completer_t { std::vector acquire_completions() { return std::move(completions); } }; -// Autoloader for completions -static autoload_t completion_autoloader(L"fish_complete_path"); +// Autoloader for completions. +static owning_lock completion_autoloader{autoloader_t(L"fish_complete_path")}; /// Create a new completion entry. void append_completion(std::vector *completions, wcstring comp, wcstring desc, @@ -860,7 +860,16 @@ static void complete_load(const wcstring &name, bool reload) { // We have to load this as a function, since it may define a --wraps or signature. // See issue #2466. function_load(name); - completion_autoloader.load(name, reload); + + // It's important to NOT hold the lock around completion loading. + // We need to take the lock to decide what to load, drop it to perform the load, then reacquire + // it. + const environment_t &vars = parser_t::principal_parser().vars(); + maybe_t path_to_load = completion_autoloader.acquire()->resolve_command(name, vars); + if (path_to_load) { + autoloader_t::perform_autoload(*path_to_load); + completion_autoloader.acquire()->mark_autoload_finished(name); + } } /// complete_param: Given a command, find completions for the argument str of command cmd_orig with @@ -1659,7 +1668,15 @@ wcstring complete_print() { return out; } -void complete_invalidate_path() { completion_autoloader.invalidate(); } +void complete_invalidate_path() { + // TODO: here we unload all completions for commands that are loaded by the autoloader. We also + // unload any completions that the user may specified on the command line. We should in + // principle track those completions loaded by the autoloader alone. + wcstring_list_t cmds = completion_autoloader.acquire()->get_autoloaded_commands(); + for (const wcstring &cmd : cmds) { + complete_remove_all(cmd, false /* not a path */); + } +} /// Add a new target that wraps a command. Example: __fish_XYZ (function) wraps XYZ (target). bool complete_add_wrapper(const wcstring &command, const wcstring &new_target) {