From fe7d095647756c7d0e5060965efaaa96505470eb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 16 Oct 2022 15:40:33 -0700 Subject: [PATCH 01/20] Add maybe_t::value_or This enables getting the value or returning the passed-in value. This is helpful for "default if none." --- src/fish_tests.cpp | 9 +++++++++ src/maybe.h | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index f29bca3c7..28c38bd37 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -6435,6 +6435,10 @@ void test_maybe() { do_test(std::is_copy_assignable>::value == false); do_test(std::is_copy_constructible>::value == false); + // We can construct a maybe_t from noncopyable things. + maybe_t nmt{make_unique(42)}; + do_test(**nmt == 42); + maybe_t c1{"abc"}; maybe_t c2 = c1; do_test(c1.value() == "abc"); @@ -6442,6 +6446,11 @@ void test_maybe() { c2 = c1; do_test(c1.value() == "abc"); do_test(c2.value() == "abc"); + + do_test(c2.value_or("derp") == "abc"); + do_test(c2.value_or("derp") == "abc"); + c2.reset(); + do_test(c2.value_or("derp") == "derp"); } void test_layout_cache() { diff --git a/src/maybe.h b/src/maybe.h index bfcb4c83f..e48a07972 100644 --- a/src/maybe.h +++ b/src/maybe.h @@ -209,6 +209,14 @@ class maybe_t : private maybe_detail::conditionally_copyable_t { // Transfer the value to the caller. T acquire() { return impl_.acquire(); } + // Return (a copy of) our value, or the given value if we are empty. + T value_or(T v) const { + if (this->has_value()) { + return this->value(); + } + return v; + } + // Clear the value. void reset() { impl_.reset(); } From d2daa921e9a45801ff418b0719d694e171d517db Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 29 Oct 2022 12:51:13 -0700 Subject: [PATCH 02/20] Introduce re::make_anchored This allows adjusting a pattern string so that it matches an entire string, by wrapping the regex in a group like ^(?:...)$ This is a workaround for the fact that PCRE2_ENDANCHORED is unavailable on PCRE2 prior to 2017, so we have to adjust the pattern instead. Also introduce an overload of match() which creates its own match_data_t. --- src/fish_tests.cpp | 15 +++++++++++++++ src/re.cpp | 15 +++++++++++++++ src/re.h | 8 ++++++++ 3 files changed, 38 insertions(+) diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 28c38bd37..75dabd33a 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -6867,6 +6867,21 @@ static void test_re_basic() { } do_test(join_strings(matches, L',') == L"AA,CC,11"); do_test(join_strings(captures, L',') == L"A,C,1"); + + // Test make_anchored + re = regex_t::try_compile(make_anchored(L"ab(.+?)")); + do_test(re.has_value()); + do_test(!re->match(L"")); + do_test(!re->match(L"ab")); + do_test((re->match(L"abcd") == match_range_t{0, 4})); + do_test((re->match(L"abcdefghij") == match_range_t{0, 10})); + + re = regex_t::try_compile(make_anchored(L"(a+)|(b+)")); + do_test(re.has_value()); + do_test(!re->match(L"")); + do_test(!re->match(L"aabb")); + do_test((re->match(L"aaaa") == match_range_t{0, 4})); + do_test((re->match(L"bbbb") == match_range_t{0, 4})); } static void test_re_reset() { diff --git a/src/re.cpp b/src/re.cpp index 5b1424350..9dba0e18d 100644 --- a/src/re.cpp +++ b/src/re.cpp @@ -130,6 +130,11 @@ maybe_t regex_t::match(match_data_t &md, const wcstring &subject) return match_range_t{ovector[0], ovector[1]}; } +maybe_t regex_t::match(const wcstring &subject) const { + match_data_t md = this->prepare(); + return this->match(md, subject); +} + maybe_t regex_t::group(const match_data_t &md, size_t group_idx) const { if (group_idx >= md.max_capture || group_idx >= pcre2_get_ovector_count(get_md(md.data))) { return none(); @@ -288,3 +293,13 @@ regex_t::regex_t(adapters::bytecode_ptr_t &&code) : code_(std::move(code)) { } wcstring re_error_t::message() const { return message_for_code(this->code); } + +wcstring re::make_anchored(wcstring pattern) { + // PATTERN -> ^(:?PATTERN)$. + const wchar_t *prefix = L"^(?:"; + const wchar_t *suffix = L")$"; + pattern.reserve(pattern.size() + wcslen(prefix) + wcslen(suffix)); + pattern.insert(0, prefix); + pattern.append(suffix); + return pattern; +} diff --git a/src/re.h b/src/re.h index 719cc0ff5..134b01c5e 100644 --- a/src/re.h +++ b/src/re.h @@ -111,6 +111,9 @@ class regex_t : noncopyable_t { /// \return a range on a successful match, none on no match. maybe_t match(match_data_t &md, const wcstring &subject) const; + /// A convenience function which calls prepare() for you. + maybe_t match(const wcstring &subject) const; + /// \return the matched range for an indexed or named capture group. 0 means the entire match. maybe_t group(const match_data_t &md, size_t group_idx) const; maybe_t group(const match_data_t &md, const wcstring &name) const; @@ -145,5 +148,10 @@ class regex_t : noncopyable_t { adapters::bytecode_ptr_t code_; }; +/// Adjust a pattern so that it is anchored at both beginning and end. +/// This is a workaround for the fact that PCRE2_ENDANCHORED is unavailable on pre-2017 PCRE2 +/// (e.g. 10.21, on Xenial). +wcstring make_anchored(wcstring pattern); + } // namespace re #endif From 5523eb36dbfd2b943af197a3a6e9545592b01c05 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 3 Apr 2022 12:37:29 -0700 Subject: [PATCH 03/20] Switch functions tests from abbr to vared abbr was a random function that was tested by this check, but we no longer have an abbr function so switch to a new one. --- tests/checks/functions.fish | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/checks/functions.fish b/tests/checks/functions.fish index 2734513b1..7021ea0fe 100644 --- a/tests/checks/functions.fish +++ b/tests/checks/functions.fish @@ -24,23 +24,23 @@ functions -D f2 # ========== # Verify that `functions --details` works as expected when given the name of a # function that could be autoloaded but isn't currently loaded. -set x (functions -D abbr) +set x (functions -D vared) if test (count $x) -ne 1 - or not string match -q '*/share/functions/abbr.fish' "$x" - echo "Unexpected output for 'functions -D abbr': $x" >&2 + or not string match -q '*/share/functions/vared.fish' "$x" + echo "Unexpected output for 'functions -D vared': $x" >&2 end # ========== # Verify that `functions --verbose --details` works as expected when given the name of a # function that was autoloaded. -set x (functions -v -D abbr) +set x (functions -v -D vared) if test (count $x) -ne 5 - or not string match -q '*/share/functions/abbr.fish' $x[1] + or not string match -q '*/share/functions/vared.fish' $x[1] or test $x[2] != autoloaded - or test $x[3] != 1 + or test $x[3] != 6 or test $x[4] != scope-shadowing - or test $x[5] != 'Manage abbreviations' - echo "Unexpected output for 'functions -v -D abbr': $x" >&2 + or test $x[5] != 'Edit variable value' + echo "Unexpected output for 'functions -v -D vared': $x" >&2 end # ========== From 635cc3ee8d63777b9c190bff11242deb91522fa8 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 30 Jun 2022 15:17:46 -0700 Subject: [PATCH 04/20] Add interactive tests for abbreviations --- tests/pexpects/abbrs.py | 52 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/pexpects/abbrs.py diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py new file mode 100644 index 000000000..8e215f3df --- /dev/null +++ b/tests/pexpects/abbrs.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 +from pexpect_helper import SpawnedProc + +import re + +sp = SpawnedProc() +send, sendline, sleep, expect_prompt, expect_re, expect_str = ( + sp.send, + sp.sendline, + sp.sleep, + sp.expect_prompt, + sp.expect_re, + sp.expect_str, +) +expect_prompt() + +# ? prints the command line, bracketed by <>, then clears it. +sendline(r"""bind '?' 'echo "<$(commandline)>"; commandline ""'; """) +expect_prompt() + +# Basic test. +sendline(r"abbr alpha beta") +expect_prompt() + +send(r"alpha ?") +expect_str(r"") + +# Default abbreviations expand only in command position. +send(r"echo alpha ?") +expect_str(r"") + +# Abbreviation expansions may have multiple words. +sendline(r"abbr --add emacsnw emacs -nw") +expect_prompt() +send(r"emacsnw ?") +expect_str(r"") + +# Regression test that abbreviations still expand in incomplete positions. +sendline(r"""abbr --erase (abbr --list)""") +expect_prompt() +sendline(r"""abbr --add foo echo bar""") +expect_prompt() +sendline(r"if foo") +expect_str(r"if echo bar") +sendline(r"end") +expect_prompt(r"bar") + +# Regression test that 'echo (' doesn't hang. +sendline(r"echo )") +expect_str(r"Unexpected ')' for unopened parenthesis") +send(r"?") +expect_str(r"") From 1402bae7f475b42351045e1ad91dde678f68adb2 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 1 Apr 2022 12:05:27 -0700 Subject: [PATCH 05/20] Re-implement abbreviations as a built-in Prior to this change, abbreviations were stored as fish variables, often universal. However we intend to add additional features to abbreviations which would be very awkward to shoe-horn into variables. Re-implement abbreviations using a builtin, managing them internally. Existing abbreviations stored in universal variables are still imported, for compatibility. However new abbreviations will need to be added to a function. A follow-up commit will add it. Now that abbr is a built-in, remove the abbr function; but leave the abbr.fish file so that stale files from past installs do not override the abbr builtin. --- CMakeLists.txt | 8 +- doc_src/cmds/abbr.rst | 7 +- share/functions/abbr.fish | 213 +------------------------ src/abbrs.cpp | 62 ++++++++ src/abbrs.h | 53 ++++++ src/builtin.cpp | 2 + src/builtins/abbr.cpp | 327 ++++++++++++++++++++++++++++++++++++++ src/builtins/abbr.h | 11 ++ src/complete.cpp | 6 +- src/env.cpp | 5 + src/expand.cpp | 28 ---- src/expand.h | 8 - src/fish_tests.cpp | 64 ++++---- src/highlight.cpp | 4 +- src/reader.cpp | 72 +++++---- src/reader.h | 3 +- src/wcstringutil.cpp | 14 +- src/wcstringutil.h | 3 +- tests/checks/abbr.fish | 46 ++++-- tests/pexpects/abbrs.py | 11 +- 20 files changed, 608 insertions(+), 339 deletions(-) create mode 100644 src/abbrs.cpp create mode 100644 src/abbrs.h create mode 100644 src/builtins/abbr.cpp create mode 100644 src/builtins/abbr.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 5dbfd8844..08d7c54e3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -91,7 +91,7 @@ endif() # List of sources for builtin functions. set(FISH_BUILTIN_SRCS - src/builtin.cpp src/builtins/argparse.cpp + src/builtin.cpp src/builtins/abbr.cpp src/builtins/argparse.cpp src/builtins/bg.cpp src/builtins/bind.cpp src/builtins/block.cpp src/builtins/builtin.cpp src/builtins/cd.cpp src/builtins/command.cpp src/builtins/commandline.cpp src/builtins/complete.cpp src/builtins/contains.cpp @@ -107,9 +107,9 @@ set(FISH_BUILTIN_SRCS # List of other sources. set(FISH_SRCS - src/ast.cpp src/autoload.cpp src/color.cpp src/common.cpp src/complete.cpp src/env.cpp - src/env_dispatch.cpp src/env_universal_common.cpp src/event.cpp src/exec.cpp - src/expand.cpp src/fallback.cpp src/fd_monitor.cpp src/fish_version.cpp + src/ast.cpp src/abbrs.cpp src/autoload.cpp src/color.cpp src/common.cpp src/complete.cpp + src/env.cpp src/env_dispatch.cpp src/env_universal_common.cpp src/event.cpp + src/exec.cpp src/expand.cpp src/fallback.cpp src/fd_monitor.cpp src/fish_version.cpp src/flog.cpp src/function.cpp src/future_feature_flags.cpp src/highlight.cpp src/history.cpp src/history_file.cpp src/input.cpp src/input_common.cpp src/io.cpp src/iothread.cpp src/job_group.cpp src/kill.cpp diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index 2f0682698..17c65319c 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -64,17 +64,16 @@ Examples :: - abbr -a -g gco git checkout + abbr -a gco git checkout -Add a new abbreviation where ``gco`` will be replaced with ``git checkout`` global to the current shell. +Add a new abbreviation where ``gco`` will be replaced with ``git checkout``. This abbreviation will not be automatically visible to other shells unless the same command is run in those shells (such as when executing the commands in config.fish). :: - abbr -a -U l less + abbr -a l less Add a new abbreviation where ``l`` will be replaced with ``less`` universal to all shells. -Note that you omit the **-U** since it is the default. :: diff --git a/share/functions/abbr.fish b/share/functions/abbr.fish index add6cc763..f972f4c5c 100644 --- a/share/functions/abbr.fish +++ b/share/functions/abbr.fish @@ -1,210 +1,3 @@ -function abbr --description "Manage abbreviations" - set -l options --stop-nonopt --exclusive 'a,r,e,l,s,q' --exclusive 'g,U' - set -a options h/help a/add r/rename e/erase l/list s/show q/query - set -a options g/global U/universal - - argparse -n abbr $options -- $argv - or return - - if set -q _flag_help - __fish_print_help abbr - return 0 - end - - # If run with no options, treat it like --add if we have arguments, or - # --show if we do not have any arguments. - set -l _flag_add - set -l _flag_show - if not set -q _flag_add[1] - and not set -q _flag_rename[1] - and not set -q _flag_erase[1] - and not set -q _flag_list[1] - and not set -q _flag_show[1] - and not set -q _flag_query[1] - if set -q argv[1] - set _flag_add --add - else - set _flag_show --show - end - end - - set -l abbr_scope - if set -q _flag_global - set abbr_scope --global - else if set -q _flag_universal - set abbr_scope --universal - end - - if set -q _flag_add[1] - __fish_abbr_add $argv - return - else if set -q _flag_erase[1] - set -q argv[1]; or return 1 - __fish_abbr_erase $argv - return - else if set -q _flag_rename[1] - __fish_abbr_rename $argv - return - else if set -q _flag_list[1] - __fish_abbr_list $argv - return - else if set -q _flag_show[1] - __fish_abbr_show $argv - return - else if set -q _flag_query[1] - # "--query": Check if abbrs exist. - # If we don't have an argument, it's an automatic failure. - set -q argv[1]; or return 1 - set -l escaped _fish_abbr_(string escape --style=var -- $argv) - # We return 0 if any arg exists, whereas `set -q` returns the number of undefined arguments. - # But we should be consistent with `type -q` and `command -q`. - for var in $escaped - set -q $var; and return 0 - end - return 1 - else - printf ( _ "%s: Could not figure out what to do!\n" ) abbr >&2 - return 127 - end -end - -function __fish_abbr_add --no-scope-shadowing - if not set -q argv[2] - printf ( _ "%s %s: Requires at least two arguments\n" ) abbr --add >&2 - return 1 - end - - # Because of the way abbreviations are expanded there can't be any spaces in the key. - set -l abbr_name $argv[1] - set -l escaped_abbr_name (string escape -- $abbr_name) - if string match -q "* *" -- $abbr_name - set -l msg ( _ "%s %s: Abbreviation %s cannot have spaces in the word\n" ) - printf $msg abbr --add $escaped_abbr_name >&2 - return 1 - end - - set -l abbr_val "$argv[2..-1]" - set -l abbr_var_name _fish_abbr_(string escape --style=var -- $abbr_name) - - if not set -q $abbr_var_name - # We default to the universal scope if the user didn't explicitly specify a scope and the - # abbreviation isn't already defined. - set -q abbr_scope[1] - or set abbr_scope --universal - end - true # make sure the next `set` command doesn't leak the previous status - set $abbr_scope $abbr_var_name $abbr_val -end - -function __fish_abbr_erase --no-scope-shadowing - set -l ret 0 - set -l abbr_var_names - for abbr_name in $argv - # Because of the way abbreviations are expanded there can't be any spaces in the key. - set -l escaped_name (string escape -- $abbr_name) - if string match -q "* *" -- $abbr_name - set -l msg ( _ "%s %s: Abbreviation %s cannot have spaces in the word\n" ) - printf $msg abbr --erase $escaped_name >&2 - return 1 - end - - set -l abbr_var_name _fish_abbr_(string escape --style=var -- $abbr_name) - - set -a abbr_var_names $abbr_var_name - end - # And then erase them all in one go. - # Our return value is that of `set -e`. - set -e $abbr_var_names -end - -function __fish_abbr_rename --no-scope-shadowing - if test (count $argv) -ne 2 - printf ( _ "%s %s: Requires exactly two arguments\n" ) abbr --rename >&2 - return 1 - end - - set -l old_name $argv[1] - set -l new_name $argv[2] - set -l escaped_old_name (string escape -- $old_name) - set -l escaped_new_name (string escape -- $new_name) - if string match -q "* *" -- $old_name - set -l msg ( _ "%s %s: Abbreviation %s cannot have spaces in the word\n" ) - printf $msg abbr --rename $escaped_old_name >&2 - return 1 - end - if string match -q "* *" -- $new_name - set -l msg ( _ "%s %s: Abbreviation %s cannot have spaces in the word\n" ) - printf $msg abbr --rename $escaped_new_name >&2 - return 1 - end - - set -l old_var_name _fish_abbr_(string escape --style=var -- $old_name) - set -l new_var_name _fish_abbr_(string escape --style=var -- $new_name) - - if not set -q $old_var_name - printf ( _ "%s %s: No abbreviation named %s\n" ) abbr --rename $escaped_old_name >&2 - return 1 - end - if set -q $new_var_name - set -l msg ( _ "%s %s: Abbreviation %s already exists, cannot rename %s\n" ) - printf $msg abbr --rename $escaped_new_name $escaped_old_name >&2 - return 1 - end - - set -l old_var_val $$old_var_name - - if not set -q abbr_scope[1] - # User isn't forcing the scope so use the existing scope. - if set -ql $old_var_name - set abbr_scope --global - else - set abbr_scope --universal - end - end - - set -e $old_var_name - set $abbr_scope $new_var_name $old_var_val -end - -function __fish_abbr_list --no-scope-shadowing - if set -q argv[1] - printf ( _ "%s %s: Unexpected argument -- '%s'\n" ) abbr --erase $argv[1] >&2 - return 1 - end - - for var_name in (set --names) - string match -q '_fish_abbr_*' $var_name - or continue - - set -l abbr_name (string unescape --style=var (string sub -s 12 $var_name)) - echo $abbr_name - end -end - -function __fish_abbr_show --no-scope-shadowing - if set -q argv[1] - printf ( _ "%s %s: Unexpected argument -- '%s'\n" ) abbr --erase $argv[1] >&2 - return 1 - end - - for var_name in (set --names) - string match -q '_fish_abbr_*' $var_name - or continue - - set -l abbr_var_name $var_name - set -l abbr_name (string unescape --style=var -- (string sub -s 12 $abbr_var_name)) - set -l abbr_name (string escape --style=script -- $abbr_name) - set -l abbr_val $$abbr_var_name - set -l abbr_val (string escape --style=script -- $abbr_val) - - if set -ql $abbr_var_name - printf 'abbr -a %s -- %s %s\n' -l $abbr_name $abbr_val - end - if set -qg $abbr_var_name - printf 'abbr -a %s -- %s %s\n' -g $abbr_name $abbr_val - end - if set -qU $abbr_var_name - printf 'abbr -a %s -- %s %s\n' -U $abbr_name $abbr_val - end - end -end +# This file intentionally left blank. +# This is provided to overwrite existing abbr.fish files, so that any abbr +# function retained from past fish releases does not override the abbr builtin. diff --git a/src/abbrs.cpp b/src/abbrs.cpp new file mode 100644 index 000000000..435f0830a --- /dev/null +++ b/src/abbrs.cpp @@ -0,0 +1,62 @@ +#include "config.h" // IWYU pragma: keep + +#include "abbrs.h" + +#include "env.h" +#include "global_safety.h" +#include "wcstringutil.h" + +static relaxed_atomic_t k_abbrs_next_order{0}; + +abbreviation_t::abbreviation_t(wcstring replacement, abbrs_position_t position, bool from_universal) + : replacement(std::move(replacement)), + position(position), + from_universal(from_universal), + order(++k_abbrs_next_order) {} + +acquired_lock abbrs_get_map() { + static owning_lock> abbrs; + return abbrs.acquire(); +} + +maybe_t abbrs_expand(const wcstring &token, abbrs_position_t position) { + auto abbrs = abbrs_get_map(); + auto iter = abbrs->find(token); + maybe_t result{}; + if (iter != abbrs->end()) { + const abbreviation_t &abbr = iter->second; + // Expand only if the positions are "compatible." + if (abbr.position == position || abbr.position == abbrs_position_t::anywhere) { + result = abbr.replacement; + } + } + return result; +} + +wcstring_list_t abbrs_get_keys() { + auto abbrs = abbrs_get_map(); + wcstring_list_t keys; + keys.reserve(abbrs->size()); + for (const auto &kv : *abbrs) { + keys.push_back(kv.first); + } + return keys; +} + +void abbrs_import_from_uvars(const std::unordered_map &uvars) { + auto abbrs = abbrs_get_map(); + const wchar_t *const prefix = L"_fish_abbr_"; + size_t prefix_len = wcslen(prefix); + wcstring name; + const bool from_universal = true; + for (const auto &kv : uvars) { + if (string_prefixes_string(prefix, kv.first)) { + wcstring escaped_name = kv.first.substr(prefix_len); + if (unescape_string(escaped_name, &name, unescape_flags_t{}, STRING_STYLE_VAR)) { + wcstring replacement = join_strings(kv.second.as_list(), L' '); + abbrs->emplace(name, abbreviation_t{std::move(replacement), + abbrs_position_t::command, from_universal}); + } + } + } +} diff --git a/src/abbrs.h b/src/abbrs.h new file mode 100644 index 000000000..4ab06f557 --- /dev/null +++ b/src/abbrs.h @@ -0,0 +1,53 @@ +// Support for abbreviations. +// +#ifndef FISH_ABBRS_H +#define FISH_ABBRS_H + +#include + +#include "common.h" +#include "maybe.h" + +/// Controls where in the command line abbreviations may expand. +enum class abbrs_position_t : uint8_t { + command, // expand in command position + anywhere, // expand in any token +}; + +struct abbreviation_t { + // Replacement string. + wcstring replacement{}; + + // Expansion position. + abbrs_position_t position{abbrs_position_t::command}; + + // Mark if we came from a universal variable. + bool from_universal{}; + + // A monotone key to allow reconstructing definition order. + uint64_t order{}; + + explicit abbreviation_t(wcstring replacement, + abbrs_position_t position = abbrs_position_t::command, + bool from_universal = false); + + abbreviation_t() = default; +}; + +using abbrs_map_t = std::unordered_map; + +/// \return the mutable map of abbreviations, keyed by name. +acquired_lock abbrs_get_map(); + +/// \return the replacement value for a abbreviation token, if any. +/// The \p position is given to describe where the token was found. +maybe_t abbrs_expand(const wcstring &token, abbrs_position_t position); + +/// \return the list of abbreviation keys. +wcstring_list_t abbrs_get_keys(); + +/// Import any abbreviations from universal variables. +class env_var_t; +void abbrs_import_from_uvars(const std::unordered_map &uvars); + +#endif diff --git a/src/builtin.cpp b/src/builtin.cpp index e9b8c61a1..8b8a0cba8 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -29,6 +29,7 @@ #include #include +#include "builtins/abbr.h" #include "builtins/argparse.h" #include "builtins/bg.h" #include "builtins/bind.h" @@ -354,6 +355,7 @@ static constexpr builtin_data_t builtin_datas[] = { {L":", &builtin_true, N_(L"Return a successful result")}, {L"[", &builtin_test, N_(L"Test a condition")}, {L"_", &builtin_gettext, N_(L"Translate a string")}, + {L"abbr", &builtin_abbr, N_(L"Manage generics")}, {L"and", &builtin_generic, N_(L"Run command if last command succeeded")}, {L"argparse", &builtin_argparse, N_(L"Parse options in fish script")}, {L"begin", &builtin_generic, N_(L"Create a block of code")}, diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp new file mode 100644 index 000000000..cf0420f37 --- /dev/null +++ b/src/builtins/abbr.cpp @@ -0,0 +1,327 @@ +// Implementation of the read builtin. +#include "config.h" // IWYU pragma: keep + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../abbrs.h" +#include "../builtin.h" +#include "../common.h" +#include "../env.h" +#include "../io.h" +#include "../wcstringutil.h" +#include "../wgetopt.h" +#include "../wutil.h" + +namespace { + +static const wchar_t *const CMD = L"abbr"; + +struct abbr_options_t { + bool add{}; + bool rename{}; + bool show{}; + bool list{}; + bool erase{}; + bool query{}; + maybe_t position{}; + + wcstring_list_t args; + + bool validate(io_streams_t &streams) { + // Duplicate options? + wcstring_list_t cmds; + if (add) cmds.push_back(L"add"); + if (rename) cmds.push_back(L"rename"); + if (show) cmds.push_back(L"show"); + if (list) cmds.push_back(L"list"); + if (erase) cmds.push_back(L"erase"); + if (query) cmds.push_back(L"query"); + if (cmds.size() > 1) { + streams.err.append_format(_(L"%ls: Cannot combine options %ls\n"), CMD, + join_strings(cmds, L", ").c_str()); + return false; + } + + // If run with no options, treat it like --add if we have arguments, + // or --show if we do not have any arguments. + if (cmds.empty()) { + show = args.empty(); + add = !args.empty(); + } + + if (!add && position.has_value()) { + streams.err.append_format(_(L"%ls: --position option requires --add\n"), CMD); + return false; + } + return true; + } +}; + +// Print abbreviations in a fish-script friendly way. +static int abbr_show(const abbr_options_t &, io_streams_t &streams) { + const auto abbrs = abbrs_get_map(); + for (const auto &kv : *abbrs) { + wcstring name = escape_string(kv.first); + const auto &abbr = kv.second; + wcstring value = escape_string(abbr.replacement); + const wchar_t *scope = (abbr.from_universal ? L"-U " : L""); + streams.out.append_format(L"abbr -a %ls-- %ls %ls\n", scope, name.c_str(), value.c_str()); + } + return STATUS_CMD_OK; +} + +// Print the list of abbreviation names. +static int abbr_list(const abbr_options_t &opts, io_streams_t &streams) { + const wchar_t *const subcmd = L"--list"; + if (opts.args.size() > 0) { + streams.err.append_format(_(L"%ls %ls: Unexpected argument -- '%ls'\n"), CMD, subcmd, + opts.args.front().c_str()); + return STATUS_INVALID_ARGS; + } + const auto abbrs = abbrs_get_map(); + for (const auto &kv : *abbrs) { + wcstring name = escape_string(kv.first); + name.push_back(L'\n'); + streams.out.append(name); + } + return STATUS_CMD_OK; +} + +// Rename an abbreviation, deleting any existing one with the given name. +static int abbr_rename(const abbr_options_t &opts, io_streams_t &streams) { + const wchar_t *const subcmd = L"--rename"; + if (opts.args.size() != 2) { + streams.err.append_format(_(L"%ls %ls: Requires exactly two arguments\n"), CMD, subcmd); + return STATUS_INVALID_ARGS; + } + const wcstring &old_name = opts.args[0]; + const wcstring &new_name = opts.args[1]; + if (old_name.empty() || new_name.empty()) { + streams.err.append_format(_(L"%ls %ls: Name cannot be empty\n"), CMD, subcmd); + return STATUS_INVALID_ARGS; + } + + if (std::any_of(new_name.begin(), new_name.end(), iswspace)) { + streams.err.append_format( + _(L"%ls %ls: Abbreviation '%ls' cannot have spaces in the word\n"), CMD, subcmd, + new_name.c_str()); + return STATUS_INVALID_ARGS; + } + auto abbrs = abbrs_get_map(); + + auto old_iter = abbrs->find(old_name); + if (old_iter == abbrs->end()) { + streams.err.append_format(_(L"%ls %ls: No abbreviation named %ls\n"), CMD, subcmd, + old_name.c_str()); + return STATUS_CMD_ERROR; + } + + // Cannot overwrite an abbreviation. + auto new_iter = abbrs->find(new_name); + if (new_iter != abbrs->end()) { + streams.err.append_format( + _(L"%ls %ls: Abbreviation %ls already exists, cannot rename %ls\n"), CMD, subcmd, + new_name.c_str(), old_name.c_str()); + return STATUS_INVALID_ARGS; + } + + abbreviation_t abbr = std::move(old_iter->second); + abbrs->erase(old_iter); + bool inserted = abbrs->insert(std::make_pair(std::move(new_name), std::move(abbr))).second; + assert(inserted && "Should have successfully inserted"); + (void)inserted; + return STATUS_CMD_OK; +} + +// Test if any args is an abbreviation. +static int abbr_query(const abbr_options_t &opts, io_streams_t &) { + // Return success if any of our args matches an abbreviation. + const auto abbrs = abbrs_get_map(); + for (const auto &arg : opts.args) { + if (abbrs->find(arg) != abbrs->end()) { + return STATUS_CMD_OK; + } + } + return STATUS_CMD_ERROR; +} + +// Add a named abbreviation. +static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { + const wchar_t *const subcmd = L"--add"; + if (opts.args.size() < 2) { + streams.err.append_format(_(L"%ls %ls: Requires at least two arguments\n"), CMD, subcmd); + return STATUS_INVALID_ARGS; + } + const wcstring &name = opts.args[0]; + if (name.empty()) { + streams.err.append_format(_(L"%ls %ls: Name cannot be empty\n"), CMD, subcmd); + return STATUS_INVALID_ARGS; + } + if (std::any_of(name.begin(), name.end(), iswspace)) { + streams.err.append_format( + _(L"%ls %ls: Abbreviation '%ls' cannot have spaces in the word\n"), CMD, subcmd, + name.c_str()); + return STATUS_INVALID_ARGS; + } + abbreviation_t abbr{L""}; + for (auto iter = opts.args.begin() + 1; iter != opts.args.end(); ++iter) { + if (!abbr.replacement.empty()) abbr.replacement.push_back(L' '); + abbr.replacement.append(*iter); + } + if (opts.position) { + abbr.position = *opts.position; + } + + // Note historically we have allowed overwriting existing abbreviations. + auto abbrs = abbrs_get_map(); + (*abbrs)[name] = std::move(abbr); + return STATUS_CMD_OK; +} + +// Erase the named abbreviations. +static int abbr_erase(const abbr_options_t &opts, io_streams_t &) { + if (opts.args.empty()) { + // This has historically been a silent failure. + return STATUS_CMD_ERROR; + } + + // Erase each. If any is not found, return ENV_NOT_FOUND which is historical. + int result = STATUS_CMD_OK; + auto abbrs = abbrs_get_map(); + for (const auto &arg : opts.args) { + auto iter = abbrs->find(arg); + if (iter == abbrs->end()) { + result = ENV_NOT_FOUND; + } else { + abbrs->erase(iter); + } + } + return result; +} + +} // namespace + +maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t **argv) { + 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 }; + + // 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"-arseqgUh"; + static const struct woption long_options[] = {{L"add", no_argument, 'a'}, + {L"position", required_argument, 'p'}, + {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; + wgetopter_t w; + bool unrecognized_options_are_args = false; + while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) { + switch (opt) { + case NON_OPTION_ARGUMENT: + // Non-option argument. + // If --add is specified (or implied by specifying no other commands), all + // unrecognized options after the *second* non-option argument are considered part + // of the abbreviation expansion itself, rather than options to the abbr command. + // For example, `abbr e emacs -nw` works, because `-nw` occurs after the second + // non-option, and --add is implied. + opts.args.push_back(w.woptarg); + if (opts.args.size() >= 2 && + !(opts.rename || opts.show || opts.list || opts.erase || opts.query)) { + unrecognized_options_are_args = true; + } + break; + case 'a': + opts.add = true; + break; + case 'p': { + if (opts.position.has_value()) { + streams.err.append_format(_(L"%ls: Cannot specify multiple positions\n"), CMD); + return STATUS_INVALID_ARGS; + } + if (!wcscmp(w.woptarg, L"command")) { + opts.position = abbrs_position_t::command; + } else if (!wcscmp(w.woptarg, L"anywhere")) { + opts.position = abbrs_position_t::anywhere; + } else { + streams.err.append_format(_(L"%ls: Invalid position '%ls'\nPosition must be " + L"one of: command, anywhere.\n"), + CMD, w.woptarg); + return STATUS_INVALID_ARGS; + } + break; + } + case 'r': + opts.rename = true; + break; + case 'e': + opts.erase = true; + break; + case 'q': + opts.query = true; + break; + case 's': + opts.show = true; + break; + case 'l': + opts.list = true; + break; + case 'g': + case 'U': + // Kept for backwards compatibility but ignored. + break; + case 'h': { + builtin_print_help(parser, streams, cmd); + return STATUS_CMD_OK; + } + case '?': { + if (unrecognized_options_are_args) { + opts.args.push_back(argv[w.woptind - 1]); + } else { + builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]); + return STATUS_INVALID_ARGS; + } + } + } + } + opts.args.insert(opts.args.end(), argv + w.woptind, argv + argc); + if (!opts.validate(streams)) { + return STATUS_INVALID_ARGS; + } + + if (opts.add) return abbr_add(opts, streams); + if (opts.show) return abbr_show(opts, streams); + if (opts.list) return abbr_list(opts, streams); + if (opts.rename) return abbr_rename(opts, streams); + if (opts.erase) return abbr_erase(opts, streams); + if (opts.query) return abbr_query(opts, streams); + + // validate() should error or ensure at least one path is set. + DIE("unreachable"); + return STATUS_INVALID_ARGS; +} \ No newline at end of file diff --git a/src/builtins/abbr.h b/src/builtins/abbr.h new file mode 100644 index 000000000..4a1dc883b --- /dev/null +++ b/src/builtins/abbr.h @@ -0,0 +1,11 @@ +// Prototypes for executing builtin_abbr function. +#ifndef FISH_BUILTIN_ABBR_H +#define FISH_BUILTIN_ABBR_H + +#include "../maybe.h" + +class parser_t; +struct io_streams_t; + +maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t **argv); +#endif diff --git a/src/complete.cpp b/src/complete.cpp index 5cfab9712..c1ef81b2e 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -24,6 +24,7 @@ #include #include +#include "abbrs.h" #include "autoload.h" #include "builtin.h" #include "common.h" @@ -668,7 +669,8 @@ void completer_t::complete_cmd(const wcstring &str_cmd) { } void completer_t::complete_abbr(const wcstring &cmd) { - std::map abbrs = get_abbreviations(ctx.vars); + // Copy the map, so we don't hold the lock across the call to complete_strings. + abbrs_map_t abbrs = *abbrs_get_map(); completion_list_t possible_comp; possible_comp.reserve(abbrs.size()); for (const auto &kv : abbrs) { @@ -678,7 +680,7 @@ void completer_t::complete_abbr(const wcstring &cmd) { auto desc_func = [&](const wcstring &key) { auto iter = abbrs.find(key); assert(iter != abbrs.end() && "Abbreviation not found"); - return format_string(ABBR_DESC, iter->second.c_str()); + return format_string(ABBR_DESC, iter->second.replacement.c_str()); }; this->complete_strings(cmd, desc_func, possible_comp, COMPLETE_NO_SPACE); } diff --git a/src/env.cpp b/src/env.cpp index 282935dbb..2da05949d 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -18,6 +18,7 @@ #include #include +#include "abbrs.h" #include "common.h" #include "env_dispatch.h" #include "env_universal_common.h" @@ -452,6 +453,10 @@ void env_init(const struct config_paths_t *paths, bool do_uvars, bool default_pa vars.globals().remove(name, ENV_GLOBAL | ENV_EXPORT); } } + + // Import any abbreviations from uvars. + // Note we do not dynamically react to changes. + abbrs_import_from_uvars(table); } } diff --git a/src/expand.cpp b/src/expand.cpp index 02ceda44e..e04077d2b 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -1326,31 +1326,3 @@ bool fish_xdm_login_hack_hack_hack_hack(std::vector *cmds, int argc } return result; } - -maybe_t expand_abbreviation(const wcstring &src, const environment_t &vars) { - if (src.empty()) return none(); - - wcstring esc_src = escape_string(src, 0, STRING_STYLE_VAR); - if (esc_src.empty()) { - return none(); - } - wcstring var_name = L"_fish_abbr_" + esc_src; - if (auto var_value = vars.get(var_name)) { - return var_value->as_string(); - } - return none(); -} - -std::map get_abbreviations(const environment_t &vars) { - const wcstring prefix = L"_fish_abbr_"; - auto names = vars.get_names(0); - std::map result; - for (const wcstring &name : names) { - if (string_prefixes_string(prefix, name)) { - wcstring key; - unescape_string(name.substr(prefix.size()), &key, UNESCAPE_DEFAULT, STRING_STYLE_VAR); - result[key] = vars.get(name)->as_string(); - } - } - return result; -} diff --git a/src/expand.h b/src/expand.h index 5649abcb4..22f7a37b9 100644 --- a/src/expand.h +++ b/src/expand.h @@ -203,14 +203,6 @@ void expand_tilde(wcstring &input, const environment_t &vars); /// Perform the opposite of tilde expansion on the string, which is modified in place. wcstring replace_home_directory_with_tilde(const wcstring &str, const environment_t &vars); -/// Abbreviation support. Expand src as an abbreviation, returning the expanded form if found, -/// none() if not. -maybe_t expand_abbreviation(const wcstring &src, const environment_t &vars); - -/// \return a snapshot of all abbreviations as a map abbreviation->expansion. -/// The abbreviations are unescaped, i.e. they may not be valid variable identifiers (#6166). -std::map get_abbreviations(const environment_t &vars); - // Terrible hacks bool fish_xdm_login_hack_hack_hack_hack(std::vector *cmds, int argc, const char *const *argv); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 75dabd33a..07f79271b 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -46,6 +46,7 @@ #include #endif +#include "abbrs.h" #include "ast.h" #include "autoload.h" #include "builtin.h" @@ -2466,34 +2467,31 @@ static void test_ifind_fuzzy() { static void test_abbreviations() { say(L"Testing abbreviations"); - auto &vars = parser_t::principal_parser().vars(); - vars.push(true); - const std::vector> abbreviations = { - {L"gc", L"git checkout"}, - {L"foo", L"bar"}, - {L"gx", L"git checkout"}, - }; - for (const auto &kv : abbreviations) { - int ret = vars.set_one(L"_fish_abbr_" + kv.first, ENV_LOCAL, kv.second); - if (ret != 0) err(L"Unable to set abbreviation variable"); + { + auto abbrs = abbrs_get_map(); + abbrs->emplace(L"gc", L"git checkout"); + abbrs->emplace(L"foo", L"bar"); + abbrs->emplace(L"gx", L"git checkout"); + abbrs->emplace(L"yin", abbreviation_t(L"yang", abbrs_position_t::anywhere)); } - if (expand_abbreviation(L"", vars)) err(L"Unexpected success with empty abbreviation"); - if (expand_abbreviation(L"nothing", vars)) err(L"Unexpected success with missing abbreviation"); + auto cmd = abbrs_position_t::command; + if (abbrs_expand(L"", cmd)) err(L"Unexpected success with empty abbreviation"); + if (abbrs_expand(L"nothing", cmd)) err(L"Unexpected success with missing abbreviation"); - auto mresult = expand_abbreviation(L"gc", vars); + auto mresult = abbrs_expand(L"gc", cmd); if (!mresult) err(L"Unexpected failure with gc abbreviation"); if (*mresult != L"git checkout") err(L"Wrong abbreviation result for gc"); - mresult = expand_abbreviation(L"foo", vars); + mresult = abbrs_expand(L"foo", cmd); if (!mresult) err(L"Unexpected failure with foo abbreviation"); if (*mresult != L"bar") err(L"Wrong abbreviation result for foo"); maybe_t result; - auto expand_abbreviation_in_command = [](const wcstring &cmdline, size_t cursor_pos, - const environment_t &vars) -> maybe_t { - if (auto edit = reader_expand_abbreviation_in_command(cmdline, cursor_pos, vars)) { + 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)) { wcstring cmdline_expanded = cmdline; std::vector colors{cmdline_expanded.size()}; apply_edit(&cmdline_expanded, &colors, *edit); @@ -2501,49 +2499,55 @@ static void test_abbreviations() { } return none_t(); }; - result = expand_abbreviation_in_command(L"just a command", 3, vars); + result = expand_abbreviation_in_command(L"just a command", 3); if (result) err(L"Command wrongly expanded on line %ld", (long)__LINE__); - result = expand_abbreviation_in_command(L"gc somebranch", 0, vars); + result = expand_abbreviation_in_command(L"gc somebranch", 0); if (!result) err(L"Command not expanded on line %ld", (long)__LINE__); - result = expand_abbreviation_in_command(L"gc somebranch", const_strlen(L"gc"), vars); + result = expand_abbreviation_in_command(L"gc somebranch", const_strlen(L"gc")); if (!result) err(L"gc not expanded"); if (result != L"git checkout somebranch") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); // Space separation. - result = expand_abbreviation_in_command(L"gx somebranch", const_strlen(L"gc"), vars); + result = expand_abbreviation_in_command(L"gx somebranch", const_strlen(L"gc")); if (!result) err(L"gx not expanded"); if (result != L"git checkout somebranch") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); - result = expand_abbreviation_in_command(L"echo hi ; gc somebranch", - const_strlen(L"echo hi ; g"), vars); + result = + expand_abbreviation_in_command(L"echo hi ; gc somebranch", const_strlen(L"echo hi ; g")); if (!result) err(L"gc not expanded on line %ld", (long)__LINE__); if (result != L"echo hi ; git checkout somebranch") err(L"gc incorrectly expanded on line %ld", (long)__LINE__); result = expand_abbreviation_in_command(L"echo (echo (echo (echo (gc ", - const_strlen(L"echo (echo (echo (echo (gc"), vars); + const_strlen(L"echo (echo (echo (echo (gc")); if (!result) err(L"gc not expanded on line %ld", (long)__LINE__); if (result != L"echo (echo (echo (echo (git checkout ") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); // If commands should be expanded. - result = expand_abbreviation_in_command(L"if gc", const_strlen(L"if gc"), vars); + result = expand_abbreviation_in_command(L"if gc", const_strlen(L"if gc")); if (!result) err(L"gc not expanded on line %ld", (long)__LINE__); if (result != L"if git checkout") err(L"gc incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); // Others should not be. - result = expand_abbreviation_in_command(L"of gc", const_strlen(L"of gc"), vars); + result = expand_abbreviation_in_command(L"of gc", const_strlen(L"of gc")); if (result) err(L"gc incorrectly expanded on line %ld", (long)__LINE__); // Others should not be. - result = expand_abbreviation_in_command(L"command gc", const_strlen(L"command gc"), vars); + result = expand_abbreviation_in_command(L"command gc", const_strlen(L"command gc")); if (result) err(L"gc incorrectly expanded on line %ld", (long)__LINE__); - vars.pop(); + // yin/yang expands everywhere. + result = expand_abbreviation_in_command(L"command yin", const_strlen(L"command yin")); + if (!result) err(L"gc not expanded on line %ld", (long)__LINE__); + if (result != L"command yang") { + err(L"command yin incorrectly expanded on line %ld to '%ls'", (long)__LINE__, + result->c_str()); + } } /// Test path functions. @@ -3502,11 +3506,9 @@ static void test_complete() { completions.clear(); // Test abbreviations. - auto &pvars = parser_t::principal_parser().vars(); function_add(L"testabbrsonetwothreefour", func_props); - int ret = pvars.set_one(L"_fish_abbr_testabbrsonetwothreezero", ENV_LOCAL, L"expansion"); + abbrs_get_map()->emplace(L"testabbrsonetwothreezero", L"expansion"); completions = complete(L"testabbrsonetwothree", {}, parser->context()); - do_test(ret == 0); do_test(completions.size() == 2); do_test(completions.at(0).completion == L"four"); do_test((completions.at(0).flags & COMPLETE_NO_SPACE) == 0); diff --git a/src/highlight.cpp b/src/highlight.cpp index 7645573b3..680cd9466 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -16,6 +16,7 @@ #include #include +#include "abbrs.h" #include "ast.h" #include "builtin.h" #include "color.h" @@ -1334,7 +1335,8 @@ static bool command_is_valid(const wcstring &cmd, enum statement_decoration_t de if (!is_valid && function_ok) is_valid = function_exists_no_autoload(cmd); // Abbreviations - if (!is_valid && abbreviation_ok) is_valid = expand_abbreviation(cmd, vars).has_value(); + if (!is_valid && abbreviation_ok) + is_valid = abbrs_expand(cmd, abbrs_position_t::command).has_value(); // 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 691850c97..1a3b8ee09 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -44,6 +44,7 @@ #include #include +#include "abbrs.h" #include "ast.h" #include "color.h" #include "common.h" @@ -1347,10 +1348,8 @@ void reader_data_t::pager_selection_changed() { } /// Expand abbreviations at the given cursor position. Does NOT inspect 'data'. -maybe_t reader_expand_abbreviation_in_command(const wcstring &cmdline, size_t cursor_pos, - const environment_t &vars) { - // See if we are at "command position". Get the surrounding command substitution, and get the - // extent of the first token. +maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos) { + // 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); @@ -1369,40 +1368,45 @@ maybe_t reader_expand_abbreviation_in_command(const wcstring &cmdline, s ast_t::parse(subcmd, parse_flag_continue_after_error | parse_flag_accept_incomplete_tokens | parse_flag_leave_unterminated); - // Look for plain statements where the cursor is at the end of the command. - const ast::string_t *matching_cmd_node = nullptr; - for (const node_t &n : ast) { - const auto *stmt = n.try_as(); - if (!stmt) continue; - - // Skip if we have a decoration. - if (stmt->opt_decoration) continue; - - // See if the command's source range range contains our cursor, including at the end. - auto msource = stmt->command.try_source_range(); - if (!msource) continue; - - // Now see if its source range contains our cursor, including at the end. - if (subcmd_cursor_pos >= msource->start && - subcmd_cursor_pos <= msource->start + msource->length) { - // Success! - matching_cmd_node = &stmt->command; - break; + // 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) { + return none(); + } - // Now if we found a command node, expand it. - maybe_t result{}; - if (matching_cmd_node) { - assert(!matching_cmd_node->unsourced && "Should not be unsourced"); - const wcstring token = matching_cmd_node->source(subcmd); - if (auto abbreviation = expand_abbreviation(token, vars)) { - // There was an abbreviation! Replace the token in the full command. Maintain the - // relative position of the cursor. - source_range_t r = matching_cmd_node->source_range(); - result = edit_t(subcmd_offset + r.start, r.length, std::move(*abbreviation)); + // 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. + maybe_t result{}; + if (auto abbreviation = abbrs_expand(leaf->source(subcmd), expand_position)) { + // There was an abbreviation! Replace the token in the full command. Maintain the + // relative position of the cursor. + source_range_t r = leaf->source_range(); + result = edit_t(subcmd_offset + r.start, r.length, abbreviation.acquire()); + } return result; } @@ -1417,7 +1421,7 @@ bool reader_data_t::expand_abbreviation_as_necessary(size_t cursor_backtrack) { // Try expanding abbreviations. size_t cursor_pos = el->position() - std::min(el->position(), cursor_backtrack); - if (auto edit = reader_expand_abbreviation_in_command(el->text(), cursor_pos, vars())) { + if (auto edit = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos)) { push_edit(el, std::move(*edit)); update_buff_pos(el); result = true; diff --git a/src/reader.h b/src/reader.h index 7df5f1d27..284fb91a1 100644 --- a/src/reader.h +++ b/src/reader.h @@ -263,8 +263,7 @@ wcstring combine_command_and_autosuggestion(const wcstring &cmdline, /// Expand abbreviations at the given cursor position. Exposed for testing purposes only. /// \return none if no abbreviations were expanded, otherwise the new command line. -maybe_t reader_expand_abbreviation_in_command(const wcstring &cmdline, size_t cursor_pos, - const environment_t &vars); +maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos); /// 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/src/wcstringutil.cpp b/src/wcstringutil.cpp index 2fa6412ff..d4a6810ee 100644 --- a/src/wcstringutil.cpp +++ b/src/wcstringutil.cpp @@ -286,12 +286,12 @@ wcstring_list_t split_string_tok(const wcstring &val, const wcstring &seps, size return out; } -wcstring join_strings(const wcstring_list_t &vals, wchar_t sep) { +static wcstring join_strings_impl(const wcstring_list_t &vals, const wchar_t *sep, size_t seplen) { if (vals.empty()) return wcstring{}; // Reserve the size we will need. // count-1 separators, plus the length of all strings. - size_t size = vals.size() - 1; + size_t size = (vals.size() - 1) * seplen; for (const wcstring &s : vals) { size += s.size(); } @@ -302,7 +302,7 @@ wcstring join_strings(const wcstring_list_t &vals, wchar_t sep) { bool first = true; for (const wcstring &s : vals) { if (!first) { - result.push_back(sep); + result.append(sep, seplen); } result.append(s); first = false; @@ -310,6 +310,14 @@ wcstring join_strings(const wcstring_list_t &vals, wchar_t sep) { return result; } +wcstring join_strings(const wcstring_list_t &vals, wchar_t c) { + return join_strings_impl(vals, &c, 1); +} + +wcstring join_strings(const wcstring_list_t &vals, const wchar_t *sep) { + return join_strings_impl(vals, sep, wcslen(sep)); +} + void wcs2string_bad_char(wchar_t wc) { FLOGF(char_encoding, L"Wide character U+%4X has no narrow representation", wc); } diff --git a/src/wcstringutil.h b/src/wcstringutil.h index 4a768c0bd..566b3f931 100644 --- a/src/wcstringutil.h +++ b/src/wcstringutil.h @@ -137,8 +137,9 @@ wcstring_list_t split_string(const wcstring &val, wchar_t sep); wcstring_list_t split_string_tok(const wcstring &val, const wcstring &seps, size_t max_results = std::numeric_limits::max()); -/// Join a list of strings by a separator character. +/// Join a list of strings by a separator character or string. wcstring join_strings(const wcstring_list_t &vals, wchar_t sep); +wcstring join_strings(const wcstring_list_t &vals, const wchar_t *sep); inline wcstring to_string(long x) { wchar_t buff[64]; diff --git a/tests/checks/abbr.fish b/tests/checks/abbr.fish index edb25a8d6..9eb0d02e8 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -1,29 +1,36 @@ #RUN: %fish %s + +# Universal abbreviations are imported. +set -U _fish_abbr_cuckoo somevalue +set fish (status fish-path) +$fish -c abbr +# CHECK: abbr -a -U -- cuckoo somevalue + # Test basic add and list of __abbr1 abbr __abbr1 alpha beta gamma abbr | grep __abbr1 -# CHECK: abbr -a -U -- __abbr1 'alpha beta gamma' +# CHECK: abbr -a -- __abbr1 'alpha beta gamma' # Erasing one that doesn\'t exist should do nothing abbr --erase NOT_AN_ABBR abbr | grep __abbr1 -# CHECK: abbr -a -U -- __abbr1 'alpha beta gamma' +# CHECK: abbr -a -- __abbr1 'alpha beta gamma' # Adding existing __abbr1 should be idempotent abbr __abbr1 alpha beta gamma abbr | grep __abbr1 -# CHECK: abbr -a -U -- __abbr1 'alpha beta gamma' +# CHECK: abbr -a -- __abbr1 'alpha beta gamma' # Replacing __abbr1 definition abbr __abbr1 delta abbr | grep __abbr1 -# CHECK: abbr -a -U -- __abbr1 delta +# CHECK: abbr -a -- __abbr1 delta # __abbr1 -s and --show tests abbr -s | grep __abbr1 abbr --show | grep __abbr1 -# CHECK: abbr -a -U -- __abbr1 delta -# CHECK: abbr -a -U -- __abbr1 delta +# CHECK: abbr -a -- __abbr1 delta +# CHECK: abbr -a -- __abbr1 delta # Test erasing __abbr1 abbr -e __abbr1 @@ -32,13 +39,13 @@ abbr | grep __abbr1 # Ensure we escape special characters on output abbr '~__abbr2' '$xyz' abbr | grep __abbr2 -# CHECK: abbr -a -U -- '~__abbr2' '$xyz' +# CHECK: abbr -a -- '~__abbr2' '$xyz' abbr -e '~__abbr2' # Ensure we handle leading dashes in abbreviation names properly abbr -- --__abbr3 xyz abbr | grep __abbr3 -# CHECK: abbr -a -U -- --__abbr3 xyz +# CHECK: abbr -a -- --__abbr3 xyz abbr -e -- --__abbr3 # Test that an abbr word containing spaces is rejected @@ -51,7 +58,7 @@ abbr __abbr4 omega abbr | grep __abbr5 abbr -r __abbr4 __abbr5 abbr | grep __abbr5 -# CHECK: abbr -a -U -- __abbr5 omega +# CHECK: abbr -a -- __abbr5 omega abbr -e __abbr5 abbr | grep __abbr4 @@ -77,7 +84,7 @@ abbr -r __abbr8 __abbr9 __abbr10 abbr | grep __abbr8 abbr | grep __abbr9 abbr | grep __abbr10 -# CHECK: abbr -a -U -- __abbr8 omega +# CHECK: abbr -a -- __abbr8 omega # Test renaming to existing abbreviation abbr __abbr11 omega11 @@ -106,3 +113,22 @@ echo $status abbr -q banana __abbr8 foobar echo $status # CHECK: 0 + +abbr --add grape --position nowhere juice +echo $status +# CHECKERR: abbr: Invalid position 'nowhere' +# CHECKERR: Position must be one of: command, anywhere. +# CHECK: 2 + +abbr --add grape --position anywhere juice +echo $status +# CHECK: 0 + +abbr --add grape --position command juice +echo $status +# CHECK: 0 + +abbr --query banana --position anywhere +echo $status +# CHECKERR: abbr: --position option requires --add +# CHECK: 2 diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py index 8e215f3df..41364e274 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -19,13 +19,13 @@ sendline(r"""bind '?' 'echo "<$(commandline)>"; commandline ""'; """) expect_prompt() # Basic test. +# Default abbreviations expand only in command position. sendline(r"abbr alpha beta") expect_prompt() send(r"alpha ?") expect_str(r"") -# Default abbreviations expand only in command position. send(r"echo alpha ?") expect_str(r"") @@ -50,3 +50,12 @@ sendline(r"echo )") expect_str(r"Unexpected ')' for unopened parenthesis") send(r"?") expect_str(r"") + +# Support position anywhere. +sendline(r"abbr alpha --position anywhere beta2") + +send(r"alpha ?") +expect_str(r"") + +send(r"echo alpha ?") +expect_str(r"") From 470153c0df36068292a3237948b6f04fcf387849 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 15 Jul 2022 00:03:45 -0700 Subject: [PATCH 06/20] Refactor abbreviation set into its own type Previously the abbreviation map was just an unordered map; switch it to a real class so we can hang methods off of it. --- src/abbrs.cpp | 100 +++++++++++++++++++++++++++++------------- src/abbrs.h | 61 ++++++++++++++++++++------ src/builtins/abbr.cpp | 58 +++++++++--------------- src/complete.cpp | 22 ++++++---- src/env.cpp | 2 +- src/fish_tests.cpp | 27 +++++++++--- 6 files changed, 173 insertions(+), 97 deletions(-) diff --git a/src/abbrs.cpp b/src/abbrs.cpp index 435f0830a..a91d8c07f 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -6,57 +6,97 @@ #include "global_safety.h" #include "wcstringutil.h" -static relaxed_atomic_t k_abbrs_next_order{0}; - -abbreviation_t::abbreviation_t(wcstring replacement, abbrs_position_t position, bool from_universal) - : replacement(std::move(replacement)), +abbreviation_t::abbreviation_t(wcstring name, wcstring replacement, abbrs_position_t position, + bool from_universal) + : name(std::move(name)), + replacement(std::move(replacement)), position(position), - from_universal(from_universal), - order(++k_abbrs_next_order) {} + from_universal(from_universal) {} -acquired_lock abbrs_get_map() { - static owning_lock> abbrs; +acquired_lock abbrs_get_set() { + static owning_lock abbrs; return abbrs.acquire(); } -maybe_t abbrs_expand(const wcstring &token, abbrs_position_t position) { - auto abbrs = abbrs_get_map(); - auto iter = abbrs->find(token); - maybe_t result{}; - if (iter != abbrs->end()) { - const abbreviation_t &abbr = iter->second; - // Expand only if the positions are "compatible." - if (abbr.position == position || abbr.position == abbrs_position_t::anywhere) { - result = abbr.replacement; +maybe_t abbrs_set_t::expand(const wcstring &token, abbrs_position_t position) const { + // Later abbreviations take precedence so walk backwards. + for (auto it = abbrs_.rbegin(); it != abbrs_.rend(); ++it) { + const abbreviation_t &abbr = *it; + // Expand only if the abbreviation expands anywhere or in the given position. + if (!(abbr.position == position || abbr.position == abbrs_position_t::anywhere)) { + continue; + } + + // Expand only if the name matches. + if (token != abbr.name) { + continue; + } + + return abbr.replacement; + } + return none(); +} + +void abbrs_set_t::add(abbreviation_t &&abbr) { + assert(!abbr.name.empty() && "Invalid name"); + bool inserted = used_names_.insert(abbr.name).second; + if (!inserted) { + // Name was already used, do a linear scan to find it. + auto where = std::find_if(abbrs_.begin(), abbrs_.end(), [&](const abbreviation_t &other) { + return other.name == abbr.name; + }); + assert(where != abbrs_.end() && "Abbreviation not found though its name was present"); + abbrs_.erase(where); + } + abbrs_.push_back(std::move(abbr)); +} + +void abbrs_set_t::rename(const wcstring &old_name, const wcstring &new_name) { + bool erased = this->used_names_.erase(old_name) > 0; + bool inserted = this->used_names_.insert(new_name).second; + assert(erased && inserted && "Old name not found or new name already present"); + (void)erased; + (void)inserted; + for (auto &abbr : abbrs_) { + if (abbr.name == old_name) { + abbr.name = new_name; + break; } } - return result; } -wcstring_list_t abbrs_get_keys() { - auto abbrs = abbrs_get_map(); - wcstring_list_t keys; - keys.reserve(abbrs->size()); - for (const auto &kv : *abbrs) { - keys.push_back(kv.first); +bool abbrs_set_t::erase(const wcstring &name) { + bool erased = this->used_names_.erase(name) > 0; + if (!erased) { + return false; } - return keys; + for (auto it = abbrs_.begin(); it != abbrs_.end(); ++it) { + if (it->name == name) { + abbrs_.erase(it); + return true; + } + } + assert(false && "Unable to find named abbreviation"); + return false; } -void abbrs_import_from_uvars(const std::unordered_map &uvars) { - auto abbrs = abbrs_get_map(); +void abbrs_set_t::import_from_uvars(const std::unordered_map &uvars) { const wchar_t *const prefix = L"_fish_abbr_"; size_t prefix_len = wcslen(prefix); - wcstring name; const bool from_universal = true; for (const auto &kv : uvars) { if (string_prefixes_string(prefix, kv.first)) { wcstring escaped_name = kv.first.substr(prefix_len); + wcstring name; if (unescape_string(escaped_name, &name, unescape_flags_t{}, STRING_STYLE_VAR)) { wcstring replacement = join_strings(kv.second.as_list(), L' '); - abbrs->emplace(name, abbreviation_t{std::move(replacement), - abbrs_position_t::command, from_universal}); + this->add(abbreviation_t{std::move(name), std::move(replacement), + abbrs_position_t::command, from_universal}); } } } } + +maybe_t abbrs_expand(const wcstring &token, abbrs_position_t position) { + return abbrs_get_set()->expand(token, position); +} diff --git a/src/abbrs.h b/src/abbrs.h index 4ab06f557..ef6d89873 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -4,10 +4,13 @@ #define FISH_ABBRS_H #include +#include #include "common.h" #include "maybe.h" +class env_var_t; + /// Controls where in the command line abbreviations may expand. enum class abbrs_position_t : uint8_t { command, // expand in command position @@ -15,6 +18,10 @@ enum class abbrs_position_t : uint8_t { }; 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. + wcstring name{}; + // Replacement string. wcstring replacement{}; @@ -24,30 +31,56 @@ struct abbreviation_t { // Mark if we came from a universal variable. bool from_universal{}; - // A monotone key to allow reconstructing definition order. - uint64_t order{}; + // \return true if this is a regex abbreviation. + bool is_regex() const { return false; } - explicit abbreviation_t(wcstring replacement, + explicit abbreviation_t(wcstring name, wcstring replacement, abbrs_position_t position = abbrs_position_t::command, bool from_universal = false); abbreviation_t() = default; }; -using abbrs_map_t = std::unordered_map; +class abbrs_set_t { + public: + /// \return the replacement value for a abbreviation token, if any. + /// The \p position is given to describe where the token was found. + maybe_t expand(const wcstring &token, abbrs_position_t position) const; -/// \return the mutable map of abbreviations, keyed by name. -acquired_lock abbrs_get_map(); + /// Add an abbreviation. Any abbreviation with the same name is replaced. + void add(abbreviation_t &&abbr); -/// \return the replacement value for a abbreviation token, if any. + /// Rename an abbreviation. This asserts that the old name is used, and the new name is not; the + /// caller should check these beforehand with has_name(). + void rename(const wcstring &old_name, const wcstring &new_name); + + /// Erase an abbreviation by name. + /// \return true if erased, false if not found. + bool erase(const wcstring &name); + + /// \return true if we have an abbreviation with the given name. + bool has_name(const wcstring &name) const { return used_names_.count(name) > 0; } + + /// \return a reference to the abbreviation list. + const std::vector &list() const { return abbrs_; } + + /// Import from a universal variable set. + void import_from_uvars(const std::unordered_map &uvars); + + private: + /// List of abbreviations, in definition order. + std::vector abbrs_{}; + + /// Set of used abbrevation names. + /// This is to avoid a linear scan when adding new abbreviations. + std::unordered_set used_names_; +}; + +/// \return the global mutable set of abbreviations. +acquired_lock abbrs_get_set(); + +/// \return the replacement value for a abbreviation token, if any, using the global set. /// The \p position is given to describe where the token was found. maybe_t abbrs_expand(const wcstring &token, abbrs_position_t position); -/// \return the list of abbreviation keys. -wcstring_list_t abbrs_get_keys(); - -/// Import any abbreviations from universal variables. -class env_var_t; -void abbrs_import_from_uvars(const std::unordered_map &uvars); - #endif diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index cf0420f37..99c1827c5 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -73,10 +73,9 @@ struct abbr_options_t { // Print abbreviations in a fish-script friendly way. static int abbr_show(const abbr_options_t &, io_streams_t &streams) { - const auto abbrs = abbrs_get_map(); - for (const auto &kv : *abbrs) { - wcstring name = escape_string(kv.first); - const auto &abbr = kv.second; + const auto abbrs = abbrs_get_set(); + for (const auto &abbr : abbrs->list()) { + wcstring name = escape_string(abbr.name); wcstring value = escape_string(abbr.replacement); const wchar_t *scope = (abbr.from_universal ? L"-U " : L""); streams.out.append_format(L"abbr -a %ls-- %ls %ls\n", scope, name.c_str(), value.c_str()); @@ -92,9 +91,9 @@ static int abbr_list(const abbr_options_t &opts, io_streams_t &streams) { opts.args.front().c_str()); return STATUS_INVALID_ARGS; } - const auto abbrs = abbrs_get_map(); - for (const auto &kv : *abbrs) { - wcstring name = escape_string(kv.first); + const auto abbrs = abbrs_get_set(); + for (const auto &abbr : abbrs->list()) { + wcstring name = escape_string(abbr.name); name.push_back(L'\n'); streams.out.append(name); } @@ -121,38 +120,29 @@ static int abbr_rename(const abbr_options_t &opts, io_streams_t &streams) { new_name.c_str()); return STATUS_INVALID_ARGS; } - auto abbrs = abbrs_get_map(); + auto abbrs = abbrs_get_set(); - auto old_iter = abbrs->find(old_name); - if (old_iter == abbrs->end()) { + if (!abbrs->has_name(old_name)) { streams.err.append_format(_(L"%ls %ls: No abbreviation named %ls\n"), CMD, subcmd, old_name.c_str()); return STATUS_CMD_ERROR; } - - // Cannot overwrite an abbreviation. - auto new_iter = abbrs->find(new_name); - if (new_iter != abbrs->end()) { + if (abbrs->has_name(new_name)) { streams.err.append_format( _(L"%ls %ls: Abbreviation %ls already exists, cannot rename %ls\n"), CMD, subcmd, new_name.c_str(), old_name.c_str()); return STATUS_INVALID_ARGS; } - - abbreviation_t abbr = std::move(old_iter->second); - abbrs->erase(old_iter); - bool inserted = abbrs->insert(std::make_pair(std::move(new_name), std::move(abbr))).second; - assert(inserted && "Should have successfully inserted"); - (void)inserted; + abbrs->rename(old_name, new_name); return STATUS_CMD_OK; } // Test if any args is an abbreviation. static int abbr_query(const abbr_options_t &opts, io_streams_t &) { // Return success if any of our args matches an abbreviation. - const auto abbrs = abbrs_get_map(); + const auto abbrs = abbrs_get_set(); for (const auto &arg : opts.args) { - if (abbrs->find(arg) != abbrs->end()) { + if (abbrs->has_name(arg)) { return STATUS_CMD_OK; } } @@ -177,18 +167,16 @@ static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { name.c_str()); return STATUS_INVALID_ARGS; } - abbreviation_t abbr{L""}; + wcstring replacement; for (auto iter = opts.args.begin() + 1; iter != opts.args.end(); ++iter) { - if (!abbr.replacement.empty()) abbr.replacement.push_back(L' '); - abbr.replacement.append(*iter); - } - if (opts.position) { - abbr.position = *opts.position; + if (!replacement.empty()) replacement.push_back(L' '); + replacement.append(*iter); } + abbrs_position_t position = opts.position ? *opts.position : abbrs_position_t::command; + abbreviation_t abbr{name, std::move(replacement), position}; // Note historically we have allowed overwriting existing abbreviations. - auto abbrs = abbrs_get_map(); - (*abbrs)[name] = std::move(abbr); + abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -201,13 +189,10 @@ static int abbr_erase(const abbr_options_t &opts, io_streams_t &) { // Erase each. If any is not found, return ENV_NOT_FOUND which is historical. int result = STATUS_CMD_OK; - auto abbrs = abbrs_get_map(); + auto abbrs = abbrs_get_set(); for (const auto &arg : opts.args) { - auto iter = abbrs->find(arg); - if (iter == abbrs->end()) { + if (!abbrs->erase(arg)) { result = ENV_NOT_FOUND; - } else { - abbrs->erase(iter); } } return result; @@ -219,7 +204,7 @@ 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 }; + enum { NON_OPTION_ARGUMENT = 1, REGEX_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 @@ -244,7 +229,6 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) { switch (opt) { case NON_OPTION_ARGUMENT: - // Non-option argument. // If --add is specified (or implied by specifying no other commands), all // unrecognized options after the *second* non-option argument are considered part // of the abbreviation expansion itself, rather than options to the abbr command. diff --git a/src/complete.cpp b/src/complete.cpp index c1ef81b2e..d710fba4c 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -669,18 +669,24 @@ void completer_t::complete_cmd(const wcstring &str_cmd) { } void completer_t::complete_abbr(const wcstring &cmd) { - // Copy the map, so we don't hold the lock across the call to complete_strings. - abbrs_map_t abbrs = *abbrs_get_map(); + // Copy the list of names and descriptions so as not to hold the lock across the call to + // complete_strings. completion_list_t possible_comp; - possible_comp.reserve(abbrs.size()); - for (const auto &kv : abbrs) { - possible_comp.emplace_back(kv.first); + std::unordered_map descs; + { + auto abbrs = abbrs_get_set(); + for (const auto &abbr : abbrs->list()) { + if (!abbr.is_regex()) { + possible_comp.emplace_back(abbr.name); + descs[abbr.name] = abbr.replacement; + } + } } auto desc_func = [&](const wcstring &key) { - auto iter = abbrs.find(key); - assert(iter != abbrs.end() && "Abbreviation not found"); - return format_string(ABBR_DESC, iter->second.replacement.c_str()); + auto iter = descs.find(key); + assert(iter != descs.end() && "Abbreviation not found"); + return format_string(ABBR_DESC, iter->second.c_str()); }; this->complete_strings(cmd, desc_func, possible_comp, COMPLETE_NO_SPACE); } diff --git a/src/env.cpp b/src/env.cpp index 2da05949d..92bf37117 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -456,7 +456,7 @@ void env_init(const struct config_paths_t *paths, bool do_uvars, bool default_pa // Import any abbreviations from uvars. // Note we do not dynamically react to changes. - abbrs_import_from_uvars(table); + abbrs_get_set()->import_from_uvars(table); } } diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 07f79271b..114b419c1 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2467,13 +2467,12 @@ static void test_ifind_fuzzy() { static void test_abbreviations() { say(L"Testing abbreviations"); - { - auto abbrs = abbrs_get_map(); - abbrs->emplace(L"gc", L"git checkout"); - abbrs->emplace(L"foo", L"bar"); - abbrs->emplace(L"gx", L"git checkout"); - abbrs->emplace(L"yin", abbreviation_t(L"yang", abbrs_position_t::anywhere)); + auto abbrs = abbrs_get_set(); + abbrs->add(abbreviation_t(L"gc", L"git checkout")); + abbrs->add(abbreviation_t(L"foo", L"bar")); + abbrs->add(abbreviation_t(L"gx", L"git checkout")); + abbrs->add(abbreviation_t(L"yin", L"yang", abbrs_position_t::anywhere)); } auto cmd = abbrs_position_t::command; @@ -2548,6 +2547,19 @@ static void test_abbreviations() { err(L"command yin incorrectly expanded on line %ld to '%ls'", (long)__LINE__, result->c_str()); } + + // Renaming works. + { + auto abbrs = abbrs_get_set(); + do_test(!abbrs->has_name(L"gcc")); + do_test(abbrs->has_name(L"gc")); + abbrs->rename(L"gc", L"gcc"); + do_test(abbrs->has_name(L"gcc")); + do_test(!abbrs->has_name(L"gc")); + do_test(!abbrs->erase(L"gc")); + do_test(abbrs->erase(L"gcc")); + do_test(!abbrs->erase(L"gcc")); + } } /// Test path functions. @@ -3507,7 +3519,7 @@ static void test_complete() { // Test abbreviations. function_add(L"testabbrsonetwothreefour", func_props); - abbrs_get_map()->emplace(L"testabbrsonetwothreezero", L"expansion"); + abbrs_get_set()->add(abbreviation_t(L"testabbrsonetwothreezero", L"expansion")); completions = complete(L"testabbrsonetwothree", {}, parser->context()); do_test(completions.size() == 2); do_test(completions.at(0).completion == L"four"); @@ -3515,6 +3527,7 @@ static void test_complete() { // Abbreviations should not have a space after them. do_test(completions.at(1).completion == L"zero"); do_test((completions.at(1).flags & COMPLETE_NO_SPACE) != 0); + abbrs_get_set()->erase(L"testabbrsonetwothreezero"); // Test wraps. do_test(comma_join(complete_get_wrap_targets(L"wrapper1")).empty()); From d15855d3e31f6ee7de1f5067a3e7b27886388ebc Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 17 Jul 2022 13:27:35 -0700 Subject: [PATCH 07/20] Abbreviations to support matching via regex This adds the --regex option to abbreviations, allowing them to match a pattern of tokens. --- src/abbrs.cpp | 18 +++++++++++--- src/abbrs.h | 23 ++++++++++++++--- src/builtins/abbr.cpp | 55 ++++++++++++++++++++++++++++++++++++++--- src/complete.cpp | 4 +-- src/fish_tests.cpp | 14 +++++++---- tests/checks/abbr.fish | 11 +++++++++ tests/pexpects/abbrs.py | 11 +++++++++ 7 files changed, 118 insertions(+), 18 deletions(-) diff --git a/src/abbrs.cpp b/src/abbrs.cpp index a91d8c07f..d291ac802 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -6,13 +6,22 @@ #include "global_safety.h" #include "wcstringutil.h" -abbreviation_t::abbreviation_t(wcstring name, wcstring replacement, abbrs_position_t position, - bool from_universal) +abbreviation_t::abbreviation_t(wcstring name, wcstring key, wcstring replacement, + abbrs_position_t position, bool from_universal) : name(std::move(name)), + key(std::move(key)), replacement(std::move(replacement)), position(position), from_universal(from_universal) {} +bool abbreviation_t::matches(const wcstring &token) const { + if (this->is_regex()) { + return this->regex->match(token).has_value(); + } else { + return this->key == token; + } +} + acquired_lock abbrs_get_set() { static owning_lock abbrs; return abbrs.acquire(); @@ -28,7 +37,7 @@ maybe_t abbrs_set_t::expand(const wcstring &token, abbrs_position_t po } // Expand only if the name matches. - if (token != abbr.name) { + if (!abbr.matches(token)) { continue; } @@ -89,8 +98,9 @@ void abbrs_set_t::import_from_uvars(const std::unordered_mapadd(abbreviation_t{std::move(name), std::move(replacement), + this->add(abbreviation_t{std::move(name), std::move(key), std::move(replacement), abbrs_position_t::command, from_universal}); } } diff --git a/src/abbrs.h b/src/abbrs.h index ef6d89873..e9540419d 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -8,6 +8,7 @@ #include "common.h" #include "maybe.h" +#include "re.h" class env_var_t; @@ -22,19 +23,33 @@ struct abbreviation_t { // This is used as the token to match unless we have a regex. wcstring name{}; + /// The key (recognized token) - either a literal or a regex pattern. + wcstring key{}; + + /// If set, use this regex to recognize tokens. + /// If unset, the key is to be interpreted literally. + /// Note that the fish interface enforces that regexes match the entire token; + /// we accomplish this by surrounding the regex in ^ and $. + maybe_t regex{}; + // Replacement string. wcstring replacement{}; - // Expansion position. + /// Expansion position. abbrs_position_t position{abbrs_position_t::command}; - // Mark if we came from a universal variable. + /// Mark if we came from a universal variable. bool from_universal{}; // \return true if this is a regex abbreviation. - bool is_regex() const { return false; } + bool is_regex() const { return this->regex.has_value(); } - explicit abbreviation_t(wcstring name, wcstring replacement, + // \return true if we match a token. + bool matches(const wcstring &token) 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. + explicit abbreviation_t(wcstring name, wcstring key, wcstring replacement, abbrs_position_t position = abbrs_position_t::command, bool from_universal = false); diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 99c1827c5..2c9c697ef 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -22,6 +22,7 @@ #include "../common.h" #include "../env.h" #include "../io.h" +#include "../re.h" #include "../wcstringutil.h" #include "../wgetopt.h" #include "../wutil.h" @@ -37,6 +38,7 @@ struct abbr_options_t { bool list{}; bool erase{}; bool query{}; + maybe_t regex_pattern; maybe_t position{}; wcstring_list_t args; @@ -55,7 +57,6 @@ struct abbr_options_t { join_strings(cmds, L", ").c_str()); return false; } - // If run with no options, treat it like --add if we have arguments, // or --show if we do not have any arguments. if (cmds.empty()) { @@ -67,6 +68,10 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --position option requires --add\n"), CMD); return false; } + if (!add && regex_pattern.has_value()) { + streams.err.append_format(_(L"%ls: --regex option requires --add\n"), CMD); + return false; + } return true; } }; @@ -78,7 +83,16 @@ static int abbr_show(const abbr_options_t &, io_streams_t &streams) { wcstring name = escape_string(abbr.name); wcstring value = escape_string(abbr.replacement); const wchar_t *scope = (abbr.from_universal ? L"-U " : L""); - streams.out.append_format(L"abbr -a %ls-- %ls %ls\n", scope, name.c_str(), value.c_str()); + // Literal abbreviations share both name and key. + // Regex abbreviations have a pattern separate from the name. + if (!abbr.is_regex()) { + streams.out.append_format(L"abbr -a %ls-- %ls %ls\n", scope, name.c_str(), + value.c_str()); + } else { + wcstring pattern = escape_string(abbr.key); + streams.out.append_format(L"abbr -a %ls-- %ls --regex %ls %ls\n", scope, name.c_str(), + pattern.c_str(), value.c_str()); + } } return STATUS_CMD_OK; } @@ -167,15 +181,39 @@ static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { name.c_str()); return STATUS_INVALID_ARGS; } + + maybe_t regex; + wcstring key; + if (!opts.regex_pattern.has_value()) { + // The name plays double-duty as the token to replace. + key = name; + } else { + key = *opts.regex_pattern; + re::re_error_t error{}; + // Compile the regex as given; if that succeeds then wrap it in our ^$ so it matches the + // entire token. + if (!re::regex_t::try_compile(*opts.regex_pattern, re::flags_t{}, &error)) { + streams.err.append_format(_(L"%ls: Regular expression compile error: %ls\n"), CMD, + error.message().c_str()); + streams.err.append_format(L"%ls: %ls\n", CMD, opts.regex_pattern->c_str()); + streams.err.append_format(L"%ls: %*ls\n", CMD, static_cast(error.offset), L"^"); + return STATUS_INVALID_ARGS; + } + wcstring anchored = re::make_anchored(*opts.regex_pattern); + regex = re::regex_t::try_compile(anchored, re::flags_t{}, &error); + assert(regex.has_value() && "Anchored compilation should have succeeded"); + } + wcstring replacement; for (auto iter = opts.args.begin() + 1; iter != opts.args.end(); ++iter) { if (!replacement.empty()) replacement.push_back(L' '); replacement.append(*iter); } abbrs_position_t position = opts.position ? *opts.position : abbrs_position_t::command; - abbreviation_t abbr{name, std::move(replacement), position}; // Note historically we have allowed overwriting existing abbreviations. + abbreviation_t abbr{std::move(name), std::move(key), std::move(replacement), position}; + abbr.regex = std::move(regex); abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -212,6 +250,7 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t static const wchar_t *const short_options = L"-arseqgUh"; static const struct woption long_options[] = {{L"add", no_argument, 'a'}, {L"position", required_argument, 'p'}, + {L"regex", required_argument, REGEX_SHORT}, {L"rename", no_argument, 'r'}, {L"erase", no_argument, 'e'}, {L"query", no_argument, 'q'}, @@ -260,6 +299,16 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t } break; } + case REGEX_SHORT: { + if (opts.regex_pattern.has_value()) { + streams.err.append_format(_(L"%ls: Cannot specify multiple regex patterns\n"), + CMD); + return STATUS_INVALID_ARGS; + } + opts.regex_pattern = w.woptarg; + break; + } + case 'r': opts.rename = true; break; diff --git a/src/complete.cpp b/src/complete.cpp index d710fba4c..c90de110c 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -677,8 +677,8 @@ void completer_t::complete_abbr(const wcstring &cmd) { auto abbrs = abbrs_get_set(); for (const auto &abbr : abbrs->list()) { if (!abbr.is_regex()) { - possible_comp.emplace_back(abbr.name); - descs[abbr.name] = abbr.replacement; + possible_comp.emplace_back(abbr.key); + descs[abbr.key] = abbr.replacement; } } } diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 114b419c1..9dbc8d5a7 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2468,11 +2468,15 @@ static void test_ifind_fuzzy() { static void test_abbreviations() { say(L"Testing abbreviations"); { + auto literal_abbr = [](const wchar_t *name, const wchar_t *repl, + abbrs_position_t pos = abbrs_position_t::command) { + return abbreviation_t(name, name /* key */, repl, pos); + }; auto abbrs = abbrs_get_set(); - abbrs->add(abbreviation_t(L"gc", L"git checkout")); - abbrs->add(abbreviation_t(L"foo", L"bar")); - abbrs->add(abbreviation_t(L"gx", L"git checkout")); - abbrs->add(abbreviation_t(L"yin", L"yang", abbrs_position_t::anywhere)); + abbrs->add(literal_abbr(L"gc", L"git checkout")); + abbrs->add(literal_abbr(L"foo", L"bar")); + abbrs->add(literal_abbr(L"gx", L"git checkout")); + abbrs->add(literal_abbr(L"yin", L"yang", abbrs_position_t::anywhere)); } auto cmd = abbrs_position_t::command; @@ -3519,7 +3523,7 @@ static void test_complete() { // Test abbreviations. function_add(L"testabbrsonetwothreefour", func_props); - abbrs_get_set()->add(abbreviation_t(L"testabbrsonetwothreezero", L"expansion")); + abbrs_get_set()->add(abbreviation_t(L"somename", L"testabbrsonetwothreezero", L"expansion")); completions = complete(L"testabbrsonetwothree", {}, parser->context()); do_test(completions.size() == 2); do_test(completions.at(0).completion == L"four"); diff --git a/tests/checks/abbr.fish b/tests/checks/abbr.fish index 9eb0d02e8..89983db52 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -132,3 +132,14 @@ abbr --query banana --position anywhere echo $status # CHECKERR: abbr: --position option requires --add # CHECK: 2 + +# Erase all abbreviations +abbr --erase (abbr --list) +abbr --show +# Should be no output + +abbr --add nonregex_name foo +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 diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py index 41364e274..4b0e76127 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -59,3 +59,14 @@ expect_str(r"") send(r"echo alpha ?") expect_str(r"") + +# Support regex. +sendline(r"abbr alpha --regex 'A[0-9]+Z' beta3") +send(r"A123Z ?") +expect_str(r"") +send(r"AZ ?") +expect_str(r"") +send(r"QA123Z ?") +expect_str(r"") +send(r"A0000000000000000000009Z ?") +expect_str(r"") From 8135c52c13d276ed66eba28012b21158a293dcb6 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 6 Aug 2022 16:57:26 -0700 Subject: [PATCH 08/20] Abbreviations to support functions This adds support for the `--function` option of abbreviations, so that the expansion of an abbreviation may be generated dynamically via a fish function. --- src/abbrs.cpp | 36 +++++++------ src/abbrs.h | 38 ++++++++++--- src/builtins/abbr.cpp | 115 +++++++++++++++++++++++++++++++--------- src/fish_tests.cpp | 25 ++++++--- src/highlight.cpp | 2 +- src/reader.cpp | 45 ++++++++++++---- src/reader.h | 8 +-- tests/checks/abbr.fish | 16 ++++++ tests/pexpects/abbrs.py | 46 ++++++++++++++++ 9 files changed, 261 insertions(+), 70 deletions(-) diff --git a/src/abbrs.cpp b/src/abbrs.cpp index d291ac802..7da84ce2f 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -14,7 +14,11 @@ abbreviation_t::abbreviation_t(wcstring name, wcstring key, wcstring replacement position(position), from_universal(from_universal) {} -bool abbreviation_t::matches(const wcstring &token) const { +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) { + return false; + } if (this->is_regex()) { return this->regex->match(token).has_value(); } else { @@ -27,23 +31,25 @@ acquired_lock abbrs_get_set() { return abbrs.acquire(); } -maybe_t abbrs_set_t::expand(const wcstring &token, abbrs_position_t position) const { +abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t position) 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; - // Expand only if the abbreviation expands anywhere or in the given position. - if (!(abbr.position == position || abbr.position == abbrs_position_t::anywhere)) { - continue; + if (abbr.matches(token, position)) { + result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function}); } - - // Expand only if the name matches. - if (!abbr.matches(token)) { - continue; - } - - return abbr.replacement; } - return none(); + return result; +} + +bool abbrs_set_t::has_match(const wcstring &token, abbrs_position_t position) const { + for (const auto &abbr : abbrs_) { + if (abbr.matches(token, position)) { + return true; + } + } + return false; } void abbrs_set_t::add(abbreviation_t &&abbr) { @@ -106,7 +112,3 @@ void abbrs_set_t::import_from_uvars(const std::unordered_map abbrs_expand(const wcstring &token, abbrs_position_t position) { - return abbrs_get_set()->expand(token, position); -} diff --git a/src/abbrs.h b/src/abbrs.h index e9540419d..db5ff5e45 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -32,20 +32,29 @@ struct abbreviation_t { /// we accomplish this by surrounding the regex in ^ and $. maybe_t regex{}; - // Replacement string. + /// Replacement string. wcstring replacement{}; + /// If set, the replacement is a function name. + bool replacement_is_function{}; + /// Expansion position. abbrs_position_t position{abbrs_position_t::command}; /// 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}; + // \return true if this is a regex abbreviation. bool is_regex() const { return this->regex.has_value(); } - // \return true if we match a token. - bool matches(const wcstring &token) const; + // \return true if we match a token at a given position. + bool matches(const wcstring &token, abbrs_position_t position) 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. @@ -56,11 +65,24 @@ struct abbreviation_t { abbreviation_t() = default; }; +/// The result of an abbreviation expansion. +struct abbrs_replacer_t { + /// The string to use to replace the incoming token, either literal or as a function name. + wcstring replacement; + + /// If true, treat 'replacement' as the name of a function. + bool is_function; +}; +using abbrs_replacer_list_t = std::vector; + class abbrs_set_t { public: - /// \return the replacement value for a abbreviation token, if any. + /// \return the list of replacers for an input token, in priority order. /// The \p position is given to describe where the token was found. - maybe_t expand(const wcstring &token, abbrs_position_t position) const; + abbrs_replacer_list_t match(const wcstring &token, abbrs_position_t position) 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; /// Add an abbreviation. Any abbreviation with the same name is replaced. void add(abbreviation_t &&abbr); @@ -94,8 +116,10 @@ class abbrs_set_t { /// \return the global mutable set of abbreviations. acquired_lock abbrs_get_set(); -/// \return the replacement value for a abbreviation token, if any, using the global 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. -maybe_t abbrs_expand(const wcstring &token, abbrs_position_t position); +inline abbrs_replacer_list_t abbrs_match(const wcstring &token, abbrs_position_t position) { + return abbrs_get_set()->match(token, position); +} #endif diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 2c9c697ef..90d4948e4 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -38,9 +38,13 @@ struct abbr_options_t { bool list{}; bool erase{}; bool query{}; + bool function{}; maybe_t regex_pattern; maybe_t position{}; + bool expand_on_entry{}; + bool expand_on_execute{}; + wcstring_list_t args; bool validate(io_streams_t &streams) { @@ -72,6 +76,21 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --regex option requires --add\n"), CMD); return false; } + if (!add && function) { + 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); + 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; } }; @@ -79,20 +98,36 @@ struct abbr_options_t { // Print abbreviations in a fish-script friendly way. static int abbr_show(const abbr_options_t &, io_streams_t &streams) { const auto abbrs = abbrs_get_set(); + wcstring_list_t comps{}; for (const auto &abbr : abbrs->list()) { - wcstring name = escape_string(abbr.name); - wcstring value = escape_string(abbr.replacement); - const wchar_t *scope = (abbr.from_universal ? L"-U " : L""); - // Literal abbreviations share both name and key. + comps.clear(); + comps.push_back(L"abbr -a"); + if (abbr.from_universal) comps.push_back(L"-U"); + comps.push_back(L"--"); + // Literal abbreviations have the name and key as the same. // Regex abbreviations have a pattern separate from the name. - if (!abbr.is_regex()) { - streams.out.append_format(L"abbr -a %ls-- %ls %ls\n", scope, name.c_str(), - value.c_str()); - } else { - wcstring pattern = escape_string(abbr.key); - streams.out.append_format(L"abbr -a %ls-- %ls --regex %ls %ls\n", scope, name.c_str(), - pattern.c_str(), value.c_str()); + comps.push_back(escape_string(abbr.name)); + if (abbr.is_regex()) { + 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.replacement_is_function) { + comps.push_back(L"--function"); + } + comps.push_back(escape_string(abbr.replacement)); + wcstring result = join_strings(comps, L' '); + result.push_back(L'\n'); + streams.out.append(result); } return STATUS_CMD_OK; } @@ -209,11 +244,21 @@ static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { if (!replacement.empty()) replacement.push_back(L' '); replacement.append(*iter); } + // Abbreviation function names disallow spaces. + // This is to prevent accidental usage of e.g. `--function 'string replace'` + if (opts.function && + (!valid_func_name(replacement) || replacement.find(L' ') != wcstring::npos)) { + streams.err.append_format(_(L"%ls: Invalid function name: %ls\n"), CMD, + replacement.c_str()); + return STATUS_INVALID_ARGS; + } + abbrs_position_t position = opts.position ? *opts.position : abbrs_position_t::command; // Note historically we have allowed overwriting existing abbreviations. abbreviation_t abbr{std::move(name), std::move(key), std::move(replacement), position}; abbr.regex = std::move(regex); + abbr.replacement_is_function = opts.function; abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -242,24 +287,27 @@ 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 }; + enum { NON_OPTION_ARGUMENT = 1, REGEX_SHORT, EXPAND_ON_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"-arseqgUh"; - static const struct woption long_options[] = {{L"add", no_argument, 'a'}, - {L"position", required_argument, 'p'}, - {L"regex", required_argument, REGEX_SHORT}, - {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 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'}, + {}}; int argc = builtin_count_args(argv); int opt; @@ -308,7 +356,22 @@ 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; + } + break; + } + case 'f': + opts.function = true; + break; case 'r': opts.rename = true; break; diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 9dbc8d5a7..fd0fed0ad 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2479,22 +2479,35 @@ static void test_abbreviations() { abbrs->add(literal_abbr(L"yin", L"yang", abbrs_position_t::anywhere)); } - auto cmd = abbrs_position_t::command; - if (abbrs_expand(L"", cmd)) err(L"Unexpected success with empty abbreviation"); - if (abbrs_expand(L"nothing", cmd)) err(L"Unexpected success with missing abbreviation"); + // 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); + if (result.size() > 1) { + err(L"abbreviation expansion for %ls returned more than 1 result", token.c_str()); + } + if (result.empty()) { + return none(); + } + return result.front().replacement; + }; - auto mresult = abbrs_expand(L"gc", cmd); + auto cmd = abbrs_position_t::command; + if (abbr_expand_1(L"", cmd)) err(L"Unexpected success with empty abbreviation"); + if (abbr_expand_1(L"nothing", cmd)) err(L"Unexpected success with missing abbreviation"); + + auto mresult = abbr_expand_1(L"gc", cmd); if (!mresult) err(L"Unexpected failure with gc abbreviation"); if (*mresult != L"git checkout") err(L"Wrong abbreviation result for gc"); - mresult = abbrs_expand(L"foo", cmd); + mresult = abbr_expand_1(L"foo", cmd); if (!mresult) err(L"Unexpected failure with foo abbreviation"); if (*mresult != L"bar") err(L"Wrong abbreviation result for foo"); 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)) { + if (auto edit = reader_expand_abbreviation_at_cursor(cmdline, cursor_pos, + parser_t::principal_parser())) { wcstring cmdline_expanded = cmdline; std::vector colors{cmdline_expanded.size()}; apply_edit(&cmdline_expanded, &colors, *edit); diff --git a/src/highlight.cpp b/src/highlight.cpp index 680cd9466..ce0f29977 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_expand(cmd, abbrs_position_t::command).has_value(); + is_valid = abbrs_get_set()->has_match(cmd, abbrs_position_t::command); // 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 1a3b8ee09..57bf266e4 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1347,8 +1347,30 @@ 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. +maybe_t expand_replacer(const wcstring &token, const abbrs_replacer_t &repl, + parser_t &parser) { + if (!repl.is_function) { + // Literal replacement cannot fail. + return repl.replacement; + } + + wcstring cmd = escape_string(repl.replacement); + cmd.push_back(L' '); + cmd.append(escape_string(token)); + + 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'); +} + /// 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) { +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; @@ -1400,14 +1422,17 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, si leaf_is_command ? abbrs_position_t::command : abbrs_position_t::anywhere; // Now we can expand the abbreviation. - maybe_t result{}; - if (auto abbreviation = abbrs_expand(leaf->source(subcmd), expand_position)) { - // There was an abbreviation! Replace the token in the full command. Maintain the - // relative position of the cursor. - source_range_t r = leaf->source_range(); - result = edit_t(subcmd_offset + r.start, r.length, abbreviation.acquire()); + // Find the first which succeeds. + wcstring token = leaf->source(subcmd); + auto replacers = abbrs_match(token, expand_position); + 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()); + } } - return result; + return none(); } /// Expand abbreviations at the current cursor position, minus the given cursor backtrack. This may @@ -1419,9 +1444,9 @@ bool reader_data_t::expand_abbreviation_as_necessary(size_t cursor_backtrack) { if (conf.expand_abbrev_ok && el == &command_line) { // 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)) { + if (auto edit = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, parser())) { push_edit(el, std::move(*edit)); update_buff_pos(el); result = true; diff --git a/src/reader.h b/src/reader.h index 284fb91a1..9a5b94da0 100644 --- a/src/reader.h +++ b/src/reader.h @@ -261,9 +261,11 @@ bool fish_is_unwinding_for_exit(); wcstring combine_command_and_autosuggestion(const wcstring &cmdline, const wcstring &autosuggestion); -/// Expand abbreviations at the given cursor position. Exposed for testing purposes only. -/// \return none if no abbreviations were expanded, otherwise the new command line. -maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos); +/// 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. +maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, + 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 89983db52..cfd0e0622 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -133,6 +133,22 @@ echo $status # CHECKERR: abbr: --position option requires --add # CHECK: 2 +abbr --query banana --function +echo $status +# CHECKERR: abbr: --function option requires --add +# CHECK: 2 + +abbr --add peach --function invalid/function/name +echo $status +# CHECKERR: abbr: Invalid function name: invalid/function/name +# CHECK: 2 + +# Function names cannot contain spaces, to prevent confusion with fish script. +abbr --add peach --function 'no space allowed' +echo $status +# CHECKERR: abbr: Invalid function name: no space allowed +# CHECK: 2 + # Erase all abbreviations abbr --erase (abbr --list) abbr --show diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py index 4b0e76127..e89e62ae7 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -53,6 +53,7 @@ expect_str(r"") # Support position anywhere. sendline(r"abbr alpha --position anywhere beta2") +expect_prompt() send(r"alpha ?") expect_str(r"") @@ -62,6 +63,7 @@ expect_str(r"") # Support regex. sendline(r"abbr alpha --regex 'A[0-9]+Z' beta3") +expect_prompt() send(r"A123Z ?") expect_str(r"") send(r"AZ ?") @@ -70,3 +72,47 @@ send(r"QA123Z ?") expect_str(r"") send(r"A0000000000000000000009Z ?") expect_str(r"") + +# Support functions. Here anything in between @@ is uppercased, except 'nope'. +sendline( + r"""function uppercaser; + string match --quiet '*nope*' $argv[1] && return 1 + string trim --chars @ $argv | string upper + end""" +) +expect_prompt() +sendline(r"abbr uppercaser --regex '@.+@' --function uppercaser") +expect_prompt() +send(r"@abc@ ?") +expect_str(r"") +send(r"@nope@ ?") +expect_str(r"<@nope@ >") +sendline(r"abbr --erase uppercaser") +expect_prompt() + +# -f works as shorthand for --function. +sendline(r"abbr uppercaser2 --regex '@.+@' -f uppercaser") +expect_prompt() +send(r"@abc@ ?") +expect_str(r"") +send(r"@nope@ ?") +expect_str(r"<@nope@ >") +sendline(r"abbr --erase uppercaser2") +expect_prompt() + +# Verify that 'commandline' is accurate. +# Abbreviation functions cannot usefully change the command line, but they can read it. +sendline( + r"""function check_cmdline + set -g last_cmdline (commandline) + set -g last_cursor (commandline --cursor) + false + end""" +) +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@ ") From 1d205d0bbda2acd9c261248e0f141c813d2a96cb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 7 Aug 2022 13:31:10 -0700 Subject: [PATCH 09/20] 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") From 7118cb1ae1d520af369a81d97abe3394b0af6715 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 9 Oct 2022 14:26:44 -0700 Subject: [PATCH 10/20] Implement set-cursor for abbreviations set-cursor enables abbreviations to specify the cursor location after expansion, by passing in a string which is expected to be found in the expansion. For example you may create an abbreviation like `L!`: abbr L! --position anywhere --set-cursor ! "! | less" and the cursor will be positioned where the "!" is after expansion, with the "| less" appearing to its right. --- src/abbrs.cpp | 19 ++++++++++++++++++- src/abbrs.h | 26 ++++++++++++++++++++++++++ src/builtins/abbr.cpp | 28 ++++++++++++++++++++++++++++ src/fish_tests.cpp | 4 ++-- src/reader.cpp | 41 ++++++++++++++++++++++------------------- src/reader.h | 16 +++++++++++----- tests/pexpects/abbrs.py | 8 ++++++++ 7 files changed, 115 insertions(+), 27 deletions(-) diff --git a/src/abbrs.cpp b/src/abbrs.cpp index 4891df93e..dfb9cd623 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -54,7 +54,8 @@ abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t for (auto it = abbrs_.rbegin(); it != abbrs_.rend(); ++it) { const abbreviation_t &abbr = *it; if (abbr.matches(token, position, phase)) { - result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function}); + result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function, + abbr.set_cursor_indicator}); } } return result; @@ -130,3 +131,19 @@ void abbrs_set_t::import_from_uvars(const std::unordered_mapsize()); + result.cursor = pos + range.start; + } + } + return result; +} diff --git a/src/abbrs.h b/src/abbrs.h index 3880f8add..34aaa39dc 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -8,6 +8,7 @@ #include "common.h" #include "maybe.h" +#include "parse_constants.h" #include "re.h" class env_var_t; @@ -48,6 +49,9 @@ struct abbreviation_t { /// Expansion position. abbrs_position_t position{abbrs_position_t::command}; + /// If set, then move the cursor to the first instance of this string in the expansion. + maybe_t set_cursor_indicator{}; + /// Mark if we came from a universal variable. bool from_universal{}; @@ -84,9 +88,31 @@ struct abbrs_replacer_t { /// If true, treat 'replacement' as the name of a function. bool is_function; + + /// If set, the cursor should be moved to the first instance of this string in the expansion. + maybe_t set_cursor_indicator; }; using abbrs_replacer_list_t = std::vector; +/// A helper type for replacing a range in a string. +struct abbrs_replacement_t { + /// The original range of the token in the command line. + source_range_t range{}; + + /// The string to replace with. + wcstring text{}; + + /// The new cursor location, or none to use the default. + /// This is relative to the original range. + maybe_t cursor{}; + + /// Construct a replacement from a replacer. + /// The \p range is the range of the text matched by the replacer in the command line. + /// The text is passed in separately as it may be the output of the replacer's function. + static abbrs_replacement_t from(source_range_t range, wcstring text, + const abbrs_replacer_t &replacer); +}; + class abbrs_set_t { public: /// \return the list of replacers for an input token, in priority order. diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 9880e06dc..7ca6de259 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -41,6 +41,7 @@ struct abbr_options_t { bool function{}; maybe_t regex_pattern; maybe_t position{}; + maybe_t set_cursor_indicator{}; bool quiet{}; @@ -83,6 +84,18 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --quiet option requires --add\n"), CMD); return false; } + if (!add && set_cursor_indicator.has_value()) { + streams.err.append_format(_(L"%ls: --set-cursor option requires --add\n"), CMD); + return false; + } + if (set_cursor_indicator.has_value() && quiet) { + streams.err.append_format(_(L"%ls: --quiet cannot be used with --set-cursor\n"), CMD); + return false; + } + if (set_cursor_indicator.has_value() && set_cursor_indicator->empty()) { + streams.err.append_format(_(L"%ls: --set-cursor argument cannot be empty\n"), CMD); + return false; + } return true; } }; @@ -103,6 +116,10 @@ static int abbr_show(const abbr_options_t &, io_streams_t &streams) { comps.push_back(L"--regex"); comps.push_back(escape_string(abbr.key)); } + if (abbr.set_cursor_indicator.has_value()) { + comps.push_back(L"--set-cursor"); + comps.push_back(escape_string(*abbr.set_cursor_indicator)); + } if (abbr.is_quiet) { comps.push_back(L"--quiet"); } @@ -244,6 +261,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.set_cursor_indicator = opts.set_cursor_indicator; abbr.is_quiet = opts.quiet; abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; @@ -283,6 +301,7 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t {L"position", required_argument, 'p'}, {L"regex", required_argument, REGEX_SHORT}, {L"quiet", no_argument, QUIET_SHORT}, + {L"set-cursor", required_argument, 'C'}, {L"function", no_argument, 'f'}, {L"rename", no_argument, 'r'}, {L"erase", no_argument, 'e'}, @@ -345,6 +364,15 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t opts.quiet = true; break; } + case 'C': { + if (opts.set_cursor_indicator.has_value()) { + streams.err.append_format( + _(L"%ls: Cannot specify multiple set-cursor options\n"), CMD); + return STATUS_INVALID_ARGS; + } + opts.set_cursor_indicator = w.woptarg; + break; + } case 'f': opts.function = true; break; diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index b7bc8b24f..292bcca1e 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2506,11 +2506,11 @@ 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( + if (auto replacement = 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); + apply_edit(&cmdline_expanded, &colors, edit_t{replacement->range, replacement->text}); return cmdline_expanded; } return none_t(); diff --git a/src/reader.cpp b/src/reader.cpp index 89899e49c..bb3263b75 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1362,15 +1362,15 @@ 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. This may run fish script! -maybe_t expand_replacer(const wcstring &token, const abbrs_replacer_t &repl, - parser_t &parser) { +/// Expand an abbreviation replacer, which may mean running its function. +/// \return the replacement, or none to skip it. This may run fish script! +maybe_t expand_replacer(source_range_t range, 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; + return abbrs_replacement_t::from(range, repl.replacement, repl); } wcstring cmd = escape_string(repl.replacement); @@ -1386,7 +1386,7 @@ maybe_t expand_replacer(const wcstring &token, const abbrs_replacer_t } wcstring result = join_strings(outputs, L'\n'); FLOGF(abbrs, L"Expanded function abbreviation <%ls> -> <%ls>", token.c_str(), result.c_str()); - return result; + return abbrs_replacement_t::from(range, std::move(result), repl); } // Extract all the token ranges in \p str, along with whether they are an undecorated command. @@ -1449,9 +1449,12 @@ static std::vector extract_tokens(const wcstring &str) { 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, - abbrs_phase_t phase, parser_t &parser) { +/// Expand abbreviations in the given phase at the given cursor position. +/// cursor. \return the replacement. This does NOT inspect the current reader data. +maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, + size_t cursor_pos, + 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) { @@ -1467,8 +1470,8 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, si 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_str, replacer, parser)) { - return edit_t{range.start, range.length, replacement.acquire()}; + if (auto replacement = expand_replacer(range, token_str, replacer, parser)) { + return replacement; } } return none(); @@ -1485,10 +1488,10 @@ bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs // 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, phase, parser())) { - push_edit(el, std::move(*edit)); - update_buff_pos(el); + if (auto replacement = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, phase, + this->parser())) { + push_edit(el, edit_t{replacement->range, std::move(replacement->text)}); + update_buff_pos(el, replacement->cursor); result = true; } } @@ -1516,13 +1519,13 @@ static bool expand_quiet_abbreviations(wcstring *inout_str, parser_t &parser) { 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) { + const auto replacement = expand_replacer(orig_range, token, replacer, parser); + if (replacement && replacement->text != token) { modified = true; result.replace(orig_range.start + repl_lengths - orig_lengths, orig_range.length, - *replacement); + replacement->text); orig_lengths += orig_range.length; - repl_lengths += replacement->length(); + repl_lengths += replacement->text.length(); break; } } diff --git a/src/reader.h b/src/reader.h index 0303a60ea..d6558a533 100644 --- a/src/reader.h +++ b/src/reader.h @@ -42,6 +42,9 @@ struct edit_t { explicit edit_t(size_t offset, size_t length, wcstring replacement) : offset(offset), length(length), replacement(std::move(replacement)) {} + explicit edit_t(source_range_t range, wcstring replacement) + : edit_t(range.start, range.length, std::move(replacement)) {} + /// Used for testing. bool operator==(const edit_t &other) const; }; @@ -261,12 +264,15 @@ bool fish_is_unwinding_for_exit(); wcstring combine_command_and_autosuggestion(const wcstring &cmdline, const wcstring &autosuggestion); -/// 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. +/// Expand at most one abbreviation at the given cursor position, updating the position if the +/// abbreviation wants to move the cursor. 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, - abbrs_phase_t phase, parser_t &parser); +struct abbrs_replacement_t; +maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, + size_t cursor_pos, + 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/pexpects/abbrs.py b/tests/pexpects/abbrs.py index 099bf86f2..516c94cd1 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -152,3 +152,11 @@ expect_prompt() # The quiet one sees a token starting with 'a' and ending with 'z' and uppercases it. sendline(r"echo %abcdez%") expect_prompt(r"ABCDEZ") + +# Test cursor positioning. +sendline(r"""abbr --erase (abbr --list) """) +expect_prompt() +sendline(r"""abbr LLL --position anywhere --set-cursor !HERE! '!HERE! | less'""") +expect_prompt() +send(r"""echo LLL derp?""") +expect_str(r"echo derp | less ") From c51a1f1f606d69060c000b8f8e6dab30ec6c31cd Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 16 Oct 2022 14:32:25 -0700 Subject: [PATCH 11/20] Implement trigger-on for abbreviations trigger-on enables abbreviations to trigger only on "entry" (anything which closes a token, like space) or only on "exec" (typically enter key). --- src/abbrs.cpp | 24 ++++++-------------- src/abbrs.h | 42 ++++++++++++++++++++++------------- src/builtins/abbr.cpp | 49 ++++++++++++++++++++++++++++++++++------- src/fish_tests.cpp | 4 ++-- src/highlight.cpp | 2 +- src/reader.cpp | 21 +++++++++--------- src/reader.h | 4 ++-- tests/checks/abbr.fish | 15 +++++++++++++ tests/pexpects/abbrs.py | 17 ++++++++++++++ 9 files changed, 123 insertions(+), 55 deletions(-) diff --git a/src/abbrs.cpp b/src/abbrs.cpp index dfb9cd623..31f377587 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -18,21 +18,11 @@ 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_phases(abbrs_phases_t p) const { return bool(this->phases & p); } 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)) { + abbrs_phases_t phases) const { + if (!this->matches_position(position) || !this->matches_phases(phases)) { return false; } if (this->is_regex()) { @@ -48,12 +38,12 @@ acquired_lock abbrs_get_set() { } abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t position, - abbrs_phase_t phase) const { + abbrs_phases_t phases) 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, phase)) { + if (abbr.matches(token, position, phases)) { result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function, abbr.set_cursor_indicator}); } @@ -62,9 +52,9 @@ abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t } bool abbrs_set_t::has_match(const wcstring &token, abbrs_position_t position, - abbrs_phase_t phase) const { + abbrs_phases_t phases) const { for (const auto &abbr : abbrs_) { - if (abbr.matches(token, position, phase)) { + if (abbr.matches(token, position, phases)) { return true; } } diff --git a/src/abbrs.h b/src/abbrs.h index 34aaa39dc..56c9576f3 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -20,11 +20,24 @@ enum class abbrs_position_t : uint8_t { }; /// 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. +enum abbrs_phase_t : uint8_t { + // Expands "on space" immediately after the user types it. + abbrs_phase_entry = 1 << 0, + + // Expands "on enter" before submitting the command to be executed. + abbrs_phase_exec = 1 << 1, + + // Expands "quietly" after submitting the command. This is incompatible with the other + // phases. + abbrs_phase_quiet = 1 << 2, + + // Default set of phases. + abbrs_phases_default = abbrs_phase_entry | abbrs_phase_exec, + + // All phases. + abbrs_phases_all = abbrs_phase_entry | abbrs_phase_exec | abbrs_phase_quiet, }; +using abbrs_phases_t = uint8_t; struct abbreviation_t { // Abbreviation name. This is unique within the abbreviation set. @@ -55,15 +68,14 @@ struct abbreviation_t { /// Mark if we came from a universal variable. bool from_universal{}; - /// 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}; + /// Set of phases in which this abbreviation expands. + abbrs_phases_t phases{abbrs_phases_default}; // \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 in a given phase. - bool matches(const wcstring &token, abbrs_position_t position, abbrs_phase_t phase) const; + // \return true if we match a token at a given position in a given set of phases. + bool matches(const wcstring &token, abbrs_position_t position, abbrs_phases_t phases) 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. @@ -77,8 +89,8 @@ struct abbreviation_t { // \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; + // \return if we expand in a given set of phases. + bool matches_phases(abbrs_phases_t p) const; }; /// The result of an abbreviation expansion. @@ -118,10 +130,10 @@ class abbrs_set_t { /// \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, - abbrs_phase_t phase) const; + abbrs_phases_t phases) const; /// \return whether we would have at least one replacer for a given token. - bool has_match(const wcstring &token, abbrs_position_t position, abbrs_phase_t phase) const; + bool has_match(const wcstring &token, abbrs_position_t position, abbrs_phases_t phases) const; /// Add an abbreviation. Any abbreviation with the same name is replaced. void add(abbreviation_t &&abbr); @@ -158,8 +170,8 @@ 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, - abbrs_phase_t phase) { - return abbrs_get_set()->match(token, position, phase); + abbrs_phases_t phases) { + return abbrs_get_set()->match(token, position, phases); } #endif diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 7ca6de259..ff308b5b4 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -41,13 +41,14 @@ struct abbr_options_t { bool function{}; maybe_t regex_pattern; maybe_t position{}; + maybe_t phases{}; maybe_t set_cursor_indicator{}; - bool quiet{}; - wcstring_list_t args; bool validate(io_streams_t &streams) { + const bool quiet = (phases.value_or(0) & abbrs_phase_quiet); + // Duplicate options? wcstring_list_t cmds; if (add) cmds.push_back(L"add"); @@ -83,6 +84,9 @@ struct abbr_options_t { if (!add && quiet) { streams.err.append_format(_(L"%ls: --quiet option requires --add\n"), CMD); return false; + } else if (!add && phases.has_value()) { + streams.err.append_format(_(L"%ls: --trigger-on option requires --add\n"), CMD); + return false; } if (!add && set_cursor_indicator.has_value()) { streams.err.append_format(_(L"%ls: --set-cursor option requires --add\n"), CMD); @@ -96,6 +100,11 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --set-cursor argument cannot be empty\n"), CMD); return false; } + if (quiet && (*phases & ~abbrs_phase_quiet) != 0) { + streams.err.append_format(_(L"%ls: Cannot use --quiet with --trigger-on\n"), CMD); + return false; + } + return true; } }; @@ -120,8 +129,16 @@ static int abbr_show(const abbr_options_t &, io_streams_t &streams) { comps.push_back(L"--set-cursor"); comps.push_back(escape_string(*abbr.set_cursor_indicator)); } - if (abbr.is_quiet) { - comps.push_back(L"--quiet"); + if (abbr.phases != abbrs_phases_default) { + if (abbr.phases & abbrs_phase_entry) { + comps.push_back(L"--trigger-on entry"); + } + if (abbr.phases & abbrs_phase_exec) { + comps.push_back(L"--trigger-on exec"); + } + if (abbr.phases & abbrs_phase_quiet) { + comps.push_back(L"--quiet"); + } } if (abbr.replacement_is_function) { comps.push_back(L"--function"); @@ -262,7 +279,7 @@ static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { abbr.regex = std::move(regex); abbr.replacement_is_function = opts.function; abbr.set_cursor_indicator = opts.set_cursor_indicator; - abbr.is_quiet = opts.quiet; + abbr.phases = opts.phases.value_or(abbrs_phases_default); abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -300,6 +317,7 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t static const struct woption long_options[] = {{L"add", no_argument, 'a'}, {L"position", required_argument, 'p'}, {L"regex", required_argument, REGEX_SHORT}, + {L"trigger-on", required_argument, 't'}, {L"quiet", no_argument, QUIET_SHORT}, {L"set-cursor", required_argument, 'C'}, {L"function", no_argument, 'f'}, @@ -344,8 +362,8 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t } else if (!wcscmp(w.woptarg, L"anywhere")) { opts.position = abbrs_position_t::anywhere; } else { - streams.err.append_format(_(L"%ls: Invalid position '%ls'\nPosition must be " - L"one of: command, anywhere.\n"), + streams.err.append_format(_(L"%ls: Invalid position '%ls'\n" + L"Position must be one of: command, anywhere.\n"), CMD, w.woptarg); return STATUS_INVALID_ARGS; } @@ -360,8 +378,23 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t opts.regex_pattern = w.woptarg; break; } + case 't': { + abbrs_phases_t phases = opts.phases.value_or(0); + if (!wcscmp(w.woptarg, L"entry")) { + phases |= abbrs_phase_entry; + } else if (!wcscmp(w.woptarg, L"exec")) { + phases |= abbrs_phase_exec; + } else { + streams.err.append_format(_(L"%ls: Invalid --trigger-on '%ls'\n" + L"Must be one of: entry, exec.\n"), + CMD, w.woptarg); + return STATUS_INVALID_ARGS; + } + opts.phases = phases; + break; + } case QUIET_SHORT: { - opts.quiet = true; + opts.phases = opts.phases.value_or(0) | abbrs_phase_quiet; break; } case 'C': { diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 292bcca1e..031ede549 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, abbrs_phase_t::any); + auto result = abbrs_match(token, pos, abbrs_phases_all); if (result.size() > 1) { err(L"abbreviation expansion for %ls returned more than 1 result", token.c_str()); } @@ -2507,7 +2507,7 @@ static void test_abbreviations() { auto expand_abbreviation_in_command = [](const wcstring &cmdline, size_t cursor_pos) -> maybe_t { if (auto replacement = reader_expand_abbreviation_at_cursor( - cmdline, cursor_pos, abbrs_phase_t::noisy, parser_t::principal_parser())) { + cmdline, cursor_pos, abbrs_phases_default, parser_t::principal_parser())) { wcstring cmdline_expanded = cmdline; std::vector colors{cmdline_expanded.size()}; apply_edit(&cmdline_expanded, &colors, edit_t{replacement->range, replacement->text}); diff --git a/src/highlight.cpp b/src/highlight.cpp index f4ad0e32f..a7885bfee 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, abbrs_phase_t::any); + is_valid = abbrs_get_set()->has_match(cmd, abbrs_position_t::command, abbrs_phases_default); // 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 bb3263b75..1726f8efe 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -792,8 +792,9 @@ class reader_data_t : public std::enable_shared_from_this { /// Do what we need to do whenever our pager selection changes. void pager_selection_changed(); - /// Expand abbreviations at the current cursor position, minus backtrack_amt. - bool expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_phase_t phase); + /// Expand abbreviations in the given phases at the current cursor position, minus + /// cursor_backtrack. + bool expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_phases_t phases); /// \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. @@ -1453,7 +1454,7 @@ static std::vector extract_tokens(const wcstring &str) { /// cursor. \return the replacement. This does NOT inspect the current reader data. maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - abbrs_phase_t phase, + abbrs_phases_t phases, parser_t &parser) { // Find the token containing the cursor. Usually users edit from the end, so walk backwards. const auto tokens = extract_tokens(cmdline); @@ -1468,7 +1469,7 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring iter->is_cmd ? abbrs_position_t::command : abbrs_position_t::anywhere; wcstring token_str = cmdline.substr(range.start, range.length); - auto replacers = abbrs_match(token_str, position, phase); + auto replacers = abbrs_match(token_str, position, phases); for (const auto &replacer : replacers) { if (auto replacement = expand_replacer(range, token_str, replacer, parser)) { return replacement; @@ -1480,7 +1481,7 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring /// 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_at_cursor(size_t cursor_backtrack, abbrs_phase_t phase) { +bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_phases_t phases) { bool result = false; editable_line_t *el = active_edit_line(); @@ -1488,7 +1489,7 @@ bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs // Try expanding abbreviations. this->update_commandline_state(); size_t cursor_pos = el->position() - std::min(el->position(), cursor_backtrack); - if (auto replacement = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, phase, + if (auto replacement = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, phases, this->parser())) { push_edit(el, edit_t{replacement->range, std::move(replacement->text)}); update_buff_pos(el, replacement->cursor); @@ -1517,7 +1518,7 @@ static bool expand_quiet_abbreviations(wcstring *inout_str, parser_t &parser) { 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); + auto replacers = abbrs_match(token, position, abbrs_phase_quiet); for (const auto &replacer : replacers) { const auto replacement = expand_replacer(orig_range, token, replacer, parser); if (replacement && replacement->text != token) { @@ -4230,7 +4231,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } case rl::expand_abbr: { - if (expand_abbreviation_at_cursor(1, abbrs_phase_t::noisy)) { + if (expand_abbreviation_at_cursor(1, abbrs_phase_entry)) { inputter.function_set_status(true); } else { inputter.function_set_status(false); @@ -4328,9 +4329,9 @@ parser_test_error_bits_t reader_data_t::expand_for_execute(wcstring *to_exec) { if (test_res & PARSER_TEST_ERROR) return test_res; } - // Noisy abbreviations at the cursor. + // Exec abbreviations at the cursor. // Note we want to expand abbreviations even if incomplete. - if (expand_abbreviation_at_cursor(0, abbrs_phase_t::noisy)) { + if (expand_abbreviation_at_cursor(0, abbrs_phase_exec)) { // Trigger syntax highlighting as we are likely about to execute this command. this->super_highlight_me_plenty(); if (conf.syntax_check_ok) { diff --git a/src/reader.h b/src/reader.h index d6558a533..e1c707bf4 100644 --- a/src/reader.h +++ b/src/reader.h @@ -267,11 +267,11 @@ wcstring combine_command_and_autosuggestion(const wcstring &cmdline, /// Expand at most one abbreviation at the given cursor position, updating the position if the /// abbreviation wants to move the cursor. 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; +using abbrs_phases_t = uint8_t; struct abbrs_replacement_t; maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - abbrs_phase_t phase, + abbrs_phases_t phases, parser_t &parser); /// Apply a completion string. Exposed for testing only. diff --git a/tests/checks/abbr.fish b/tests/checks/abbr.fish index 9245c5f5b..e4a94dcda 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -168,8 +168,23 @@ abbr --add bogus --position never stuff abbr --add bogus --position anywhere --position command stuff # CHECKERR: abbr: Cannot specify multiple positions +abbr --add --trigger-on derp zzxjoanw stuff +# CHECKERR: abbr: Invalid --trigger-on 'derp' +# CHECKERR: Must be one of: entry, exec. + +abbr --add --trigger-on entry --quiet zzxjoanw stuff +# CHECKERR: abbr: Cannot use --quiet with --trigger-on + +abbr --add --quiet --trigger-on exec zzxjoanw stuff +# CHECKERR: abbr: Cannot use --quiet with --trigger-on + 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 + +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 516c94cd1..c9540cc39 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -160,3 +160,20 @@ sendline(r"""abbr LLL --position anywhere --set-cursor !HERE! '!HERE! | less'""" expect_prompt() send(r"""echo LLL derp?""") expect_str(r"echo derp | less ") + +# Test trigger-on. +sendline(r"""abbr --erase (abbr --list) """) +expect_prompt() +sendline(r"""abbr --add entry-only --position anywhere --trigger-on entry 'worked1'""") +expect_prompt() +sendline(r"""abbr --add exec-only --position anywhere --trigger-on exec 'worked2'""") +expect_prompt() +sendline(r"echo entry-only") # should not trigger, no space +expect_prompt(r"entry-only") +sendline(r"echo entry-only ") # should trigger, there is a space +expect_prompt(r"worked1") + +sendline(r"echo exec-only") # should trigger, no space +expect_prompt(r"worked2") +sendline(r"echo exec-only ") # should not trigger, there is a space +expect_prompt(r"exec-only") From 22bd43f9d58c8d5afb5062621905e2cdb7e169ef Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 22 Aug 2022 11:05:20 -0700 Subject: [PATCH 12/20] Document new abbreviation features --- doc_src/cmds/abbr.rst | 134 ++++++++++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 44 deletions(-) diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index 17c65319c..8e606c283 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -8,12 +8,14 @@ Synopsis .. synopsis:: - abbr --add [SCOPE] WORD EXPANSION - abbr --erase WORD ... - abbr --rename [SCOPE] OLD_WORD NEW_WORD + abbr --add NAME [--quiet] [--position command | anywhere] [--regex PATTERN] + [--set-cursor SENTINEL] [--trigger-on entry | exec]... + [-f | --function] EXPANSION + abbr --erase NAME ... + abbr --rename OLD_WORD NEW_WORD abbr --show abbr --list - abbr --query WORD ... + abbr --query NAME ... Description ----------- @@ -23,80 +25,124 @@ Description For example, a frequently-run command like ``git checkout`` can be abbreviated to ``gco``. After entering ``gco`` and pressing :kbd:`Space` or :kbd:`Enter`, the full text ``git checkout`` will appear in the command line. -Options -------- +An abbreviation may match a literal word, or it may match a pattern given by a regular expression. When an abbreviation matches a word, that word is replaced by new text, called its *expansion*. This expansion may be a fixed new phrase, or it can be dynamically created via a fish function. -The following options are available: +Abbreviations may expand either after their word is entered, or if they are executed with the enter key. The ``--trigger-on`` option allows limiting expansion to only entering, or only executing. -**-a** *WORD* *EXPANSION* or **--add** *WORD* *EXPANSION* - Adds a new abbreviation, causing *WORD* to be expanded to *EXPANSION* +By default abbreviations print the new command line after being entered. The ``--quiet`` option causes abbreviations to expand silently. -**-r** *OLD_WORD* *NEW_WORD* or **--rename** *OLD_WORD* *NEW_WORD* - Renames an abbreviation, from *OLD_WORD* to *NEW_WORD* +Combining these features, it is possible to create custom syntaxes, where a regular expression recognizes matching tokens, and the expansion function interprets them. See the `Examples`_ section. -**-s** or **--show** - Show all abbreviations in a manner suitable for import and export +Abbreviations may be added to :ref:`config.fish `. Abbreviations are only expanded for typed-in commands, not in scripts. -**-l** or **--list** - Lists all abbreviated words -**-e** *WORD* or **--erase** *WORD* ... - Erase the given abbreviations +"add" subcommand +-------------------- -**-q** or **--query** - Return 0 (true) if one of the *WORD* is an abbreviation. +.. synopsis:: -**-h** or **--help** - Displays help about using this command. + abbr [-a | --add] NAME [--quiet] [--position command | anywhere] [--regex PATTERN] + [--set-cursor SENTINEL] [--trigger-on entry | exec]... + [-f | --function] EXPANSION -In addition, when adding or renaming abbreviations, one of the following **SCOPE** options can be used: +``abbr --add`` creates a new abbreviation. With no other options, the string **NAME** is replaced by **EXPANSION**. -**-g** or **--global** - Use a global variable +With **--quiet**, the expansion occurs without printing the new command line. The original, unexpanded command is saved in history. Without **--quiet** the new command line is shown with the abbreviation expanded. If a **--quiet** abbreviation results in an incomplete command or syntax error, the command line is printed as if it were not quiet. -**-U** or **--universal** - Use a universal variable (default) +With **--position command**, the abbreviation will only expand when it is positioned as a command, not as an argument to another command. With **--position anywhere** the abbreviation may expand anywhere in the command line. The default is **command**. + +With **--regex**, the abbreviation matches using the regular expression given by **PATTERN**, instead of the literal **NAME**. The pattern is interpreted using PCRE2 syntax and must match the entire token. If multiple abbreviations match the same token, the last abbreviation added is used. + +With **--set-cursor**, the cursor is moved to the first occurrence of **SENTINEL** in the expansion. That **SENTINEL** value is erased. + +With **--trigger-on entry**, the abbreviation will expand after its word or pattern is ended, for example by typing a space. With **--trigger-on exec**, the abbreviation will expand when the enter key is pressed. These options may be combined, but are incompatible with **--quiet**. The default is both **entry** and **exec**. + +With **-f** or **--function**, **EXPANSION** is treated as the name of a fish function instead of a literal replacement. When the abbreviation matches, the function will be called with the matching token as an argument. If the function's exit status is 0 (success), the token will be replaced by the function's output; otherwise the token will be left unchanged. -See the "Internals" section for more on them. Examples --------- +######## :: - abbr -a gco git checkout + abbr --add gco git checkout Add a new abbreviation where ``gco`` will be replaced with ``git checkout``. -This abbreviation will not be automatically visible to other shells unless the same command is run in those shells (such as when executing the commands in config.fish). :: - abbr -a l less + abbr -a --position anywhere -- -C --color -Add a new abbreviation where ``l`` will be replaced with ``less`` universal to all shells. +Add a new abbreviation where ``-C`` will be replaced with ``--color``. The ``--`` allows ``-C`` to be treated as the name of the abbreviation, instead of an option. :: - abbr -r gco gch + abbr -a L --position anywhere --set-cursor ! "! | less" + +Add a new abbreviation where ``L`` will be replaced with ``| less``, placing the cursor before the pipe. -Renames an existing abbreviation from ``gco`` to ``gch``. :: - abbr -e gco + function last_history_item + echo $history[1] + end + abbr -a !! --position anywhere --function last_history_item --quiet -Erase the ``gco`` abbreviation. +This first creates a function ``last_history_item`` which outputs the last entered command. It then adds an abbreviation which replaces ``!!`` with the result of calling this function. The ``--quiet`` option causes the expansion to happen without visibly modifying the text. Taken together, this implements the ``!!`` history expansion feature of bash. :: - ssh another_host abbr -s | source + function vim_edit + echo vim $argv + end + abbr -a vim_edit_texts --position command --regex ".+\.txt" --function vim_edit -Import the abbreviations defined on another_host over SSH. +This first creates a function ``vim_edit`` which prepends ``vim`` before its argument. It then adds an abbreviation which matches commands ending in ``.txt``, and replaces the command with the result of calling this function. This allows text files to be "executed" as a command to open them in vim, similar to the "suffix alias" feature in zsh. -Internals ---------- -Each abbreviation is stored in its own global or universal variable. -The name consists of the prefix ``_fish_abbr_`` followed by the WORD after being transformed by ``string escape style=var``. -The WORD cannot contain a space but all other characters are legal. +:: + + abbr 4DIRS --trigger-on entry --set-cursor ! "$(string join \n -- 'for dir in */' 'cd $dir' '!' 'cd ..' 'end')" + +This creates an abbreviation "4DIRS" which expands to a multi-line loop "template." The template enters each directory and then leaves it. The cursor is positioned ready to enter the command to run in each directory, at the location of the ``!``, which is itself erased. + +Other subcommands +-------------------- + + +:: + + abbr [-r | --rename] OLD_NAME NEW_NAME + +Renames an abbreviation, from *OLD_NAME* to *NEW_NAME* + +:: + + abbr [-s | --show] + +Show all abbreviations in a manner suitable for import and export + +:: + + abbr [-l | --list] + +Prints the names of all abbreviation + +:: + + abbr [-e | --erase] NAME + +Erases the abbreviation with the given name + +:: + + abbr -q or --query [NAME...] + +Return 0 (true) if one of the *NAME* is an abbreviation. + +:: + + abbr -h or --help + +Displays help for the `abbr` command. -Abbreviations created with the **--universal** flag will be visible to other fish sessions, whilst **--global** will be limited to the current session. From 695cc74c8844df85733b6090e7329ecb788fe189 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 24 Sep 2022 14:20:21 -0700 Subject: [PATCH 13/20] Changelog new abbreviation features --- CHANGELOG.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6b8598b5c..210eb8b0d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -28,12 +28,24 @@ Notable improvements and fixes - It is now possible to specify multiple scopes for ``set -e`` and all of the named variables present in any of the specified scopes will be erased. This makes it possible to remove all instances of a variable in all scopes (``set -efglU foo``) in one go (:issue:`7711`). - A possible stack overflow when recursively evaluating substitutions has been fixed (:issue:`9302`). - `status current-commandline` has been added and retrieves the entirety of the currently executing commandline when called from a function during execution, allowing easier job introspection (:issue:`8905`). +- Abbrevations are more flexible: + + - They may optionally replace tokens anywhere on the command line, instead of only commands + - Matching tokens may be described using a regular expression instead of a literal word + - The replacement text may be produced by a fish function, instead of a literal word + - They may optionally only trigger after space, or after enter + - They may optionally run "quietly," without visibly modifying the command line + - They may position the cursor anywhere in the expansion, instead of at the end + + See the documentation for more. + Deprecations and removed features --------------------------------- - The difference between ``\xAB`` and ``\XAB`` has been removed. Before, ``\x`` would do the same thing as ``\X`` except that it would error if the value was larger than "7f" (127 in decimal, the highest ASCII value) (:issue:`9247`, :issue:`1352`). - The ``fish_git_prompt`` will now only turn on features if the corresponding boolean variable has been set to a true value (of "1", "yes" or "true") instead of just checking if it is defined. This allows specifically turning features *off* without having to erase variables, e.g. via universal variables. If you have defined a variable to a different value and expect it to count as true, you need to change it (:issue:`9274`). For example, ``set -g __fish_git_prompt_show_informative_status 0`` previously would have enabled informative status (because any value would have done so), now it turns it off. +- Abbreviations are no longer stored in universal variables. Existing universal abbreviations are still imported, but new abbreviations should be added to ``config.fish``. Scripting improvements ---------------------- From 5841e9f712d9a5e0f3c02649cfe580c4a186fc1e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 27 Nov 2022 11:29:16 -0800 Subject: [PATCH 14/20] Remove '--quiet' feature of abbreviations Per code review, this is too risky to introduce now. Leaving the feature in history should want want to revisit this in the future. --- CHANGELOG.rst | 1 - doc_src/cmds/abbr.rst | 14 +++---- src/abbrs.h | 7 ---- src/builtins/abbr.cpp | 25 +----------- src/fish_tests.cpp | 2 +- src/reader.cpp | 85 ++++------------------------------------- tests/checks/abbr.fish | 17 --------- tests/pexpects/abbrs.py | 27 ++++--------- 8 files changed, 22 insertions(+), 156 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 210eb8b0d..c1906f52b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -34,7 +34,6 @@ Notable improvements and fixes - Matching tokens may be described using a regular expression instead of a literal word - The replacement text may be produced by a fish function, instead of a literal word - They may optionally only trigger after space, or after enter - - They may optionally run "quietly," without visibly modifying the command line - They may position the cursor anywhere in the expansion, instead of at the end See the documentation for more. diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index 8e606c283..10abca527 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -8,7 +8,7 @@ Synopsis .. synopsis:: - abbr --add NAME [--quiet] [--position command | anywhere] [--regex PATTERN] + abbr --add NAME [--position command | anywhere] [--regex PATTERN] [--set-cursor SENTINEL] [--trigger-on entry | exec]... [-f | --function] EXPANSION abbr --erase NAME ... @@ -29,8 +29,6 @@ An abbreviation may match a literal word, or it may match a pattern given by a r Abbreviations may expand either after their word is entered, or if they are executed with the enter key. The ``--trigger-on`` option allows limiting expansion to only entering, or only executing. -By default abbreviations print the new command line after being entered. The ``--quiet`` option causes abbreviations to expand silently. - Combining these features, it is possible to create custom syntaxes, where a regular expression recognizes matching tokens, and the expansion function interprets them. See the `Examples`_ section. Abbreviations may be added to :ref:`config.fish `. Abbreviations are only expanded for typed-in commands, not in scripts. @@ -41,21 +39,19 @@ Abbreviations may be added to :ref:`config.fish `. Abbreviations .. synopsis:: - abbr [-a | --add] NAME [--quiet] [--position command | anywhere] [--regex PATTERN] + abbr [-a | --add] NAME [--position command | anywhere] [--regex PATTERN] [--set-cursor SENTINEL] [--trigger-on entry | exec]... [-f | --function] EXPANSION ``abbr --add`` creates a new abbreviation. With no other options, the string **NAME** is replaced by **EXPANSION**. -With **--quiet**, the expansion occurs without printing the new command line. The original, unexpanded command is saved in history. Without **--quiet** the new command line is shown with the abbreviation expanded. If a **--quiet** abbreviation results in an incomplete command or syntax error, the command line is printed as if it were not quiet. - With **--position command**, the abbreviation will only expand when it is positioned as a command, not as an argument to another command. With **--position anywhere** the abbreviation may expand anywhere in the command line. The default is **command**. With **--regex**, the abbreviation matches using the regular expression given by **PATTERN**, instead of the literal **NAME**. The pattern is interpreted using PCRE2 syntax and must match the entire token. If multiple abbreviations match the same token, the last abbreviation added is used. With **--set-cursor**, the cursor is moved to the first occurrence of **SENTINEL** in the expansion. That **SENTINEL** value is erased. -With **--trigger-on entry**, the abbreviation will expand after its word or pattern is ended, for example by typing a space. With **--trigger-on exec**, the abbreviation will expand when the enter key is pressed. These options may be combined, but are incompatible with **--quiet**. The default is both **entry** and **exec**. +With **--trigger-on entry**, the abbreviation will expand after its word or pattern is ended, for example by typing a space. With **--trigger-on exec**, the abbreviation will expand when the enter key is pressed. These options may be combined. The default is both **entry** and **exec**. With **-f** or **--function**, **EXPANSION** is treated as the name of a fish function instead of a literal replacement. When the abbreviation matches, the function will be called with the matching token as an argument. If the function's exit status is 0 (success), the token will be replaced by the function's output; otherwise the token will be left unchanged. @@ -87,9 +83,9 @@ Add a new abbreviation where ``L`` will be replaced with ``| less``, placing the function last_history_item echo $history[1] end - abbr -a !! --position anywhere --function last_history_item --quiet + abbr -a !! --position anywhere --function last_history_item -This first creates a function ``last_history_item`` which outputs the last entered command. It then adds an abbreviation which replaces ``!!`` with the result of calling this function. The ``--quiet`` option causes the expansion to happen without visibly modifying the text. Taken together, this implements the ``!!`` history expansion feature of bash. +This first creates a function ``last_history_item`` which outputs the last entered command. It then adds an abbreviation which replaces ``!!`` with the result of calling this function. Taken together, this is similar to the ``!!`` history expansion feature of bash. :: diff --git a/src/abbrs.h b/src/abbrs.h index 56c9576f3..b709e118e 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -27,15 +27,8 @@ enum abbrs_phase_t : uint8_t { // Expands "on enter" before submitting the command to be executed. abbrs_phase_exec = 1 << 1, - // Expands "quietly" after submitting the command. This is incompatible with the other - // phases. - abbrs_phase_quiet = 1 << 2, - // Default set of phases. abbrs_phases_default = abbrs_phase_entry | abbrs_phase_exec, - - // All phases. - abbrs_phases_all = abbrs_phase_entry | abbrs_phase_exec | abbrs_phase_quiet, }; using abbrs_phases_t = uint8_t; diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index ff308b5b4..45cca7264 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -47,8 +47,6 @@ struct abbr_options_t { wcstring_list_t args; bool validate(io_streams_t &streams) { - const bool quiet = (phases.value_or(0) & abbrs_phase_quiet); - // Duplicate options? wcstring_list_t cmds; if (add) cmds.push_back(L"add"); @@ -81,10 +79,7 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --function option requires --add\n"), CMD); return false; } - if (!add && quiet) { - streams.err.append_format(_(L"%ls: --quiet option requires --add\n"), CMD); - return false; - } else if (!add && phases.has_value()) { + if (!add && phases.has_value()) { streams.err.append_format(_(L"%ls: --trigger-on option requires --add\n"), CMD); return false; } @@ -92,18 +87,10 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --set-cursor option requires --add\n"), CMD); return false; } - if (set_cursor_indicator.has_value() && quiet) { - streams.err.append_format(_(L"%ls: --quiet cannot be used with --set-cursor\n"), CMD); - return false; - } if (set_cursor_indicator.has_value() && set_cursor_indicator->empty()) { streams.err.append_format(_(L"%ls: --set-cursor argument cannot be empty\n"), CMD); return false; } - if (quiet && (*phases & ~abbrs_phase_quiet) != 0) { - streams.err.append_format(_(L"%ls: Cannot use --quiet with --trigger-on\n"), CMD); - return false; - } return true; } @@ -136,9 +123,6 @@ static int abbr_show(const abbr_options_t &, io_streams_t &streams) { if (abbr.phases & abbrs_phase_exec) { comps.push_back(L"--trigger-on exec"); } - if (abbr.phases & abbrs_phase_quiet) { - comps.push_back(L"--quiet"); - } } if (abbr.replacement_is_function) { comps.push_back(L"--function"); @@ -308,7 +292,7 @@ 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, QUIET_SHORT }; + enum { NON_OPTION_ARGUMENT = 1, REGEX_SHORT, EXPAND_ON_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 @@ -318,7 +302,6 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t {L"position", required_argument, 'p'}, {L"regex", required_argument, REGEX_SHORT}, {L"trigger-on", required_argument, 't'}, - {L"quiet", no_argument, QUIET_SHORT}, {L"set-cursor", required_argument, 'C'}, {L"function", no_argument, 'f'}, {L"rename", no_argument, 'r'}, @@ -393,10 +376,6 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t opts.phases = phases; break; } - case QUIET_SHORT: { - opts.phases = opts.phases.value_or(0) | abbrs_phase_quiet; - break; - } case 'C': { if (opts.set_cursor_indicator.has_value()) { streams.err.append_format( diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 031ede549..18b3b3647 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, abbrs_phases_all); + auto result = abbrs_match(token, pos, abbrs_phases_default); if (result.size() > 1) { err(L"abbreviation expansion for %ls returned more than 1 result", token.c_str()); } diff --git a/src/reader.cpp b/src/reader.cpp index 1726f8efe..3e010e0ab 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -618,10 +618,6 @@ 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()}; }; @@ -886,10 +882,9 @@ class reader_data_t : public std::enable_shared_from_this { 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. + // Replace the command line with any abbreviations as needed. // \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); + parser_test_error_bits_t expand_for_execute(); void clear_pager(); void select_completion_in_direction(selection_motion_t dir, @@ -1499,44 +1494,6 @@ bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs 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_quiet); - for (const auto &replacer : replacers) { - const auto replacement = expand_replacer(orig_range, token, replacer, parser); - if (replacement && replacement->text != token) { - modified = true; - result.replace(orig_range.start + repl_lengths - orig_lengths, orig_range.length, - replacement->text); - orig_lengths += orig_range.length; - repl_lengths += replacement->text.length(); - break; - } - } - } - if (modified) { - *inout_str = std::move(result); - } - return modified; -} - void reader_reset_interrupted() { interrupted = 0; } int reader_test_and_clear_interrupted() { @@ -4309,16 +4266,9 @@ void reader_data_t::add_to_history() const { } } -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. +parser_test_error_bits_t reader_data_t::expand_for_execute() { + // Expand abbreviations at the cursor. // 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; @@ -4338,25 +4288,7 @@ parser_test_error_bits_t reader_data_t::expand_for_execute(wcstring *to_exec) { 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; + return test_res; } bool reader_data_t::handle_execute(readline_loop_state_t &rls) { @@ -4403,8 +4335,7 @@ bool reader_data_t::handle_execute(readline_loop_state_t &rls) { // 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); + parser_test_error_bits_t test_res = this->expand_for_execute(); if (test_res & PARSER_TEST_ERROR) { return false; } else if (test_res & PARSER_TEST_INCOMPLETE) { @@ -4414,7 +4345,6 @@ bool reader_data_t::handle_execute(readline_loop_state_t &rls) { 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; @@ -4508,7 +4438,6 @@ 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; } @@ -4632,7 +4561,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{std::move(rls.cmd)} : none(); + return rls.finished ? maybe_t{command_line.text()} : none(); } bool reader_data_t::jump(jump_direction_t dir, jump_precision_t precision, editable_line_t *el, diff --git a/tests/checks/abbr.fish b/tests/checks/abbr.fish index e4a94dcda..b46e56752 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -171,20 +171,3 @@ abbr --add bogus --position anywhere --position command stuff abbr --add --trigger-on derp zzxjoanw stuff # CHECKERR: abbr: Invalid --trigger-on 'derp' # CHECKERR: Must be one of: entry, exec. - -abbr --add --trigger-on entry --quiet zzxjoanw stuff -# CHECKERR: abbr: Cannot use --quiet with --trigger-on - -abbr --add --quiet --trigger-on exec zzxjoanw stuff -# CHECKERR: abbr: Cannot use --quiet with --trigger-on - -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 - -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 c9540cc39..0bc734bfa 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -102,10 +102,10 @@ 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 '('") +# are visibly expanded. +sendline(r"abbr openparen '(' --position anywhere") expect_prompt() -sendline(r"abbr closeparen --position anywhere --quiet ')'") +sendline(r"abbr closeparen ')' --position anywhere") expect_prompt() sendline(r"echo openparen") expect_str(r"echo (") @@ -115,8 +115,11 @@ sendline(r"echo closeparen") expect_str(r"echo )") send(r"?") expect_str(r"") -sendline(r"echo openparen seq 5 closeparen") +sendline(r"echo openparen seq 5 closeparen") # expands on enter expect_prompt(r"1 2 3 4 5") +sendline(r"echo openparen seq 5 closeparen ") # expands on space +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. @@ -137,22 +140,6 @@ 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") - # Test cursor positioning. sendline(r"""abbr --erase (abbr --list) """) expect_prompt() From 35a4688650eae25459b1de169adb3de94e1306b8 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 27 Nov 2022 13:04:00 -0800 Subject: [PATCH 15/20] Rename abbreviation triggers This renames abbreviation triggers from `--trigger-on entry` and `--trigger-on exec` to `--on-space` and `--on-enter`. These names are less precise, as abbreviations trigger on any character that terminates a word or any key binding that triggers exec, but they're also more human friendly and that's a better tradeoff. --- doc_src/cmds/abbr.rst | 10 +++++----- src/abbrs.cpp | 14 +++++++------- src/abbrs.h | 39 +++++++++++++++++++------------------ src/builtins/abbr.cpp | 43 ++++++++++++++++++----------------------- src/fish_tests.cpp | 4 ++-- src/highlight.cpp | 3 ++- src/reader.cpp | 21 ++++++++++---------- src/reader.h | 7 ++++--- tests/checks/abbr.fish | 4 ---- tests/pexpects/abbrs.py | 6 +++--- 10 files changed, 73 insertions(+), 78 deletions(-) diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index 10abca527..d9ad4c2db 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -9,7 +9,7 @@ Synopsis .. synopsis:: abbr --add NAME [--position command | anywhere] [--regex PATTERN] - [--set-cursor SENTINEL] [--trigger-on entry | exec]... + [--set-cursor SENTINEL] [--on-space] [--on-enter]... [-f | --function] EXPANSION abbr --erase NAME ... abbr --rename OLD_WORD NEW_WORD @@ -27,7 +27,7 @@ After entering ``gco`` and pressing :kbd:`Space` or :kbd:`Enter`, the full text An abbreviation may match a literal word, or it may match a pattern given by a regular expression. When an abbreviation matches a word, that word is replaced by new text, called its *expansion*. This expansion may be a fixed new phrase, or it can be dynamically created via a fish function. -Abbreviations may expand either after their word is entered, or if they are executed with the enter key. The ``--trigger-on`` option allows limiting expansion to only entering, or only executing. +Abbreviations by default expand either after their word is entered and the user presses space, or if they are executed with the enter key. The ``--on-space`` and ``-on-enter`` options allows limiting expansion to only space or enter, respectively. Combining these features, it is possible to create custom syntaxes, where a regular expression recognizes matching tokens, and the expansion function interprets them. See the `Examples`_ section. @@ -40,7 +40,7 @@ Abbreviations may be added to :ref:`config.fish `. Abbreviations .. synopsis:: abbr [-a | --add] NAME [--position command | anywhere] [--regex PATTERN] - [--set-cursor SENTINEL] [--trigger-on entry | exec]... + [--set-cursor SENTINEL] [--on-space] [--on-enter] [-f | --function] EXPANSION ``abbr --add`` creates a new abbreviation. With no other options, the string **NAME** is replaced by **EXPANSION**. @@ -51,7 +51,7 @@ With **--regex**, the abbreviation matches using the regular expression given by With **--set-cursor**, the cursor is moved to the first occurrence of **SENTINEL** in the expansion. That **SENTINEL** value is erased. -With **--trigger-on entry**, the abbreviation will expand after its word or pattern is ended, for example by typing a space. With **--trigger-on exec**, the abbreviation will expand when the enter key is pressed. These options may be combined. The default is both **entry** and **exec**. +With **--on-space**, the abbreviation will expand after its word or pattern is entered and the user presses space (or another word-boundary character such as semicolon). With **--on-enter**, the abbreviation will expand when the enter key is pressed to execute it. These options may be combined. The default is both **space** and **enter**. With **-f** or **--function**, **EXPANSION** is treated as the name of a fish function instead of a literal replacement. When the abbreviation matches, the function will be called with the matching token as an argument. If the function's exit status is 0 (success), the token will be replaced by the function's output; otherwise the token will be left unchanged. @@ -98,7 +98,7 @@ This first creates a function ``vim_edit`` which prepends ``vim`` before its arg :: - abbr 4DIRS --trigger-on entry --set-cursor ! "$(string join \n -- 'for dir in */' 'cd $dir' '!' 'cd ..' 'end')" + abbr 4DIRS --on-space --set-cursor ! "$(string join \n -- 'for dir in */' 'cd $dir' '!' 'cd ..' 'end')" This creates an abbreviation "4DIRS" which expands to a multi-line loop "template." The template enters each directory and then leaves it. The cursor is positioned ready to enter the command to run in each directory, at the location of the ``!``, which is itself erased. diff --git a/src/abbrs.cpp b/src/abbrs.cpp index 31f377587..3e056ee66 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -18,11 +18,11 @@ bool abbreviation_t::matches_position(abbrs_position_t position) const { return this->position == abbrs_position_t::anywhere || this->position == position; } -bool abbreviation_t::matches_phases(abbrs_phases_t p) const { return bool(this->phases & p); } +bool abbreviation_t::triggers_on(abbrs_triggers_t t) const { return bool(this->triggers & t); } bool abbreviation_t::matches(const wcstring &token, abbrs_position_t position, - abbrs_phases_t phases) const { - if (!this->matches_position(position) || !this->matches_phases(phases)) { + abbrs_triggers_t trigger) const { + if (!this->matches_position(position) || !this->triggers_on(trigger)) { return false; } if (this->is_regex()) { @@ -38,12 +38,12 @@ acquired_lock abbrs_get_set() { } abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t position, - abbrs_phases_t phases) const { + abbrs_triggers_t trigger) 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, phases)) { + if (abbr.matches(token, position, trigger)) { result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function, abbr.set_cursor_indicator}); } @@ -52,9 +52,9 @@ abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t } bool abbrs_set_t::has_match(const wcstring &token, abbrs_position_t position, - abbrs_phases_t phases) const { + abbrs_triggers_t trigger) const { for (const auto &abbr : abbrs_) { - if (abbr.matches(token, position, phases)) { + if (abbr.matches(token, position, trigger)) { return true; } } diff --git a/src/abbrs.h b/src/abbrs.h index b709e118e..e3a8a38bc 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -19,18 +19,18 @@ enum class abbrs_position_t : uint8_t { anywhere, // expand in any token }; -/// Describes a phase of expansion. -enum abbrs_phase_t : uint8_t { - // Expands "on space" immediately after the user types it. - abbrs_phase_entry = 1 << 0, +/// Describes the reason for triggering expansion. +enum abbrs_trigger_on_t : uint8_t { + // Expands on "space" (any token-closing character) immediately after the user types it. + abbrs_trigger_on_space = 1 << 0, - // Expands "on enter" before submitting the command to be executed. - abbrs_phase_exec = 1 << 1, + // Expands on "enter" (any exec key binding) before submitting the command to be executed. + abbrs_trigger_on_enter = 1 << 1, - // Default set of phases. - abbrs_phases_default = abbrs_phase_entry | abbrs_phase_exec, + // Default set of triggers. + abbrs_trigger_on_default = abbrs_trigger_on_space | abbrs_trigger_on_enter, }; -using abbrs_phases_t = uint8_t; +using abbrs_triggers_t = uint8_t; struct abbreviation_t { // Abbreviation name. This is unique within the abbreviation set. @@ -61,14 +61,14 @@ struct abbreviation_t { /// Mark if we came from a universal variable. bool from_universal{}; - /// Set of phases in which this abbreviation expands. - abbrs_phases_t phases{abbrs_phases_default}; + /// Set of conditions in which this abbreviation expands. + abbrs_triggers_t triggers{abbrs_trigger_on_default}; // \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 in a given set of phases. - bool matches(const wcstring &token, abbrs_position_t position, abbrs_phases_t phases) const; + // \return true if we match a token at a given position and trigger. + bool matches(const wcstring &token, abbrs_position_t position, abbrs_triggers_t trigger) 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. @@ -82,8 +82,8 @@ struct abbreviation_t { // \return if we expand in a given position. bool matches_position(abbrs_position_t position) const; - // \return if we expand in a given set of phases. - bool matches_phases(abbrs_phases_t p) const; + // \return if we trigger in this phase. + bool triggers_on(abbrs_triggers_t t) const; }; /// The result of an abbreviation expansion. @@ -123,10 +123,11 @@ class abbrs_set_t { /// \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, - abbrs_phases_t phases) const; + abbrs_triggers_t trigger) const; /// \return whether we would have at least one replacer for a given token. - bool has_match(const wcstring &token, abbrs_position_t position, abbrs_phases_t phases) const; + bool has_match(const wcstring &token, abbrs_position_t position, + abbrs_triggers_t trigger) const; /// Add an abbreviation. Any abbreviation with the same name is replaced. void add(abbreviation_t &&abbr); @@ -163,8 +164,8 @@ 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, - abbrs_phases_t phases) { - return abbrs_get_set()->match(token, position, phases); + abbrs_triggers_t trigger) { + return abbrs_get_set()->match(token, position, trigger); } #endif diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 45cca7264..8d1ac5678 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -41,7 +41,7 @@ struct abbr_options_t { bool function{}; maybe_t regex_pattern; maybe_t position{}; - maybe_t phases{}; + maybe_t triggers{}; maybe_t set_cursor_indicator{}; wcstring_list_t args; @@ -79,8 +79,9 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --function option requires --add\n"), CMD); return false; } - if (!add && phases.has_value()) { - streams.err.append_format(_(L"%ls: --trigger-on option requires --add\n"), CMD); + if (!add && triggers.has_value()) { + streams.err.append_format(_(L"%ls: --on-space and --on-enter options require --add\n"), + CMD); return false; } if (!add && set_cursor_indicator.has_value()) { @@ -116,12 +117,12 @@ static int abbr_show(const abbr_options_t &, io_streams_t &streams) { comps.push_back(L"--set-cursor"); comps.push_back(escape_string(*abbr.set_cursor_indicator)); } - if (abbr.phases != abbrs_phases_default) { - if (abbr.phases & abbrs_phase_entry) { - comps.push_back(L"--trigger-on entry"); + if (abbr.triggers != abbrs_trigger_on_default) { + if (abbr.triggers & abbrs_trigger_on_space) { + comps.push_back(L"--on-space"); } - if (abbr.phases & abbrs_phase_exec) { - comps.push_back(L"--trigger-on exec"); + if (abbr.triggers & abbrs_trigger_on_enter) { + comps.push_back(L"--on-enter"); } } if (abbr.replacement_is_function) { @@ -263,7 +264,7 @@ static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { abbr.regex = std::move(regex); abbr.replacement_is_function = opts.function; abbr.set_cursor_indicator = opts.set_cursor_indicator; - abbr.phases = opts.phases.value_or(abbrs_phases_default); + abbr.triggers = opts.triggers.value_or(abbrs_trigger_on_default); abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -292,7 +293,7 @@ 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, ON_SPACE_SHORT, ON_ENTER_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 @@ -301,7 +302,8 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t static const struct woption long_options[] = {{L"add", no_argument, 'a'}, {L"position", required_argument, 'p'}, {L"regex", required_argument, REGEX_SHORT}, - {L"trigger-on", required_argument, 't'}, + {L"on-space", no_argument, ON_SPACE_SHORT}, + {L"on-enter", no_argument, ON_ENTER_SHORT}, {L"set-cursor", required_argument, 'C'}, {L"function", no_argument, 'f'}, {L"rename", no_argument, 'r'}, @@ -361,19 +363,12 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t opts.regex_pattern = w.woptarg; break; } - case 't': { - abbrs_phases_t phases = opts.phases.value_or(0); - if (!wcscmp(w.woptarg, L"entry")) { - phases |= abbrs_phase_entry; - } else if (!wcscmp(w.woptarg, L"exec")) { - phases |= abbrs_phase_exec; - } else { - streams.err.append_format(_(L"%ls: Invalid --trigger-on '%ls'\n" - L"Must be one of: entry, exec.\n"), - CMD, w.woptarg); - return STATUS_INVALID_ARGS; - } - opts.phases = phases; + case ON_SPACE_SHORT: { + opts.triggers = opts.triggers.value_or(0) | abbrs_trigger_on_space; + break; + } + case ON_ENTER_SHORT: { + opts.triggers = opts.triggers.value_or(0) | abbrs_trigger_on_enter; break; } case 'C': { diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 18b3b3647..47c742bc4 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, abbrs_phases_default); + auto result = abbrs_match(token, pos, abbrs_trigger_on_default); if (result.size() > 1) { err(L"abbreviation expansion for %ls returned more than 1 result", token.c_str()); } @@ -2507,7 +2507,7 @@ static void test_abbreviations() { auto expand_abbreviation_in_command = [](const wcstring &cmdline, size_t cursor_pos) -> maybe_t { if (auto replacement = reader_expand_abbreviation_at_cursor( - cmdline, cursor_pos, abbrs_phases_default, parser_t::principal_parser())) { + cmdline, cursor_pos, abbrs_trigger_on_default, parser_t::principal_parser())) { wcstring cmdline_expanded = cmdline; std::vector colors{cmdline_expanded.size()}; apply_edit(&cmdline_expanded, &colors, edit_t{replacement->range, replacement->text}); diff --git a/src/highlight.cpp b/src/highlight.cpp index a7885bfee..a38e4f554 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -1336,7 +1336,8 @@ 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, abbrs_phases_default); + is_valid = + abbrs_get_set()->has_match(cmd, abbrs_position_t::command, abbrs_trigger_on_default); // 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 3e010e0ab..5cbbfd608 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -788,9 +788,9 @@ class reader_data_t : public std::enable_shared_from_this { /// Do what we need to do whenever our pager selection changes. void pager_selection_changed(); - /// Expand abbreviations in the given phases at the current cursor position, minus + /// Expand abbreviations in the given triggers at the current cursor position, minus /// cursor_backtrack. - bool expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_phases_t phases); + bool expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_triggers_t triggers); /// \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. @@ -1445,11 +1445,11 @@ static std::vector extract_tokens(const wcstring &str) { return result; } -/// Expand abbreviations in the given phase at the given cursor position. +/// Expand abbreviations in the given triggers at the given cursor position. /// cursor. \return the replacement. This does NOT inspect the current reader data. maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - abbrs_phases_t phases, + abbrs_triggers_t triggers, parser_t &parser) { // Find the token containing the cursor. Usually users edit from the end, so walk backwards. const auto tokens = extract_tokens(cmdline); @@ -1464,7 +1464,7 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring iter->is_cmd ? abbrs_position_t::command : abbrs_position_t::anywhere; wcstring token_str = cmdline.substr(range.start, range.length); - auto replacers = abbrs_match(token_str, position, phases); + auto replacers = abbrs_match(token_str, position, triggers); for (const auto &replacer : replacers) { if (auto replacement = expand_replacer(range, token_str, replacer, parser)) { return replacement; @@ -1476,7 +1476,8 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring /// 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_at_cursor(size_t cursor_backtrack, abbrs_phases_t phases) { +bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack, + abbrs_triggers_t triggers) { bool result = false; editable_line_t *el = active_edit_line(); @@ -1484,8 +1485,8 @@ bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs // Try expanding abbreviations. this->update_commandline_state(); size_t cursor_pos = el->position() - std::min(el->position(), cursor_backtrack); - if (auto replacement = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, phases, - this->parser())) { + if (auto replacement = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, + triggers, this->parser())) { push_edit(el, edit_t{replacement->range, std::move(replacement->text)}); update_buff_pos(el, replacement->cursor); result = true; @@ -4188,7 +4189,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } case rl::expand_abbr: { - if (expand_abbreviation_at_cursor(1, abbrs_phase_entry)) { + if (expand_abbreviation_at_cursor(1, abbrs_trigger_on_space)) { inputter.function_set_status(true); } else { inputter.function_set_status(false); @@ -4281,7 +4282,7 @@ parser_test_error_bits_t reader_data_t::expand_for_execute() { // Exec abbreviations at the cursor. // Note we want to expand abbreviations even if incomplete. - if (expand_abbreviation_at_cursor(0, abbrs_phase_exec)) { + if (expand_abbreviation_at_cursor(0, abbrs_trigger_on_enter)) { // Trigger syntax highlighting as we are likely about to execute this command. this->super_highlight_me_plenty(); if (conf.syntax_check_ok) { diff --git a/src/reader.h b/src/reader.h index e1c707bf4..c27f1364d 100644 --- a/src/reader.h +++ b/src/reader.h @@ -266,12 +266,13 @@ wcstring combine_command_and_autosuggestion(const wcstring &cmdline, /// Expand at most one abbreviation at the given cursor position, updating the position if the /// abbreviation wants to move the cursor. Use the parser to run any abbreviations which want -/// function calls. \return none if no abbreviations were expanded, otherwise the resulting edit. -using abbrs_phases_t = uint8_t; +/// function calls. \return none if no abbreviations were expanded, otherwise the resulting +/// replacement. +using abbrs_triggers_t = uint8_t; struct abbrs_replacement_t; maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - abbrs_phases_t phases, + abbrs_triggers_t triggers, parser_t &parser); /// Apply a completion string. Exposed for testing only. diff --git a/tests/checks/abbr.fish b/tests/checks/abbr.fish index b46e56752..e539cb4db 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -167,7 +167,3 @@ abbr --add bogus --position never stuff abbr --add bogus --position anywhere --position command stuff # CHECKERR: abbr: Cannot specify multiple positions - -abbr --add --trigger-on derp zzxjoanw stuff -# CHECKERR: abbr: Invalid --trigger-on 'derp' -# CHECKERR: Must be one of: entry, exec. diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py index 0bc734bfa..320abcc27 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -148,12 +148,12 @@ expect_prompt() send(r"""echo LLL derp?""") expect_str(r"echo derp | less ") -# Test trigger-on. +# Test --on-enter and --on-space. sendline(r"""abbr --erase (abbr --list) """) expect_prompt() -sendline(r"""abbr --add entry-only --position anywhere --trigger-on entry 'worked1'""") +sendline(r"""abbr --add entry-only --position anywhere --on-space 'worked1'""") expect_prompt() -sendline(r"""abbr --add exec-only --position anywhere --trigger-on exec 'worked2'""") +sendline(r"""abbr --add exec-only --position anywhere --on-enter 'worked2'""") expect_prompt() sendline(r"echo entry-only") # should not trigger, no space expect_prompt(r"entry-only") From 01039537b09b10f3b06467863981a87609fd41ec Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 10 Dec 2022 12:45:21 -0800 Subject: [PATCH 16/20] Remove abbreviation triggers Per code review, this does not add enough value to introduce now. Leaving the feature in history should want want to revisit this in the future. --- CHANGELOG.rst | 1 - doc_src/cmds/abbr.rst | 12 ++++-------- src/abbrs.cpp | 17 ++++++----------- src/abbrs.h | 34 ++++++---------------------------- src/builtins/abbr.cpp | 27 +-------------------------- src/fish_tests.cpp | 6 +++--- src/highlight.cpp | 3 +-- src/reader.cpp | 23 ++++++++++------------- src/reader.h | 2 -- tests/pexpects/abbrs.py | 17 ----------------- 10 files changed, 31 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c1906f52b..9bc5ff1c2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -33,7 +33,6 @@ Notable improvements and fixes - They may optionally replace tokens anywhere on the command line, instead of only commands - Matching tokens may be described using a regular expression instead of a literal word - The replacement text may be produced by a fish function, instead of a literal word - - They may optionally only trigger after space, or after enter - They may position the cursor anywhere in the expansion, instead of at the end See the documentation for more. diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index d9ad4c2db..ba5a6293b 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -9,7 +9,7 @@ Synopsis .. synopsis:: abbr --add NAME [--position command | anywhere] [--regex PATTERN] - [--set-cursor SENTINEL] [--on-space] [--on-enter]... + [--set-cursor SENTINEL] [-f | --function] EXPANSION abbr --erase NAME ... abbr --rename OLD_WORD NEW_WORD @@ -25,9 +25,7 @@ Description For example, a frequently-run command like ``git checkout`` can be abbreviated to ``gco``. After entering ``gco`` and pressing :kbd:`Space` or :kbd:`Enter`, the full text ``git checkout`` will appear in the command line. -An abbreviation may match a literal word, or it may match a pattern given by a regular expression. When an abbreviation matches a word, that word is replaced by new text, called its *expansion*. This expansion may be a fixed new phrase, or it can be dynamically created via a fish function. - -Abbreviations by default expand either after their word is entered and the user presses space, or if they are executed with the enter key. The ``--on-space`` and ``-on-enter`` options allows limiting expansion to only space or enter, respectively. +An abbreviation may match a literal word, or it may match a pattern given by a regular expression. When an abbreviation matches a word, that word is replaced by new text, called its *expansion*. This expansion may be a fixed new phrase, or it can be dynamically created via a fish function. This expansion occurs after pressing space or enter. Combining these features, it is possible to create custom syntaxes, where a regular expression recognizes matching tokens, and the expansion function interprets them. See the `Examples`_ section. @@ -40,7 +38,7 @@ Abbreviations may be added to :ref:`config.fish `. Abbreviations .. synopsis:: abbr [-a | --add] NAME [--position command | anywhere] [--regex PATTERN] - [--set-cursor SENTINEL] [--on-space] [--on-enter] + [--set-cursor SENTINEL] [-f | --function] EXPANSION ``abbr --add`` creates a new abbreviation. With no other options, the string **NAME** is replaced by **EXPANSION**. @@ -51,8 +49,6 @@ With **--regex**, the abbreviation matches using the regular expression given by With **--set-cursor**, the cursor is moved to the first occurrence of **SENTINEL** in the expansion. That **SENTINEL** value is erased. -With **--on-space**, the abbreviation will expand after its word or pattern is entered and the user presses space (or another word-boundary character such as semicolon). With **--on-enter**, the abbreviation will expand when the enter key is pressed to execute it. These options may be combined. The default is both **space** and **enter**. - With **-f** or **--function**, **EXPANSION** is treated as the name of a fish function instead of a literal replacement. When the abbreviation matches, the function will be called with the matching token as an argument. If the function's exit status is 0 (success), the token will be replaced by the function's output; otherwise the token will be left unchanged. @@ -98,7 +94,7 @@ This first creates a function ``vim_edit`` which prepends ``vim`` before its arg :: - abbr 4DIRS --on-space --set-cursor ! "$(string join \n -- 'for dir in */' 'cd $dir' '!' 'cd ..' 'end')" + abbr 4DIRS --set-cursor ! "$(string join \n -- 'for dir in */' 'cd $dir' '!' 'cd ..' 'end')" This creates an abbreviation "4DIRS" which expands to a multi-line loop "template." The template enters each directory and then leaves it. The cursor is positioned ready to enter the command to run in each directory, at the location of the ``!``, which is itself erased. diff --git a/src/abbrs.cpp b/src/abbrs.cpp index 3e056ee66..4199e4bb3 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -18,11 +18,8 @@ bool abbreviation_t::matches_position(abbrs_position_t position) const { return this->position == abbrs_position_t::anywhere || this->position == position; } -bool abbreviation_t::triggers_on(abbrs_triggers_t t) const { return bool(this->triggers & t); } - -bool abbreviation_t::matches(const wcstring &token, abbrs_position_t position, - abbrs_triggers_t trigger) const { - if (!this->matches_position(position) || !this->triggers_on(trigger)) { +bool abbreviation_t::matches(const wcstring &token, abbrs_position_t position) const { + if (!this->matches_position(position)) { return false; } if (this->is_regex()) { @@ -37,13 +34,12 @@ acquired_lock abbrs_get_set() { return abbrs.acquire(); } -abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t position, - abbrs_triggers_t trigger) const { +abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t position) 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, trigger)) { + if (abbr.matches(token, position)) { result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function, abbr.set_cursor_indicator}); } @@ -51,10 +47,9 @@ abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t return result; } -bool abbrs_set_t::has_match(const wcstring &token, abbrs_position_t position, - abbrs_triggers_t trigger) const { +bool abbrs_set_t::has_match(const wcstring &token, abbrs_position_t position) const { for (const auto &abbr : abbrs_) { - if (abbr.matches(token, position, trigger)) { + if (abbr.matches(token, position)) { return true; } } diff --git a/src/abbrs.h b/src/abbrs.h index e3a8a38bc..4d4012f4b 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -19,19 +19,6 @@ enum class abbrs_position_t : uint8_t { anywhere, // expand in any token }; -/// Describes the reason for triggering expansion. -enum abbrs_trigger_on_t : uint8_t { - // Expands on "space" (any token-closing character) immediately after the user types it. - abbrs_trigger_on_space = 1 << 0, - - // Expands on "enter" (any exec key binding) before submitting the command to be executed. - abbrs_trigger_on_enter = 1 << 1, - - // Default set of triggers. - abbrs_trigger_on_default = abbrs_trigger_on_space | abbrs_trigger_on_enter, -}; -using abbrs_triggers_t = uint8_t; - 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. @@ -61,14 +48,11 @@ struct abbreviation_t { /// Mark if we came from a universal variable. bool from_universal{}; - /// Set of conditions in which this abbreviation expands. - abbrs_triggers_t triggers{abbrs_trigger_on_default}; - // \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 and trigger. - bool matches(const wcstring &token, abbrs_position_t position, abbrs_triggers_t trigger) const; + // \return true if we match a token at a given position. + bool matches(const wcstring &token, abbrs_position_t position) 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. @@ -81,9 +65,6 @@ struct abbreviation_t { private: // \return if we expand in a given position. bool matches_position(abbrs_position_t position) const; - - // \return if we trigger in this phase. - bool triggers_on(abbrs_triggers_t t) const; }; /// The result of an abbreviation expansion. @@ -122,12 +103,10 @@ 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, - abbrs_triggers_t trigger) const; + abbrs_replacer_list_t match(const wcstring &token, abbrs_position_t position) const; /// \return whether we would have at least one replacer for a given token. - bool has_match(const wcstring &token, abbrs_position_t position, - abbrs_triggers_t trigger) const; + bool has_match(const wcstring &token, abbrs_position_t position) const; /// Add an abbreviation. Any abbreviation with the same name is replaced. void add(abbreviation_t &&abbr); @@ -163,9 +142,8 @@ 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, - abbrs_triggers_t trigger) { - return abbrs_get_set()->match(token, position, trigger); +inline abbrs_replacer_list_t abbrs_match(const wcstring &token, abbrs_position_t position) { + return abbrs_get_set()->match(token, position); } #endif diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 8d1ac5678..649b8709a 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -41,7 +41,6 @@ struct abbr_options_t { bool function{}; maybe_t regex_pattern; maybe_t position{}; - maybe_t triggers{}; maybe_t set_cursor_indicator{}; wcstring_list_t args; @@ -79,11 +78,6 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --function option requires --add\n"), CMD); return false; } - if (!add && triggers.has_value()) { - streams.err.append_format(_(L"%ls: --on-space and --on-enter options require --add\n"), - CMD); - return false; - } if (!add && set_cursor_indicator.has_value()) { streams.err.append_format(_(L"%ls: --set-cursor option requires --add\n"), CMD); return false; @@ -117,14 +111,6 @@ static int abbr_show(const abbr_options_t &, io_streams_t &streams) { comps.push_back(L"--set-cursor"); comps.push_back(escape_string(*abbr.set_cursor_indicator)); } - if (abbr.triggers != abbrs_trigger_on_default) { - if (abbr.triggers & abbrs_trigger_on_space) { - comps.push_back(L"--on-space"); - } - if (abbr.triggers & abbrs_trigger_on_enter) { - comps.push_back(L"--on-enter"); - } - } if (abbr.replacement_is_function) { comps.push_back(L"--function"); } @@ -264,7 +250,6 @@ static int abbr_add(const abbr_options_t &opts, io_streams_t &streams) { abbr.regex = std::move(regex); abbr.replacement_is_function = opts.function; abbr.set_cursor_indicator = opts.set_cursor_indicator; - abbr.triggers = opts.triggers.value_or(abbrs_trigger_on_default); abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -293,7 +278,7 @@ 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, ON_SPACE_SHORT, ON_ENTER_SHORT }; + enum { NON_OPTION_ARGUMENT = 1, REGEX_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 @@ -302,8 +287,6 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t static const struct woption long_options[] = {{L"add", no_argument, 'a'}, {L"position", required_argument, 'p'}, {L"regex", required_argument, REGEX_SHORT}, - {L"on-space", no_argument, ON_SPACE_SHORT}, - {L"on-enter", no_argument, ON_ENTER_SHORT}, {L"set-cursor", required_argument, 'C'}, {L"function", no_argument, 'f'}, {L"rename", no_argument, 'r'}, @@ -363,14 +346,6 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t opts.regex_pattern = w.woptarg; break; } - case ON_SPACE_SHORT: { - opts.triggers = opts.triggers.value_or(0) | abbrs_trigger_on_space; - break; - } - case ON_ENTER_SHORT: { - opts.triggers = opts.triggers.value_or(0) | abbrs_trigger_on_enter; - break; - } case 'C': { if (opts.set_cursor_indicator.has_value()) { streams.err.append_format( diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 47c742bc4..08c6eaa3c 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, abbrs_trigger_on_default); + auto result = abbrs_match(token, pos); 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 replacement = reader_expand_abbreviation_at_cursor( - cmdline, cursor_pos, abbrs_trigger_on_default, parser_t::principal_parser())) { + if (auto replacement = reader_expand_abbreviation_at_cursor(cmdline, cursor_pos, + parser_t::principal_parser())) { wcstring cmdline_expanded = cmdline; std::vector colors{cmdline_expanded.size()}; apply_edit(&cmdline_expanded, &colors, edit_t{replacement->range, replacement->text}); diff --git a/src/highlight.cpp b/src/highlight.cpp index a38e4f554..ce0f29977 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -1336,8 +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, abbrs_trigger_on_default); + is_valid = abbrs_get_set()->has_match(cmd, abbrs_position_t::command); // 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 5cbbfd608..36b5aa452 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -788,9 +788,8 @@ class reader_data_t : public std::enable_shared_from_this { /// Do what we need to do whenever our pager selection changes. void pager_selection_changed(); - /// Expand abbreviations in the given triggers at the current cursor position, minus - /// cursor_backtrack. - bool expand_abbreviation_at_cursor(size_t cursor_backtrack, abbrs_triggers_t triggers); + /// Expand abbreviations at the current cursor position, minus cursor_backtrack. + bool expand_abbreviation_at_cursor(size_t cursor_backtrack); /// \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. @@ -1445,11 +1444,10 @@ static std::vector extract_tokens(const wcstring &str) { return result; } -/// Expand abbreviations in the given triggers at the given cursor position. -/// cursor. \return the replacement. This does NOT inspect the current reader data. +/// Expand abbreviations at the given cursor position. +/// \return the replacement. This does NOT inspect the current reader data. maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - abbrs_triggers_t triggers, parser_t &parser) { // Find the token containing the cursor. Usually users edit from the end, so walk backwards. const auto tokens = extract_tokens(cmdline); @@ -1464,7 +1462,7 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring iter->is_cmd ? abbrs_position_t::command : abbrs_position_t::anywhere; wcstring token_str = cmdline.substr(range.start, range.length); - auto replacers = abbrs_match(token_str, position, triggers); + auto replacers = abbrs_match(token_str, position); for (const auto &replacer : replacers) { if (auto replacement = expand_replacer(range, token_str, replacer, parser)) { return replacement; @@ -1476,8 +1474,7 @@ maybe_t reader_expand_abbreviation_at_cursor(const wcstring /// 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_at_cursor(size_t cursor_backtrack, - abbrs_triggers_t triggers) { +bool reader_data_t::expand_abbreviation_at_cursor(size_t cursor_backtrack) { bool result = false; editable_line_t *el = active_edit_line(); @@ -1485,8 +1482,8 @@ bool reader_data_t::expand_abbreviation_at_cursor(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 replacement = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, - triggers, this->parser())) { + if (auto replacement = + reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, this->parser())) { push_edit(el, edit_t{replacement->range, std::move(replacement->text)}); update_buff_pos(el, replacement->cursor); result = true; @@ -4189,7 +4186,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } case rl::expand_abbr: { - if (expand_abbreviation_at_cursor(1, abbrs_trigger_on_space)) { + if (expand_abbreviation_at_cursor(1)) { inputter.function_set_status(true); } else { inputter.function_set_status(false); @@ -4282,7 +4279,7 @@ parser_test_error_bits_t reader_data_t::expand_for_execute() { // Exec abbreviations at the cursor. // Note we want to expand abbreviations even if incomplete. - if (expand_abbreviation_at_cursor(0, abbrs_trigger_on_enter)) { + if (expand_abbreviation_at_cursor(0)) { // Trigger syntax highlighting as we are likely about to execute this command. this->super_highlight_me_plenty(); if (conf.syntax_check_ok) { diff --git a/src/reader.h b/src/reader.h index c27f1364d..d6a4b35cd 100644 --- a/src/reader.h +++ b/src/reader.h @@ -268,11 +268,9 @@ wcstring combine_command_and_autosuggestion(const wcstring &cmdline, /// abbreviation wants to move the cursor. Use the parser to run any abbreviations which want /// function calls. \return none if no abbreviations were expanded, otherwise the resulting /// replacement. -using abbrs_triggers_t = uint8_t; struct abbrs_replacement_t; maybe_t reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos, - abbrs_triggers_t triggers, parser_t &parser); /// Apply a completion string. Exposed for testing only. diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py index 320abcc27..370c1cd4d 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -147,20 +147,3 @@ sendline(r"""abbr LLL --position anywhere --set-cursor !HERE! '!HERE! | less'""" expect_prompt() send(r"""echo LLL derp?""") expect_str(r"echo derp | less ") - -# Test --on-enter and --on-space. -sendline(r"""abbr --erase (abbr --list) """) -expect_prompt() -sendline(r"""abbr --add entry-only --position anywhere --on-space 'worked1'""") -expect_prompt() -sendline(r"""abbr --add exec-only --position anywhere --on-enter 'worked2'""") -expect_prompt() -sendline(r"echo entry-only") # should not trigger, no space -expect_prompt(r"entry-only") -sendline(r"echo entry-only ") # should trigger, there is a space -expect_prompt(r"worked1") - -sendline(r"echo exec-only") # should trigger, no space -expect_prompt(r"worked2") -sendline(r"echo exec-only ") # should not trigger, there is a space -expect_prompt(r"exec-only") From e08f4db1f93a28f8ab4fd57c9d3da8579374beeb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 27 Nov 2022 14:33:35 -0800 Subject: [PATCH 17/20] Rename abbreviation cursor "sentinel" to "marker" Also default the marker to '%'. So you may write: abbr -a L --position anywhere --set-cursor "% | less" or set an explicit marker: abbr -a L --position anywhere --set-cursor=! "! | less" --- doc_src/cmds/abbr.rst | 8 +++---- src/abbrs.cpp | 8 +++---- src/abbrs.h | 4 ++-- src/builtins/abbr.cpp | 51 +++++++++++++++++++++-------------------- tests/pexpects/abbrs.py | 11 +++++++-- 5 files changed, 45 insertions(+), 37 deletions(-) diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index ba5a6293b..00a13e672 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -9,7 +9,7 @@ Synopsis .. synopsis:: abbr --add NAME [--position command | anywhere] [--regex PATTERN] - [--set-cursor SENTINEL] + [--set-cursor[=MARKER]] [-f | --function] EXPANSION abbr --erase NAME ... abbr --rename OLD_WORD NEW_WORD @@ -38,7 +38,7 @@ Abbreviations may be added to :ref:`config.fish `. Abbreviations .. synopsis:: abbr [-a | --add] NAME [--position command | anywhere] [--regex PATTERN] - [--set-cursor SENTINEL] + [--set-cursor[=MARKER]] [-f | --function] EXPANSION ``abbr --add`` creates a new abbreviation. With no other options, the string **NAME** is replaced by **EXPANSION**. @@ -47,7 +47,7 @@ With **--position command**, the abbreviation will only expand when it is positi With **--regex**, the abbreviation matches using the regular expression given by **PATTERN**, instead of the literal **NAME**. The pattern is interpreted using PCRE2 syntax and must match the entire token. If multiple abbreviations match the same token, the last abbreviation added is used. -With **--set-cursor**, the cursor is moved to the first occurrence of **SENTINEL** in the expansion. That **SENTINEL** value is erased. +With **--set-cursor=MARKER**, the cursor is moved to the first occurrence of **MARKER** in the expansion. The **MARKER** value is erased. The **MARKER** may be omitted (i.e. simply ``--set-cursor``), in which case it defaults to ``%``. With **-f** or **--function**, **EXPANSION** is treated as the name of a fish function instead of a literal replacement. When the abbreviation matches, the function will be called with the matching token as an argument. If the function's exit status is 0 (success), the token will be replaced by the function's output; otherwise the token will be left unchanged. @@ -69,7 +69,7 @@ Add a new abbreviation where ``-C`` will be replaced with ``--color``. The ``--` :: - abbr -a L --position anywhere --set-cursor ! "! | less" + abbr -a L --position anywhere --set-cursor "% | less" Add a new abbreviation where ``L`` will be replaced with ``| less``, placing the cursor before the pipe. diff --git a/src/abbrs.cpp b/src/abbrs.cpp index 4199e4bb3..a7b31c323 100644 --- a/src/abbrs.cpp +++ b/src/abbrs.cpp @@ -41,7 +41,7 @@ abbrs_replacer_list_t abbrs_set_t::match(const wcstring &token, abbrs_position_t const abbreviation_t &abbr = *it; if (abbr.matches(token, position)) { result.push_back(abbrs_replacer_t{abbr.replacement, abbr.replacement_is_function, - abbr.set_cursor_indicator}); + abbr.set_cursor_marker}); } } return result; @@ -123,10 +123,10 @@ abbrs_replacement_t abbrs_replacement_t::from(source_range_t range, wcstring tex abbrs_replacement_t result{}; result.range = range; result.text = std::move(text); - if (replacer.set_cursor_indicator.has_value()) { - size_t pos = result.text.find(*replacer.set_cursor_indicator); + if (replacer.set_cursor_marker.has_value()) { + size_t pos = result.text.find(*replacer.set_cursor_marker); if (pos != wcstring::npos) { - result.text.erase(pos, replacer.set_cursor_indicator->size()); + result.text.erase(pos, replacer.set_cursor_marker->size()); result.cursor = pos + range.start; } } diff --git a/src/abbrs.h b/src/abbrs.h index 4d4012f4b..f257eb511 100644 --- a/src/abbrs.h +++ b/src/abbrs.h @@ -43,7 +43,7 @@ struct abbreviation_t { abbrs_position_t position{abbrs_position_t::command}; /// If set, then move the cursor to the first instance of this string in the expansion. - maybe_t set_cursor_indicator{}; + maybe_t set_cursor_marker{}; /// Mark if we came from a universal variable. bool from_universal{}; @@ -76,7 +76,7 @@ struct abbrs_replacer_t { bool is_function; /// If set, the cursor should be moved to the first instance of this string in the expansion. - maybe_t set_cursor_indicator; + maybe_t set_cursor_marker; }; using abbrs_replacer_list_t = std::vector; diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index 649b8709a..f28bdcbdf 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -41,7 +41,7 @@ struct abbr_options_t { bool function{}; maybe_t regex_pattern; maybe_t position{}; - maybe_t set_cursor_indicator{}; + maybe_t set_cursor_marker{}; wcstring_list_t args; @@ -78,11 +78,11 @@ struct abbr_options_t { streams.err.append_format(_(L"%ls: --function option requires --add\n"), CMD); return false; } - if (!add && set_cursor_indicator.has_value()) { + if (!add && set_cursor_marker.has_value()) { streams.err.append_format(_(L"%ls: --set-cursor option requires --add\n"), CMD); return false; } - if (set_cursor_indicator.has_value() && set_cursor_indicator->empty()) { + if (set_cursor_marker.has_value() && set_cursor_marker->empty()) { streams.err.append_format(_(L"%ls: --set-cursor argument cannot be empty\n"), CMD); return false; } @@ -107,9 +107,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)); } - if (abbr.set_cursor_indicator.has_value()) { - comps.push_back(L"--set-cursor"); - comps.push_back(escape_string(*abbr.set_cursor_indicator)); + if (abbr.set_cursor_marker.has_value()) { + comps.push_back(L"--set-cursor=" + escape_string(*abbr.set_cursor_marker)); } if (abbr.replacement_is_function) { comps.push_back(L"--function"); @@ -249,7 +248,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.set_cursor_indicator = opts.set_cursor_indicator; + abbr.set_cursor_marker = opts.set_cursor_marker; abbrs_get_set()->add(std::move(abbr)); return STATUS_CMD_OK; } @@ -278,26 +277,27 @@ 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 }; + enum { NON_OPTION_ARGUMENT = 1, REGEX_SHORT, SET_CURSOR_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"set-cursor", required_argument, 'C'}, - {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"set-cursor", optional_argument, SET_CURSOR_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; @@ -346,13 +346,14 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t opts.regex_pattern = w.woptarg; break; } - case 'C': { - if (opts.set_cursor_indicator.has_value()) { + case SET_CURSOR_SHORT: { + if (opts.set_cursor_marker.has_value()) { streams.err.append_format( _(L"%ls: Cannot specify multiple set-cursor options\n"), CMD); return STATUS_INVALID_ARGS; } - opts.set_cursor_indicator = w.woptarg; + // The default set-cursor indicator is '%'. + opts.set_cursor_marker = w.woptarg ? w.woptarg : L"%"; break; } case 'f': diff --git a/tests/pexpects/abbrs.py b/tests/pexpects/abbrs.py index 370c1cd4d..a9f900712 100644 --- a/tests/pexpects/abbrs.py +++ b/tests/pexpects/abbrs.py @@ -143,7 +143,14 @@ expect_prompt(r"6 : @abc@ ") # Test cursor positioning. sendline(r"""abbr --erase (abbr --list) """) expect_prompt() -sendline(r"""abbr LLL --position anywhere --set-cursor !HERE! '!HERE! | less'""") +sendline(r"""abbr LLL --position anywhere --set-cursor 'abc%ghi'""") +expect_prompt() +send(r"""echo LLL def?""") +expect_str(r"") + +sendline(r"""abbr --erase (abbr --list) """) +expect_prompt() +sendline(r"""abbr LLL --position anywhere --set-cursor=!HERE! '!HERE! | less'""") expect_prompt() send(r"""echo LLL derp?""") -expect_str(r"echo derp | less ") +expect_str(r"") From d8dbb9b259ca7c927303a91c115e144ab712d6df Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 10 Dec 2022 14:16:26 -0800 Subject: [PATCH 18/20] Switch abbreviation '-r' flag from --rename to --regex This will be the more common option and provides consistency with `string`. --- CHANGELOG.rst | 1 + doc_src/cmds/abbr.rst | 9 ++++----- src/builtins/abbr.cpp | 29 +++++++++++------------------ tests/checks/abbr.fish | 12 ++++++------ 4 files changed, 22 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9bc5ff1c2..1d8157851 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -44,6 +44,7 @@ Deprecations and removed features - The ``fish_git_prompt`` will now only turn on features if the corresponding boolean variable has been set to a true value (of "1", "yes" or "true") instead of just checking if it is defined. This allows specifically turning features *off* without having to erase variables, e.g. via universal variables. If you have defined a variable to a different value and expect it to count as true, you need to change it (:issue:`9274`). For example, ``set -g __fish_git_prompt_show_informative_status 0`` previously would have enabled informative status (because any value would have done so), now it turns it off. - Abbreviations are no longer stored in universal variables. Existing universal abbreviations are still imported, but new abbreviations should be added to ``config.fish``. +- The short option ``-r`` for abbreviations has changed from ``rename`` to ``regex``, for consistency with ``string``. Scripting improvements ---------------------- diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index 00a13e672..6431251fa 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -8,7 +8,7 @@ Synopsis .. synopsis:: - abbr --add NAME [--position command | anywhere] [--regex PATTERN] + abbr --add NAME [--position command | anywhere] [-r | --regex PATTERN] [--set-cursor[=MARKER]] [-f | --function] EXPANSION abbr --erase NAME ... @@ -37,9 +37,8 @@ Abbreviations may be added to :ref:`config.fish `. Abbreviations .. synopsis:: - abbr [-a | --add] NAME [--position command | anywhere] [--regex PATTERN] - [--set-cursor[=MARKER]] - [-f | --function] EXPANSION + abbr [-a | --add] NAME [--position command | anywhere] [-r | --regex PATTERN] + [--set-cursor[=MARKER]] [-f | --function] EXPANSION ``abbr --add`` creates a new abbreviation. With no other options, the string **NAME** is replaced by **EXPANSION**. @@ -104,7 +103,7 @@ Other subcommands :: - abbr [-r | --rename] OLD_NAME NEW_NAME + abbr --rename OLD_NAME NEW_NAME Renames an abbreviation, from *OLD_NAME* to *NEW_NAME* diff --git a/src/builtins/abbr.cpp b/src/builtins/abbr.cpp index f28bdcbdf..ce234f1b6 100644 --- a/src/builtins/abbr.cpp +++ b/src/builtins/abbr.cpp @@ -277,27 +277,20 @@ 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, SET_CURSOR_SHORT }; + enum { NON_OPTION_ARGUMENT = 1, SET_CURSOR_SHORT, RENAME_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 wchar_t *const short_options = L"-afr:seqgUh"; static const struct woption long_options[] = { - {L"add", no_argument, 'a'}, - {L"position", required_argument, 'p'}, - {L"regex", required_argument, REGEX_SHORT}, - {L"set-cursor", optional_argument, SET_CURSOR_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'}, - {}}; + {L"add", no_argument, 'a'}, {L"position", required_argument, 'p'}, + {L"regex", required_argument, 'r'}, {L"set-cursor", optional_argument, SET_CURSOR_SHORT}, + {L"function", no_argument, 'f'}, {L"rename", no_argument, RENAME_SHORT}, + {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; @@ -337,7 +330,7 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t } break; } - case REGEX_SHORT: { + case 'r': { if (opts.regex_pattern.has_value()) { streams.err.append_format(_(L"%ls: Cannot specify multiple regex patterns\n"), CMD); @@ -359,7 +352,7 @@ maybe_t builtin_abbr(parser_t &parser, io_streams_t &streams, const wchar_t case 'f': opts.function = true; break; - case 'r': + case RENAME_SHORT: opts.rename = true; break; case 'e': diff --git a/tests/checks/abbr.fish b/tests/checks/abbr.fish index e539cb4db..b5a348c28 100644 --- a/tests/checks/abbr.fish +++ b/tests/checks/abbr.fish @@ -56,30 +56,30 @@ abbr | grep 'a b c' # Test renaming abbr __abbr4 omega abbr | grep __abbr5 -abbr -r __abbr4 __abbr5 +abbr --rename __abbr4 __abbr5 abbr | grep __abbr5 # CHECK: abbr -a -- __abbr5 omega abbr -e __abbr5 abbr | grep __abbr4 # Test renaming a nonexistent abbreviation -abbr -r __abbr6 __abbr +abbr --rename __abbr6 __abbr # CHECKERR: abbr --rename: No abbreviation named __abbr6 # Test renaming to a abbreviation with spaces abbr __abbr4 omega -abbr -r __abbr4 "g h i" +abbr --rename __abbr4 "g h i" # CHECKERR: abbr --rename: Abbreviation 'g h i' cannot have spaces in the word abbr -e __abbr4 # Test renaming without arguments abbr __abbr7 omega -abbr -r __abbr7 +abbr --rename __abbr7 # CHECKERR: abbr --rename: Requires exactly two arguments # Test renaming with too many arguments abbr __abbr8 omega -abbr -r __abbr8 __abbr9 __abbr10 +abbr --rename __abbr8 __abbr9 __abbr10 # CHECKERR: abbr --rename: Requires exactly two arguments abbr | grep __abbr8 abbr | grep __abbr9 @@ -89,7 +89,7 @@ abbr | grep __abbr10 # Test renaming to existing abbreviation abbr __abbr11 omega11 abbr __abbr12 omega12 -abbr -r __abbr11 __abbr12 +abbr --rename __abbr11 __abbr12 # CHECKERR: abbr --rename: Abbreviation __abbr12 already exists, cannot rename __abbr11 abbr __abbr-with-dashes omega From 4c3953065abb942ca62b0601f19c160028988a49 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 10 Dec 2022 10:21:20 -0800 Subject: [PATCH 19/20] Update abbreviation completions to reflect new features --- share/completions/abbr.fish | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/share/completions/abbr.fish b/share/completions/abbr.fish index 6a42fa48b..af23b1a78 100644 --- a/share/completions/abbr.fish +++ b/share/completions/abbr.fish @@ -1,10 +1,18 @@ +# "add" is implicit. +set __fish_abbr_not_add_cond '__fish_seen_subcommand_from --query --rename --erase --show --list --help' +set __fish_abbr_add_cond 'not __fish_seen_subcommand_from --query --rename --erase --show --list --help' + complete -c abbr -f -complete -c abbr -f -s a -l add -d 'Add abbreviation' -complete -c abbr -f -s q -l query -d 'Check if an abbreviation exists' -complete -c abbr -f -s r -l rename -d 'Rename an abbreviation' -xa '(abbr --list)' -complete -c abbr -f -s e -l erase -d 'Erase abbreviation' -xa '(abbr --list)' -complete -c abbr -f -s s -l show -d 'Print all abbreviations' -complete -c abbr -f -s l -l list -d 'Print all abbreviation names' -complete -c abbr -f -s g -l global -d 'Store abbreviation in a global variable' -complete -c abbr -f -s U -l universal -d 'Store abbreviation in a universal variable' -complete -c abbr -f -s h -l help -d Help +complete -c abbr -f -n $__fish_abbr_not_add_cond -s a -l add -d 'Add abbreviation' +complete -c abbr -f -n $__fish_abbr_not_add_cond -s q -l query -d 'Check if an abbreviation exists' +complete -c abbr -f -n $__fish_abbr_not_add_cond -l rename -d 'Rename an abbreviation' -xa '(abbr --list)' +complete -c abbr -f -n $__fish_abbr_not_add_cond -s e -l erase -d 'Erase abbreviation' -xa '(abbr --list)' +complete -c abbr -f -n $__fish_abbr_not_add_cond -s s -l show -d 'Print all abbreviations' +complete -c abbr -f -n $__fish_abbr_not_add_cond -s l -l list -d 'Print all abbreviation names' +complete -c abbr -f -n $__fish_abbr_not_add_cond -s h -l help -d Help + + +complete -c abbr -f -n $__fish_abbr_add_cond -s p -l position -a 'command anywhere' -d 'Expand only as a command, or anywhere' -x +complete -c abbr -f -n $__fish_abbr_add_cond -s f -l function -d 'Treat value as a fish function' +complete -c abbr -f -n $__fish_abbr_add_cond -s r -l regex -d 'Match a regular expression' -x +complete -c abbr -f -n $__fish_abbr_add_cond -l set-cursor -d 'Position the cursor at % in the output' From b2ee9c73e1de057de1aa8d8da3b865b7fe8df5a3 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 10 Dec 2022 14:36:00 -0800 Subject: [PATCH 20/20] Call out more forcefully that abbreviations are interactive only --- doc_src/cmds/abbr.rst | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc_src/cmds/abbr.rst b/doc_src/cmds/abbr.rst index 6431251fa..f4bd79778 100644 --- a/doc_src/cmds/abbr.rst +++ b/doc_src/cmds/abbr.rst @@ -20,7 +20,10 @@ Synopsis Description ----------- -``abbr`` manages abbreviations - user-defined words that are replaced with longer phrases after they are entered. +``abbr`` manages abbreviations - user-defined words that are replaced with longer phrases when entered. + +.. note:: + Only typed-in commands use abbreviations. Abbreviations are not expanded in scripts. For example, a frequently-run command like ``git checkout`` can be abbreviated to ``gco``. After entering ``gco`` and pressing :kbd:`Space` or :kbd:`Enter`, the full text ``git checkout`` will appear in the command line. @@ -29,7 +32,8 @@ An abbreviation may match a literal word, or it may match a pattern given by a r Combining these features, it is possible to create custom syntaxes, where a regular expression recognizes matching tokens, and the expansion function interprets them. See the `Examples`_ section. -Abbreviations may be added to :ref:`config.fish `. Abbreviations are only expanded for typed-in commands, not in scripts. +Abbreviations may be added to :ref:`config.fish `. + "add" subcommand