From a3dbca90d588c93baee05576b81e24b65f8dd81c Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 24 Jan 2017 09:30:30 -0800 Subject: [PATCH 1/6] g++-friendly 'Rework iothread_perform for void return types' Allows iothread_perform to work with void return types on both g++ and clang This reverts commit 1040b255c72571581a7de283d7f49c4633c851b4 and reintroduces ac9a0f0dbfeb8b76207202dcda2032faff6ad541 --- src/history.cpp | 2 +- src/iothread.cpp | 2 +- src/iothread.h | 42 ++++++++++++++++++++++++++++++++++-------- src/reader.cpp | 4 ++-- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/history.cpp b/src/history.cpp index a3e326976..aa27c7366 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -1773,7 +1773,7 @@ void history_t::add_pending_with_file_detection(const wcstring &str) { // 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); + iothread_perform(&threaded_perform_file_detection, &perform_file_detection_done, context); } // Actually add the item to the history. diff --git a/src/iothread.cpp b/src/iothread.cpp index 0ed6f52b1..7fe6c9caf 100644 --- a/src/iothread.cpp +++ b/src/iothread.cpp @@ -190,7 +190,7 @@ static void iothread_spawn() { VOMIT_ON_FAILURE(pthread_sigmask(SIG_SETMASK, &saved_set, NULL)); } -int iothread_perform(void_function_t &&func, void_function_t &&completion) { +int iothread_perform_impl(void_function_t &&func, void_function_t &&completion) { ASSERT_IS_MAIN_THREAD(); ASSERT_IS_NOT_FORKED_CHILD(); iothread_init(); diff --git a/src/iothread.h b/src/iothread.h index 5faa477e7..0b26a4209 100644 --- a/src/iothread.h +++ b/src/iothread.h @@ -26,16 +26,42 @@ void iothread_service_completion(void); /// Waits for all iothreads to terminate. void iothread_drain_all(void); -int iothread_perform(std::function &&func, - std::function &&completion = std::function()); +// Internal implementation +int iothread_perform_impl(std::function &&func, + std::function &&completion); -// Variant that allows computing a value in func, and then passing it to the completion handler +// Template helpers template -int iothread_perform(std::function &&handler, std::function &&completion) { - T *result = new T(); - return iothread_perform([=](){ *result = handler(); }, - [=](){ completion(std::move(*result)); delete result; } - ); +struct _iothread_trampoline { + template + static int perform(const HANDLER &handler, const COMPLETION &completion) { + T *result = new T(); + return iothread_perform_impl([=](){ *result = handler(); }, + [=](){ completion(std::move(*result)); delete result; }); + } +}; + + +// Void specialization +template<> +struct _iothread_trampoline { + template + static int perform(const HANDLER &handler, const COMPLETION &completion) { + return iothread_perform_impl(handler, completion); + } +}; + +// iothread_perform invokes a handler on a background thread, and then a completion function +// on the main thread. The value returned from the handler is passed to the completion. +template +int iothread_perform(const HANDLER &handler, const COMPLETION &completion) { + return _iothread_trampoline::perform(handler, completion); +} + +// variant of iothread_perform without a completion handler +inline int iothread_perform(std::function &&func) { + return iothread_perform_impl(std::move(func), + std::function()); } /// Legacy templates diff --git a/src/reader.cpp b/src/reader.cpp index b018a8fcd..de7c50fd9 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1222,7 +1222,7 @@ static void update_autosuggestion(void) { const editable_line_t *el = data->active_edit_line(); autosuggestion_context_t *ctx = new autosuggestion_context_t(data->history, el->text, el->position); - iothread_perform(threaded_autosuggest, autosuggest_completed, ctx); + iothread_perform(&threaded_autosuggest, &autosuggest_completed, ctx); } } @@ -2178,7 +2178,7 @@ static void reader_super_highlight_me_plenty(int match_highlight_pos_adjust, boo highlight_complete(ctx, result); } else { // Highlighting including I/O proceeds in the background. - iothread_perform(threaded_highlight, highlight_complete, ctx); + iothread_perform(&threaded_highlight, &highlight_complete, ctx); } highlight_search(); From 8af00d2b789a24f74c5bf1b2499bc43422ffa686 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 24 Jan 2017 08:23:24 -0800 Subject: [PATCH 2/6] Adopt lambda-style iothread_perform in syntax highlighting Allows eliminating background_highlight_context_t --- src/env.h | 6 ++-- src/reader.cpp | 95 ++++++++++++++++++-------------------------------- 2 files changed, 37 insertions(+), 64 deletions(-) diff --git a/src/env.h b/src/env.h index 314fb4e49..9060d6097 100644 --- a/src/env.h +++ b/src/env.h @@ -153,10 +153,10 @@ class env_vars_snapshot_t { std::map vars; bool is_current() const; - env_vars_snapshot_t(const env_vars_snapshot_t &); - void operator=(const env_vars_snapshot_t &); +public: + env_vars_snapshot_t(const env_vars_snapshot_t &) = default; + env_vars_snapshot_t &operator=(const env_vars_snapshot_t &) = default; - public: env_vars_snapshot_t(const wchar_t *const *keys); env_vars_snapshot_t(); diff --git a/src/reader.cpp b/src/reader.cpp index de7c50fd9..1bcaf1488 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -2072,50 +2072,6 @@ void reader_import_history_if_necessary(void) { } } -/// A class as the context pointer for a background (threaded) highlight operation. -class background_highlight_context_t { - public: - /// The string to highlight. - const wcstring string_to_highlight; - /// Color buffer. - std::vector colors; - /// The position to use for bracket matching. - const size_t match_highlight_pos; - /// Function for syntax highlighting. - const highlight_function_t highlight_function; - /// Environment variables. - const env_vars_snapshot_t vars; - /// When the request was made. - const double when; - /// Gen count at the time the request was made. - const unsigned int generation_count; - - background_highlight_context_t(const wcstring &pbuff, size_t phighlight_pos, - highlight_function_t phighlight_func) - : string_to_highlight(pbuff), - colors(pbuff.size(), 0), - match_highlight_pos(phighlight_pos), - highlight_function(phighlight_func), - vars(env_vars_snapshot_t::highlighting_keys), - when(timef()), - generation_count(s_generation_count) {} - - int perform_highlight() { - if (generation_count != s_generation_count) { - // The gen count has changed, so don't do anything. - return 0; - } - VOMIT_ON_FAILURE( - pthread_setspecific(generation_count_key, (void *)(uintptr_t)generation_count)); - - if (!string_to_highlight.empty()) { - highlight_function(string_to_highlight, colors, match_highlight_pos, NULL /* error */, - vars); - } - return 0; - } -}; - /// Called to set the highlight flag for search results. static void highlight_search(void) { if (!data->search_buff.empty() && !data->history_search.is_at_end()) { @@ -2131,26 +2087,46 @@ static void highlight_search(void) { } } -static void highlight_complete(background_highlight_context_t *ctx, int result) { - UNUSED(result); // ignored because of the indirect invocation via iothread_perform() +struct highlight_result_t { + std::vector colors; + wcstring text; +}; + +static void highlight_complete(highlight_result_t result) { ASSERT_IS_MAIN_THREAD(); - if (ctx->string_to_highlight == data->command_line.text) { + if (result.text == data->command_line.text) { // The data hasn't changed, so swap in our colors. The colors may not have changed, so do // nothing if they have not. - assert(ctx->colors.size() == data->command_line.size()); - if (data->colors != ctx->colors) { - data->colors.swap(ctx->colors); + assert(result.colors.size() == data->command_line.size()); + if (data->colors != result.colors) { + data->colors = std::move(result.colors); sanity_check(); highlight_search(); reader_repaint(); } } - - delete ctx; } -static int threaded_highlight(background_highlight_context_t *ctx) { - return ctx->perform_highlight(); +// Given text, bracket matching position, and whether IO is allowed, +// return a function that performs highlighting. The function may be invoked on a background thread. +static std::function get_highlight_performer(const wcstring &text, + long match_highlight_pos, bool no_io) { + env_vars_snapshot_t vars(env_vars_snapshot_t::highlighting_keys); + unsigned int generation_count = s_generation_count; + highlight_function_t highlight_func = no_io ? highlight_shell_no_io : data->highlight_function; + return [=]() -> highlight_result_t { + if (generation_count != s_generation_count) { + // The gen count has changed, so don't do anything. + return {}; + } + if (text.empty()) { + return {}; + } + VOMIT_ON_FAILURE(pthread_setspecific(generation_count_key, (void *)(uintptr_t)generation_count)); + std::vector colors(text.size(), 0); + highlight_func(text, colors, match_highlight_pos, NULL /* error */, vars); + return {std::move(colors), text}; + }; } /// Call specified external highlighting function and then do search highlighting. Lastly, clear the @@ -2169,16 +2145,13 @@ static void reader_super_highlight_me_plenty(int match_highlight_pos_adjust, boo reader_sanity_check(); - highlight_function_t highlight_func = no_io ? highlight_shell_no_io : data->highlight_function; - background_highlight_context_t *ctx = - new background_highlight_context_t(el->text, match_highlight_pos, highlight_func); + auto highlight_performer = get_highlight_performer(el->text, match_highlight_pos, no_io); if (no_io) { - // Highlighting without IO, we just do it. Note that highlight_complete deletes ctx. - int result = ctx->perform_highlight(); - highlight_complete(ctx, result); + // Highlighting without IO, we just do it. + highlight_complete(highlight_performer()); } else { // Highlighting including I/O proceeds in the background. - iothread_perform(&threaded_highlight, &highlight_complete, ctx); + iothread_perform(highlight_performer, &highlight_complete); } highlight_search(); From 87152877b2fd33d0064ffda1c64c1ca3babc480f Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 24 Jan 2017 09:27:11 -0800 Subject: [PATCH 3/6] Switch to use lambda-style iothread for autosuggestions Allows eliminating autosuggestion_context_t --- src/reader.cpp | 91 +++++++++++++++++++++++--------------------------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/src/reader.cpp b/src/reader.cpp index 1bcaf1488..0298ad17e 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1111,42 +1111,38 @@ static void completion_insert(const wchar_t *val, complete_flags_t flags) { reader_set_buffer_maintaining_pager(new_command_line, cursor); } -struct autosuggestion_context_t { +struct autosuggestion_result_t { + wcstring suggestion; wcstring search_string; - wcstring autosuggestion; - size_t cursor_pos; - history_search_t searcher; - file_detection_context_t detector; - const wcstring working_directory; - const env_vars_snapshot_t vars; - const unsigned int generation_count; - - autosuggestion_context_t(history_t *history, const wcstring &term, size_t pos) - : search_string(term), - cursor_pos(pos), - searcher(*history, term, HISTORY_SEARCH_TYPE_PREFIX), - detector(history), - working_directory(env_get_pwd_slash()), - vars(env_vars_snapshot_t::highlighting_keys), - generation_count(s_generation_count) {} - - // The function run in the background thread to determine an autosuggestion. - int threaded_autosuggest(void) { - ASSERT_IS_BACKGROUND_THREAD(); +}; +// Returns a function that can be invoked (potentially +// on a background thread) to determine the autosuggestion +static std::function +get_autosuggestion_performer(const wcstring &search_string, size_t cursor_pos, history_t *history) { + const unsigned int generation_count = s_generation_count; + const wcstring working_directory(env_get_pwd_slash()); + env_vars_snapshot_t vars(env_vars_snapshot_t::highlighting_keys); + // TODO: suspicious use of 'history' here + // This is safe because histories are immortal, but perhaps + // this should use shared_ptr + return [=]() -> autosuggestion_result_t { + const autosuggestion_result_t nothing = {}; // If the main thread has moved on, skip all the work. if (generation_count != s_generation_count) { - return 0; + return nothing; } - VOMIT_ON_FAILURE( - pthread_setspecific(generation_count_key, (void *)(uintptr_t)generation_count)); + VOMIT_ON_FAILURE(pthread_setspecific(generation_count_key, + (void *)(uintptr_t)generation_count)); // Let's make sure we aren't using the empty string. if (search_string.empty()) { - return 0; + return nothing; } + 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(); @@ -1155,23 +1151,22 @@ struct autosuggestion_context_t { if (autosuggest_validate_from_history(item, detector, working_directory, vars)) { // The command autosuggestion was handled specially, so we're done. - this->autosuggestion = searcher.current_string(); - return 1; + return {searcher.current_string(), search_string}; } } // Maybe cancel here. - if (reader_thread_job_is_stale()) return 0; + if (reader_thread_job_is_stale()) return nothing; // Here we do something a little funny. If the line ends with a space, and the cursor is not // at the end, don't use completion autosuggestions. It ends up being pretty weird seeing // stuff get spammed on the right while you go back to edit a line const wchar_t last_char = search_string.at(search_string.size() - 1); - const bool cursor_at_end = (this->cursor_pos == search_string.size()); - if (!cursor_at_end && iswspace(last_char)) return 0; + const bool cursor_at_end = (cursor_pos == search_string.size()); + if (!cursor_at_end && iswspace(last_char)) return nothing; // On the other hand, if the line ends with a quote, don't go dumping stuff after the quote. - if (wcschr(L"'\"", last_char) && cursor_at_end) return 0; + if (wcschr(L"'\"", last_char) && cursor_at_end) return nothing; // Try normal completions. std::vector completions; @@ -1179,18 +1174,15 @@ struct autosuggestion_context_t { completions_sort_and_prioritize(&completions); if (!completions.empty()) { const completion_t &comp = completions.at(0); - size_t cursor = this->cursor_pos; - this->autosuggestion = completion_apply_to_command_line( - comp.completion, comp.flags, this->search_string, &cursor, true /* append only */); - return 1; + size_t cursor = cursor_pos; + wcstring suggestion = completion_apply_to_command_line(comp.completion, comp.flags, + search_string, &cursor, + true /* append only */); + return {std::move(suggestion), search_string}; } - return 0; - } -}; - -static int threaded_autosuggest(autosuggestion_context_t *ctx) { - return ctx->threaded_autosuggest(); + return nothing; + }; } static bool can_autosuggest(void) { @@ -1202,15 +1194,17 @@ static bool can_autosuggest(void) { el == &data->command_line && el->text.find_first_not_of(whitespace) != wcstring::npos; } -static void autosuggest_completed(autosuggestion_context_t *ctx, int result) { - if (result && can_autosuggest() && ctx->search_string == data->command_line.text && - string_prefixes_string_case_insensitive(ctx->search_string, ctx->autosuggestion)) { +// Called after an autosuggestion has been computed on a background thread +static void autosuggest_completed(autosuggestion_result_t result) { + if (! result.suggestion.empty() && + can_autosuggest() && + result.search_string == data->command_line.text && + string_prefixes_string_case_insensitive(result.search_string, result.suggestion)) { // Autosuggestion is active and the search term has not changed, so we're good to go. - data->autosuggestion = ctx->autosuggestion; + data->autosuggestion = std::move(result.suggestion); sanity_check(); reader_repaint(); } - delete ctx; } static void update_autosuggestion(void) { @@ -1220,9 +1214,8 @@ static void update_autosuggestion(void) { if (data->allow_autosuggestion && !data->suppress_autosuggestion && !data->command_line.empty() && data->history_search.is_at_end()) { const editable_line_t *el = data->active_edit_line(); - autosuggestion_context_t *ctx = - new autosuggestion_context_t(data->history, el->text, el->position); - iothread_perform(&threaded_autosuggest, &autosuggest_completed, ctx); + auto performer = get_autosuggestion_performer(el->text, el->position, data->history); + iothread_perform(performer, &autosuggest_completed); } } From f29139e853e4e994e5d1310b2dc8a9adab885eca Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 24 Jan 2017 17:52:28 -0800 Subject: [PATCH 4/6] Change clients of file_detection_context_t to use lambdas Allows for eliminating file_detection_context_t --- src/highlight.cpp | 7 +---- src/highlight.h | 1 - src/history.cpp | 70 +++++++++++++++-------------------------------- src/history.h | 36 +++++------------------- src/reader.cpp | 5 ++-- 5 files changed, 33 insertions(+), 86 deletions(-) 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}; } From 2f29473efc263d006907eff0d742eaaef1b37b70 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 26 Jan 2017 09:25:15 -0800 Subject: [PATCH 5/6] Remove legacy iothread_perform templates We no longer need to have overloads for function pointers. --- src/iothread.h | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/iothread.h b/src/iothread.h index 0b26a4209..f3af54fbf 100644 --- a/src/iothread.h +++ b/src/iothread.h @@ -64,19 +64,6 @@ inline int iothread_perform(std::function &&func) { std::function()); } -/// Legacy templates -template -int iothread_perform(int (*handler)(T *), void (*completion)(T *, int), T *context) { - return iothread_perform(std::function([=](){return handler(context);}), - std::function([=](int v){completion(context, v);}) - ); -} - -template -int iothread_perform(int (*handler)(T *), T *context) { - return iothread_perform([=](){ handler(context); }); -} - /// Performs a function on the main thread, blocking until it completes. void iothread_perform_on_main(std::function &&func); From 37578d8b39e094ba7bb50a265205fc7a89b9af88 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 26 Jan 2017 09:33:03 -0800 Subject: [PATCH 6/6] Add some additional commenting around iothreads --- src/iothread.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/iothread.h b/src/iothread.h index f3af54fbf..7e5500ed0 100644 --- a/src/iothread.h +++ b/src/iothread.h @@ -31,11 +31,16 @@ int iothread_perform_impl(std::function &&func, std::function &&completion); // Template helpers +// This is the glue part of the handler-completion handoff +// In general we can just allocate an object, move the result of the handler into it, +// and then call the completion with that object. However if our type is void, +// this won't work (new void() fails!). So we have to use this template. +// The type T is the return type of HANDLER and the argument to COMPLETION template struct _iothread_trampoline { template static int perform(const HANDLER &handler, const COMPLETION &completion) { - T *result = new T(); + T *result = new T(); // TODO: placement new? return iothread_perform_impl([=](){ *result = handler(); }, [=](){ completion(std::move(*result)); delete result; }); } @@ -53,6 +58,8 @@ struct _iothread_trampoline { // iothread_perform invokes a handler on a background thread, and then a completion function // on the main thread. The value returned from the handler is passed to the completion. +// In other words, this is like COMPLETION(HANDLER()) except the handler part is invoked +// on a background thread. template int iothread_perform(const HANDLER &handler, const COMPLETION &completion) { return _iothread_trampoline::perform(handler, completion);