From 1d205d0bbda2acd9c261248e0f141c813d2a96cb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 7 Aug 2022 13:31:10 -0700 Subject: [PATCH] Reimplement abbreviation expansion to support quiet abbreviations This reimplements abbreviation to support quiet abbreviations. Quiet abbreviations expand "in secret" before execution. --- src/abbrs.cpp | 32 +++- src/abbrs.h | 36 ++-- src/builtins/abbr.cpp | 70 +++----- src/fish_tests.cpp | 6 +- src/flog.h | 2 + src/highlight.cpp | 2 +- src/reader.cpp | 383 ++++++++++++++++++++++++++-------------- src/reader.h | 3 +- tests/checks/abbr.fish | 14 ++ tests/pexpects/abbrs.py | 48 ++++- 10 files changed, 384 insertions(+), 212 deletions(-) diff --git a/src/abbrs.cpp b/src/abbrs.cpp index 7da84ce2f..4891df93e 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -14,9 +14,25 @@ abbreviation_t::abbreviation_t(wcstring name, wcstring key, wcstring replacement position(position), from_universal(from_universal) {} -bool abbreviation_t::matches(const wcstring &token, abbrs_position_t position) const { - // We must either expands anywhere, or in the given position. - if (this->position != position && this->position != abbrs_position_t::anywhere) { +bool abbreviation_t::matches_position(abbrs_position_t position) const { + return this->position == abbrs_position_t::anywhere || this->position == position; +} + +bool abbreviation_t::matches_phase(abbrs_phase_t phase) const { + switch (phase) { + case abbrs_phase_t::noisy: + return !this->is_quiet; + case abbrs_phase_t::quiet: + return this->is_quiet; + case abbrs_phase_t::any: + return true; + } + DIE("Unreachable"); +} + +bool abbreviation_t::matches(const wcstring &token, abbrs_position_t position, + abbrs_phase_t phase) const { + if (!this->matches_position(position) || !this->matches_phase(phase)) { return false; } if (this->is_regex()) { @@ -31,21 +47,23 @@ acquired_lock abbrs_get_set() { return abbrs.acquire(); } -abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t position) const { +abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t position, + abbrs_phase_t phase) const { abbrs_replacer_list_t result{}; // Later abbreviations take precedence so walk backwards. for (auto it = abbrs_.rbegin(); it != abbrs_.rend(); ++it) { const abbreviation_t &abbr = *it; - if (abbr.matches(token, position)) { + if (abbr.matches(token, position, phase)) { result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function}); } } return result; } -bool abbrs_set_t::has_match(const wcstring &token, abbrs_position_t position) const { +bool abbrs_set_t::has_match(const wcstring &token, abbrs_position_t position, + abbrs_phase_t phase) const { for (const auto &abbr : abbrs_) { - if (abbr.matches(token, position)) { + if (abbr.matches(token, position, phase)) { return true; } } diff --git a/src/abbrs.h b/src/abbrs.h index db5ff5e45..3880f8add 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -18,6 +18,13 @@ enum class abbrs_position_t : uint8_t { anywhere, // expand in any token }; +/// Describes a phase of expansion. +enum class abbrs_phase_t : uint8_t { + noisy, // expand noisy abbreviations, which visibly replace their tokens. + quiet, // expand quiet abbreviations, which do not visibly replace their tokens. + any, // phase is ignored - useful for syntax highlighting. +}; + struct abbreviation_t { // Abbreviation name. This is unique within the abbreviation set. // This is used as the token to match unless we have a regex. @@ -44,17 +51,15 @@ struct abbreviation_t { /// Mark if we came from a universal variable. bool from_universal{}; - /// Whether this abbrevation expands after entry (before execution). - bool expand_on_entry{true}; - - /// Whether this abbrevation expands on execution. - bool expand_on_execute{true}; + /// Whether this abbrevation is quiet. Noisy abbreviations visibly replace their tokens in the + /// command line any history, quiet ones do not (unless expansion results in a syntax error). + bool is_quiet{false}; // \return true if this is a regex abbreviation. bool is_regex() const { return this->regex.has_value(); } - // \return true if we match a token at a given position. - bool matches(const wcstring &token, abbrs_position_t position) const; + // \return true if we match a token at a given position in a given phase. + bool matches(const wcstring &token, abbrs_position_t position, abbrs_phase_t phase) const; // Construct from a name, a key which matches a token, a replacement token, a position, and // whether we are derived from a universal variable. @@ -63,6 +68,13 @@ struct abbreviation_t { bool from_universal = false); abbreviation_t() = default; + + private: + // \return if we expand in a given position. + bool matches_position(abbrs_position_t position) const; + + // \return if we expand in a given phase. + bool matches_phase(abbrs_phase_t phase) const; }; /// The result of an abbreviation expansion. @@ -79,10 +91,11 @@ class abbrs_set_t { public: /// \return the list of replacers for an input token, in priority order. /// The \p position is given to describe where the token was found. - abbrs_replacer_list_t match(const wcstring &token, abbrs_position_t position) const; + abbrs_replacer_list_t match(const wcstring &token, abbrs_position_t position, + abbrs_phase_t phase) const; /// \return whether we would have at least one replacer for a given token. - bool has_match(const wcstring &token, abbrs_position_t position) const; + bool has_match(const wcstring &token, abbrs_position_t position, abbrs_phase_t phase) const; /// Add an abbreviation. Any abbreviation with the same name is replaced. void add(abbreviation_t &&abbr); @@ -118,8 +131,9 @@ acquired_lock abbrs_get_set(); /// \return the list of replacers for an input token, in priority order, using the global set. /// The \p position is given to describe where the token was found. -inline abbrs_replacer_list_t abbrs_match(const wcstring &token, abbrs_position_t position) { - return abbrs_get_set()->match(token, position); +inline abbrs_replacer_list_t abbrs_match(const wcstring &token, abbrs_position_t position, + abbrs_phase_t phase) { + return abbrs_get_set()->match(token, position, phase); } #endif diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 90d4948e4..9880e06dc 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -42,8 +42,7 @@ struct abbr_options_t { maybe_t regex_pattern; maybe_t position{}; - bool expand_on_entry{}; - bool expand_on_execute{}; + bool quiet{}; wcstring_list_t args; @@ -80,17 +79,10 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --function option requires --add\n"), CMD); return false; } - if (!add && (expand_on_entry || expand_on_execute)) { - streams.err.append_format(_(L"%ls: --expand-on option requires --add\n"), CMD); + if (!add && quiet) { + streams.err.append_format(_(L"%ls: --quiet option requires --add\n"), CMD); return false; } - - // If no expand-on is specified, expand on both entry and execute, to match historical - // behavior. - if (add && !expand_on_entry && !expand_on_execute) { - expand_on_entry = true; - expand_on_execute = true; - } return true; } }; @@ -111,15 +103,8 @@ static int abbr_show(const abbr_options_t &, io_streams_t &streams) { comps.push_back(L"--regex"); comps.push_back(escape_string(abbr.key)); } - // The default is to expand on both entry and execute. - // Add flags if we're not the default. - if (!(abbr.expand_on_entry && abbr.expand_on_execute)) { - if (abbr.expand_on_entry) { - comps.push_back(L"--expand-on entry"); - } - if (abbr.expand_on_execute) { - comps.push_back(L"--expand-on execute"); - } + if (abbr.is_quiet) { + comps.push_back(L"--quiet"); } if (abbr.replacement_is_function) { comps.push_back(L"--function"); @@ -259,6 +244,7 @@ static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { abbreviation_t abbr{std::move(name), std::move(key), std::move(replacement), position}; abbr.regex = std::move(regex); abbr.replacement_is_function = opts.function; + abbr.is_quiet = opts.quiet; abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -287,27 +273,26 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t const wchar_t *cmd = argv[0]; abbr_options_t opts; // Note 1 is returned by wgetopt to indicate a non-option argument. - enum { NON_OPTION_ARGUMENT = 1, REGEX_SHORT, EXPAND_ON_SHORT }; + enum { NON_OPTION_ARGUMENT = 1, REGEX_SHORT, EXPAND_ON_SHORT, QUIET_SHORT }; // Note the leading '-' causes wgetopter to return arguments in order, instead of permuting // them. We need this behavior for compatibility with pre-builtin abbreviations where options // could be given literally, for example `abbr e emacs -nw`. static const wchar_t *const short_options = L"-afrseqgUh"; - static const struct woption long_options[] = { - {L"add", no_argument, 'a'}, - {L"position", required_argument, 'p'}, - {L"regex", required_argument, REGEX_SHORT}, - {L"expand-on", required_argument, EXPAND_ON_SHORT}, - {L"function", no_argument, 'f'}, - {L"rename", no_argument, 'r'}, - {L"erase", no_argument, 'e'}, - {L"query", no_argument, 'q'}, - {L"show", no_argument, 's'}, - {L"list", no_argument, 'l'}, - {L"global", no_argument, 'g'}, - {L"universal", no_argument, 'U'}, - {L"help", no_argument, 'h'}, - {}}; + static const struct woption long_options[] = {{L"add", no_argument, 'a'}, + {L"position", required_argument, 'p'}, + {L"regex", required_argument, REGEX_SHORT}, + {L"quiet", no_argument, QUIET_SHORT}, + {L"function", no_argument, 'f'}, + {L"rename", no_argument, 'r'}, + {L"erase", no_argument, 'e'}, + {L"query", no_argument, 'q'}, + {L"show", no_argument, 's'}, + {L"list", no_argument, 'l'}, + {L"global", no_argument, 'g'}, + {L"universal", no_argument, 'U'}, + {L"help", no_argument, 'h'}, + {}}; int argc = builtin_count_args(argv); int opt; @@ -356,17 +341,8 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t opts.regex_pattern = w.woptarg; break; } - case EXPAND_ON_SHORT: { - if (!wcscmp(w.woptarg, L"entry")) { - opts.expand_on_entry = true; - } else if (!wcscmp(w.woptarg, L"execute")) { - opts.expand_on_execute = true; - } else { - streams.err.append_format(_(L"%ls: Invalid expand-on '%ls'\nexpand-on must be " - L"one of: entry, execute.\n"), - CMD, w.woptarg); - return STATUS_INVALID_ARGS; - } + case QUIET_SHORT: { + opts.quiet = true; break; } case 'f': diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index fd0fed0ad..b7bc8b24f 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2481,7 +2481,7 @@ static void test_abbreviations() { // Helper to expand an abbreviation, enforcing we have no more than one result. auto abbr_expand_1 = [](const wcstring &token, abbrs_position_t pos) -> maybe_t { - auto result = abbrs_match(token, pos); + auto result = abbrs_match(token, pos, abbrs_phase_t::any); if (result.size() > 1) { err(L"abbreviation expansion for %ls returned more than 1 result", token.c_str()); } @@ -2506,8 +2506,8 @@ static void test_abbreviations() { maybe_t result; auto expand_abbreviation_in_command = [](const wcstring &cmdline, size_t cursor_pos) -> maybe_t { - if (auto edit = reader_expand_abbreviation_at_cursor(cmdline, cursor_pos, - parser_t::principal_parser())) { + if (auto edit = reader_expand_abbreviation_at_cursor( + cmdline, cursor_pos, abbrs_phase_t::noisy, parser_t::principal_parser())) { wcstring cmdline_expanded = cmdline; std::vector colors{cmdline_expanded.size()}; apply_edit(&cmdline_expanded, &colors, *edit); diff --git a/src/flog.h b/src/flog.h index b1ecfaad5..4a3627f3f 100644 --- a/src/flog.h +++ b/src/flog.h @@ -109,6 +109,8 @@ class category_list_t { category_t path{L"path", L"Searching/using paths"}; category_t screen{L"screen", L"Screen repaints"}; + + category_t abbrs{L"abbrs", L"Abbreviation expansion"}; }; /// The class responsible for logging. diff --git a/src/highlight.cpp b/src/highlight.cpp index ce0f29977..f4ad0e32f 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -1336,7 +1336,7 @@ static bool command_is_valid(const wcstring &cmd, enum statement_decoration_t de // Abbreviations if (!is_valid && abbreviation_ok) - is_valid = abbrs_get_set()->has_match(cmd, abbrs_position_t::command); + is_valid = abbrs_get_set()->has_match(cmd, abbrs_position_t::command, abbrs_phase_t::any); // Regular commands if (!is_valid && command_ok) is_valid = path_get_path(cmd, vars).has_value(); diff --git a/src/reader.cpp b/src/reader.cpp index 57bf266e4..89899e49c 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -618,6 +618,10 @@ struct readline_loop_state_t { /// command. bool finished{false}; + /// If the loop is finished, then the command to execute; this may differ from the command line + /// itself due to quiet abbreviations. + wcstring cmd; + /// Maximum number of characters to read. size_t nchars{std::numeric_limits::max()}; }; @@ -789,7 +793,7 @@ class reader_data_t : public std::enable_shared_from_this { void pager_selection_changed(); /// Expand abbreviations at the current cursor position, minus backtrack_amt. - bool expand_abbreviation_as_necessary(size_t cursor_backtrack); + bool expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_phase_t phase); /// \return true if the command line has changed and repainting is needed. If \p colors is not /// null, then also return true if the colors have changed. @@ -872,8 +876,19 @@ class reader_data_t : public std::enable_shared_from_this { void handle_readline_command(readline_cmd_t cmd, readline_loop_state_t &rls); // Handle readline_cmd_t::execute. This may mean inserting a newline if the command is - // unfinished. - void handle_execute(readline_loop_state_t &rls); + // unfinished. It may also set 'finished' and 'cmd' inside the rls. + // \return true on success, false if we got an error, in which case the caller should fire the + // error event. + bool handle_execute(readline_loop_state_t &rls); + + // Add the current command line contents to history. + void add_to_history() const; + + // Expand abbreviations before execution. + // Replace the command line with any noisy abbreviations as needed. + // Populate \p to_exec with the command to execute, which may include silent abbreviations. + // \return the test result, which may be incomplete to insert a newline, or an error. + parser_test_error_bits_t expand_for_execute(wcstring *to_exec); void clear_pager(); void select_completion_in_direction(selection_motion_t dir, @@ -1348,11 +1363,13 @@ void reader_data_t::pager_selection_changed() { } /// Expand an abbreviation replacer, which means either returning its literal replacement or running -/// its function. \return the replacement string, or none to skip it. +/// its function. \return the replacement string, or none to skip it. This may run fish script! maybe_t expand_replacer(const wcstring &token, const abbrs_replacer_t &repl, parser_t &parser) { if (!repl.is_function) { // Literal replacement cannot fail. + FLOGF(abbrs, L"Expanded literal abbreviation <%ls> -> <%ls>", token.c_str(), + repl.replacement.c_str()); return repl.replacement; } @@ -1360,76 +1377,98 @@ maybe_t expand_replacer(const wcstring &token, const abbrs_replacer_t cmd.push_back(L' '); cmd.append(escape_string(token)); + scoped_push not_interactive(&parser.libdata().is_interactive, false); + wcstring_list_t outputs{}; int ret = exec_subshell(cmd, parser, outputs, false /* not apply_exit_status */); if (ret != STATUS_CMD_OK) { return none(); } - return join_strings(outputs, L'\n'); + wcstring result = join_strings(outputs, L'\n'); + FLOGF(abbrs, L"Expanded function abbreviation <%ls> -> <%ls>", token.c_str(), result.c_str()); + return result; +} + +// Extract all the token ranges in \p str, along with whether they are an undecorated command. +// Tokens containing command substitutions are skipped; this ensures tokens are non-overlapping. +struct positioned_token_t { + source_range_t range; + bool is_cmd; +}; +static std::vector extract_tokens(const wcstring &str) { + using namespace ast; + + parse_tree_flags_t ast_flags = parse_flag_continue_after_error | + parse_flag_accept_incomplete_tokens | + parse_flag_leave_unterminated; + auto ast = ast::ast_t::parse(str, ast_flags); + + // Helper to check if a node is the command portion of an undecorated statement. + auto is_command = [&](const node_t *node) { + for (const node_t *cursor = node; cursor; cursor = cursor->parent) { + if (const auto *stmt = cursor->try_as()) { + if (!stmt->opt_decoration && node == &stmt->command) { + return true; + } + } + } + return false; + }; + + wcstring cmdsub_contents; + std::vector result; + traversal_t tv = ast.walk(); + while (const node_t *node = tv.next()) { + // We are only interested in leaf nodes with source. + if (node->category != category_t::leaf) continue; + source_range_t r = node->source_range(); + if (r.length == 0) continue; + + // If we have command subs, then we don't include this token; instead we recurse. + bool has_cmd_subs = false; + size_t cmdsub_cursor = r.start, cmdsub_start = 0, cmdsub_end = 0; + while (parse_util_locate_cmdsubst_range(str, &cmdsub_cursor, &cmdsub_contents, + &cmdsub_start, &cmdsub_end, + true /* accept incomplete */) > 0) { + if (cmdsub_start >= r.end()) { + break; + } + has_cmd_subs = true; + for (positioned_token_t t : extract_tokens(cmdsub_contents)) { + // cmdsub_start is the open paren; the contents start one after it. + t.range.start += static_cast(cmdsub_start + 1); + result.push_back(t); + } + } + + if (!has_cmd_subs) { + // Common case of no command substitutions in this leaf node. + result.push_back(positioned_token_t{r, is_command(node)}); + } + } + return result; } /// Expand abbreviations at the given cursor position. Does NOT inspect 'data'. maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - parser_t &parser) { - // Get the surrounding command substitution. - const wchar_t *const buff = cmdline.c_str(); - const wchar_t *cmdsub_begin = nullptr, *cmdsub_end = nullptr; - parse_util_cmdsubst_extent(buff, cursor_pos, &cmdsub_begin, &cmdsub_end); - assert(cmdsub_begin != nullptr && cmdsub_begin >= buff); - assert(cmdsub_end != nullptr && cmdsub_end >= cmdsub_begin); - - // Determine the offset of this command substitution. - const size_t subcmd_offset = cmdsub_begin - buff; - - const wcstring subcmd = wcstring(cmdsub_begin, cmdsub_end - cmdsub_begin); - const size_t subcmd_cursor_pos = cursor_pos - subcmd_offset; - - // Parse this subcmd. - using namespace ast; - auto ast = - ast_t::parse(subcmd, parse_flag_continue_after_error | parse_flag_accept_incomplete_tokens | - parse_flag_leave_unterminated); - - // Find a leaf node where the cursor is at its end. - const node_t *leaf = nullptr; - traversal_t tv = ast.walk(); - while (const node_t *node = tv.next()) { - if (node->category == category_t::leaf) { - auto r = node->try_source_range(); - if (r && r->start <= subcmd_cursor_pos && subcmd_cursor_pos <= r->end()) { - leaf = node; - break; - } - } - } - if (!leaf) { + abbrs_phase_t phase, parser_t &parser) { + // Find the token containing the cursor. Usually users edit from the end, so walk backwards. + const auto tokens = extract_tokens(cmdline); + auto iter = std::find_if(tokens.rbegin(), tokens.rend(), [&](const positioned_token_t &t) { + return t.range.contains_inclusive(cursor_pos); + }); + if (iter == tokens.rend()) { return none(); } + source_range_t range = iter->range; + abbrs_position_t position = + iter->is_cmd ? abbrs_position_t::command : abbrs_position_t::anywhere; - // We found the leaf node before the cursor. - // Decide if this leaf is in "command position:" it is the command part of an undecorated - // statement. - bool leaf_is_command = false; - for (const node_t *cursor = leaf; cursor; cursor = cursor->parent) { - if (const auto *stmt = cursor->try_as()) { - if (!stmt->opt_decoration && leaf == &stmt->command) { - leaf_is_command = true; - break; - } - } - } - abbrs_position_t expand_position = - leaf_is_command ? abbrs_position_t::command : abbrs_position_t::anywhere; - - // Now we can expand the abbreviation. - // Find the first which succeeds. - wcstring token = leaf->source(subcmd); - auto replacers = abbrs_match(token, expand_position); + wcstring token_str = cmdline.substr(range.start, range.length); + auto replacers = abbrs_match(token_str, position, phase); for (const auto &replacer : replacers) { - if (auto replacement = expand_replacer(token, replacer, parser)) { - // Replace the token in the full command. Maintain the relative position of the cursor. - source_range_t r = leaf->source_range(); - return edit_t(subcmd_offset + r.start, r.length, replacement.acquire()); + if (auto replacement = expand_replacer(token_str, replacer, parser)) { + return edit_t{range.start, range.length, replacement.acquire()}; } } return none(); @@ -1438,7 +1477,7 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, si /// Expand abbreviations at the current cursor position, minus the given cursor backtrack. This may /// change the command line but does NOT repaint it. This is to allow the caller to coalesce /// repaints. -bool reader_data_t::expand_abbreviation_as_necessary(size_t cursor_backtrack) { +bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_phase_t phase) { bool result = false; editable_line_t *el = active_edit_line(); @@ -1446,7 +1485,8 @@ bool reader_data_t::expand_abbreviation_as_necessary(size_t cursor_backtrack) { // Try expanding abbreviations. this->update_commandline_state(); size_t cursor_pos = el->position() - std::min(el->position(), cursor_backtrack); - if (auto edit = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, parser())) { + if (auto edit = + reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, phase, parser())) { push_edit(el, std::move(*edit)); update_buff_pos(el); result = true; @@ -1455,6 +1495,44 @@ bool reader_data_t::expand_abbreviation_as_necessary(size_t cursor_backtrack) { return result; } +// Expand any quiet abbreviations, modifying \p str in place. +// \return true if the string changed, false if not. +static bool expand_quiet_abbreviations(wcstring *inout_str, parser_t &parser) { + bool modified = false; + const wcstring &str = *inout_str; + wcstring result = str; + // We may have multiple replacements. + // Keep track of the change in length. + size_t orig_lengths = 0; + size_t repl_lengths = 0; + + const std::vector tokens = extract_tokens(result); + wcstring token; + for (positioned_token_t pt : tokens) { + source_range_t orig_range = pt.range; + token.assign(str, orig_range.start, orig_range.length); + + abbrs_position_t position = + pt.is_cmd ? abbrs_position_t::command : abbrs_position_t::anywhere; + auto replacers = abbrs_match(token, position, abbrs_phase_t::quiet); + for (const auto &replacer : replacers) { + const auto replacement = expand_replacer(token, replacer, parser); + if (replacement && *replacement != token) { + modified = true; + result.replace(orig_range.start + repl_lengths - orig_lengths, orig_range.length, + *replacement); + orig_lengths += orig_range.length; + repl_lengths += replacement->length(); + break; + } + } + } + if (modified) { + *inout_str = std::move(result); + } + return modified; +} + void reader_reset_interrupted() { interrupted = 0; } int reader_test_and_clear_interrupted() { @@ -3619,7 +3697,10 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat break; } case rl::execute: { - this->handle_execute(rls); + if (!this->handle_execute(rls)) { + event_fire_generic(parser(), L"fish_posterror", {command_line.text()}); + screen.reset_abandoning_line(termsize_last().width); + } break; } @@ -4146,7 +4227,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } case rl::expand_abbr: { - if (expand_abbreviation_as_necessary(1)) { + if (expand_abbreviation_at_cursor(1, abbrs_phase_t::noisy)) { inputter.function_set_status(true); } else { inputter.function_set_status(false); @@ -4193,13 +4274,94 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } } -void reader_data_t::handle_execute(readline_loop_state_t &rls) { +void reader_data_t::add_to_history() const { + if (!history || conf.in_silent_mode) { + return; + } + + // Historical behavior is to trim trailing spaces, unless escape (#7661). + wcstring text = command_line.text(); + while (!text.empty() && text.back() == L' ' && + count_preceding_backslashes(text, text.size() - 1) % 2 == 0) { + text.pop_back(); + } + + // Remove ephemeral items - even if the text is empty. + history->remove_ephemeral_items(); + + if (!text.empty()) { + // Mark this item as ephemeral if there is a leading space (#615). + history_persistence_mode_t mode; + if (text.front() == L' ') { + // Leading spaces are ephemeral (#615). + mode = history_persistence_mode_t::ephemeral; + } else if (in_private_mode(this->vars())) { + // Private mode means in-memory only. + mode = history_persistence_mode_t::memory; + } else { + mode = history_persistence_mode_t::disk; + } + history_t::add_pending_with_file_detection(history, text, this->vars().snapshot(), mode); + } +} + +parser_test_error_bits_t reader_data_t::expand_for_execute(wcstring *to_exec) { + // Expand abbreviations. First noisy abbreviations at the cursor, followed by quiet + // abbreviations, anywhere that matches. + // The first expansion is "user visible" and enters into history. + // The second happens silently, unless it produces an error, in which case we show the text to + // the user. + // We update the command line with the user-visible result and then return the silent result by + // reference. + + assert(to_exec && "Null pointer"); + editable_line_t *el = &command_line; + parser_test_error_bits_t test_res = 0; + + // Syntax check before expanding abbreviations. We could consider relaxing this: a string may be + // syntactically invalid but become valid after expanding abbreviations. + if (conf.syntax_check_ok) { + test_res = reader_shell_test(parser(), el->text()); + if (test_res & PARSER_TEST_ERROR) return test_res; + } + + // Noisy abbreviations at the cursor. + // Note we want to expand abbreviations even if incomplete. + if (expand_abbreviation_at_cursor(0, abbrs_phase_t::noisy)) { + // Trigger syntax highlighting as we are likely about to execute this command. + this->super_highlight_me_plenty(); + if (conf.syntax_check_ok) { + test_res = reader_shell_test(parser(), el->text()); + } + } + + // We may have an error or be incomplete. + if (test_res) return test_res; + + // Quiet abbreviations anywhere. + wcstring text_for_exec = el->text(); + if (expand_quiet_abbreviations(&text_for_exec, parser())) { + if (conf.syntax_check_ok) { + test_res = reader_shell_test(parser(), text_for_exec); + if (test_res) { + // Replace the command line with an undoable edit, to make the replacement visible. + push_edit(el, edit_t(0, el->size(), std::move(text_for_exec))); + return test_res; + } + } + } + + *to_exec = std::move(text_for_exec); + return 0; +} + +bool reader_data_t::handle_execute(readline_loop_state_t &rls) { // Evaluate. If the current command is unfinished, or if the charater is escaped // using a backslash, insert a newline. // If the user hits return while navigating the pager, it only clears the pager. if (is_navigating_pager_contents()) { clear_pager(); - return; + return true; } // Delete any autosuggestion. @@ -4232,77 +4394,26 @@ void reader_data_t::handle_execute(readline_loop_state_t &rls) { // If the conditions are met, insert a new line at the position of the cursor. if (continue_on_next_line) { insert_char(el, L'\n'); - return; + return true; } - // See if this command is valid. - parser_test_error_bits_t command_test_result = 0; - if (conf.syntax_check_ok) { - command_test_result = reader_shell_test(parser(), el->text()); - } - if (command_test_result == 0 || command_test_result == PARSER_TEST_INCOMPLETE) { - // This command is valid, but an abbreviation may make it invalid. If so, we - // will have to test again. - if (expand_abbreviation_as_necessary(0)) { - // Trigger syntax highlighting as we are likely about to execute this command. - this->super_highlight_me_plenty(); - if (conf.syntax_check_ok) { - command_test_result = reader_shell_test(parser(), el->text()); - } - } - } - - if (command_test_result == 0) { - // Finished command, execute it. Don't add items in silent mode (#7230). - wcstring text = command_line.text(); - if (text.empty()) { - // Here the user just hit return. Make a new prompt, don't remove ephemeral - // items. - rls.finished = true; - return; - } - - // Historical behavior is to trim trailing spaces. - // However, escaped spaces ('\ ') should not be trimmed (#7661) - // This can be done by counting pre-trailing '\' - // If there's an odd number, this must be an escaped space. - while (!text.empty() && text.back() == L' ' && - count_preceding_backslashes(text, text.size() - 1) % 2 == 0) { - text.pop_back(); - } - - if (history && !conf.in_silent_mode) { - // Remove ephemeral items - even if the text is empty - history->remove_ephemeral_items(); - - if (!text.empty()) { - // Mark this item as ephemeral if there is a leading space (#615). - history_persistence_mode_t mode; - if (text.front() == L' ') { - // Leading spaces are ephemeral (#615). - mode = history_persistence_mode_t::ephemeral; - } else if (in_private_mode(this->vars())) { - // Private mode means in-memory only. - mode = history_persistence_mode_t::memory; - } else { - mode = history_persistence_mode_t::disk; - } - history_t::add_pending_with_file_detection(history, text, this->vars().snapshot(), - mode); - } - } - - rls.finished = true; - update_buff_pos(&command_line, command_line.size()); - } else if (command_test_result == PARSER_TEST_INCOMPLETE) { - // We are incomplete, continue editing. + // Expand the command line in preparation for execution. + // to_exec is the command to execute; the command line itself has the command for history. + wcstring to_exec; + parser_test_error_bits_t test_res = this->expand_for_execute(&to_exec); + if (test_res & PARSER_TEST_ERROR) { + return false; + } else if (test_res & PARSER_TEST_INCOMPLETE) { insert_char(el, L'\n'); - } else { - // Result must be some combination including an error. The error message will - // already be printed, all we need to do is repaint. - event_fire_generic(parser(), L"fish_posterror", {el->text()}); - screen.reset_abandoning_line(termsize_last().width); + return true; } + assert(test_res == 0); + + this->add_to_history(); + rls.cmd = std::move(to_exec); + rls.finished = true; + update_buff_pos(&command_line, command_line.size()); + return true; } maybe_t reader_data_t::readline(int nchars_or_0) { @@ -4393,6 +4504,7 @@ maybe_t reader_data_t::readline(int nchars_or_0) { if (rls.nchars <= command_line.size()) { // We've already hit the specified character limit. + rls.cmd = command_line.text(); rls.finished = true; break; } @@ -4516,8 +4628,7 @@ maybe_t reader_data_t::readline(int nchars_or_0) { } outputter_t::stdoutput().set_color(rgb_color_t::reset(), rgb_color_t::reset()); } - - return rls.finished ? maybe_t{command_line.text()} : none(); + return rls.finished ? maybe_t{std::move(rls.cmd)} : none(); } bool reader_data_t::jump(jump_direction_t dir, jump_precision_t precision, editable_line_t *el, diff --git a/src/reader.h b/src/reader.h index 9a5b94da0..0303a60ea 100644 --- a/src/reader.h +++ b/src/reader.h @@ -264,8 +264,9 @@ wcstring combine_command_and_autosuggestion(const wcstring &cmdline, /// Expand at most one abbreviation at the given cursor position. Use the parser to run any /// abbreviations which want function calls. /// \return none if no abbreviations were expanded, otherwise the resulting edit. +enum class abbrs_phase_t : uint8_t; maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - parser_t &parser); + abbrs_phase_t phase, parser_t &parser); /// Apply a completion string. Exposed for testing only. wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flags_t flags, diff --git a/tests/checks/abbr.fish b/tests/checks/abbr.fish index cfd0e0622..9245c5f5b 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -159,3 +159,17 @@ abbr --add regex_name --regex 'A[0-9]B' bar abbr --show # CHECK: abbr -a -- nonregex_name foo # CHECK: abbr -a -- regex_name --regex 'A[0-9]B' bar +abbr --erase (abbr --list) + +abbr --add bogus --position never stuff +# CHECKERR: abbr: Invalid position 'never' +# CHECKERR: Position must be one of: command, anywhere. + +abbr --add bogus --position anywhere --position command stuff +# CHECKERR: abbr: Cannot specify multiple positions + +abbr --add quiet-abbr --quiet foo1 +abbr --add loud-abbr foo2 +abbr --show +# CHECK: abbr -a -- quiet-abbr --quiet foo1 +# CHECK: abbr -a -- loud-abbr foo2 diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py index e89e62ae7..099bf86f2 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -68,17 +68,18 @@ send(r"A123Z ?") expect_str(r"") send(r"AZ ?") expect_str(r"") -send(r"QA123Z ?") +send(r"QA123Z ?") # fails as the regex must match the entire string expect_str(r"") send(r"A0000000000000000000009Z ?") expect_str(r"") # Support functions. Here anything in between @@ is uppercased, except 'nope'. sendline( - r"""function uppercaser; + r"""function uppercaser string match --quiet '*nope*' $argv[1] && return 1 string trim --chars @ $argv | string upper - end""" + end + """.strip() ) expect_prompt() sendline(r"abbr uppercaser --regex '@.+@' --function uppercaser") @@ -100,6 +101,23 @@ expect_str(r"<@nope@ >") sendline(r"abbr --erase uppercaser2") expect_prompt() +# Abbreviations which cause the command line to become incomplete or invalid +# are visibly expanded, even if they are quiet. +sendline(r"abbr openparen --position anywhere --quiet '('") +expect_prompt() +sendline(r"abbr closeparen --position anywhere --quiet ')'") +expect_prompt() +sendline(r"echo openparen") +expect_str(r"echo (") +send(r"?") +expect_str(r"") +sendline(r"echo closeparen") +expect_str(r"echo )") +send(r"?") +expect_str(r"") +sendline(r"echo openparen seq 5 closeparen") +expect_prompt(r"1 2 3 4 5") + # Verify that 'commandline' is accurate. # Abbreviation functions cannot usefully change the command line, but they can read it. sendline( @@ -107,12 +125,30 @@ sendline( set -g last_cmdline (commandline) set -g last_cursor (commandline --cursor) false - end""" + end + """.strip() ) expect_prompt() sendline(r"abbr check_cmdline --regex '@.+@' --function check_cmdline") expect_prompt() send(r"@abc@ ?") expect_str(r"<@abc@ >") -sendline(r"echo $last_cursor - $last_cmdline") -expect_prompt(r"6 - @abc@ ") +sendline(r"echo $last_cursor : $last_cmdline") +expect_prompt(r"6 : @abc@ ") + + +# Again but now we stack them. Noisy abbreviations expand first, then quiet ones. +sendline(r"""function strip_percents; string trim --chars % -- $argv; end""") +expect_prompt() +sendline(r"""function do_upper; string upper -- $argv; end""") +expect_prompt() + +sendline(r"abbr noisy1 --regex '%.+%' --function strip_percents --position anywhere") +expect_prompt() + +sendline(r"abbr quiet1 --regex 'a.+z' --function do_upper --quiet --position anywhere") +expect_prompt() +# The noisy abbr strips the % +# The quiet one sees a token starting with 'a' and ending with 'z' and uppercases it. +sendline(r"echo %abcdez%") +expect_prompt(r"ABCDEZ")