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.
This commit is contained in:
ridiculousfish 2022-12-10 12:45:21 -08:00
parent 35a4688650
commit 01039537b0
10 changed files with 31 additions and 111 deletions

View File

@ -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.

View File

@ -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 <configuration>`. 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.

View File

@ -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_set_t> 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;
}
}

View File

@ -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_set_t> 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

View File

@ -41,7 +41,6 @@ struct abbr_options_t {
bool function{};
maybe_t<wcstring> regex_pattern;
maybe_t<abbrs_position_t> position{};
maybe_t<abbrs_triggers_t> triggers{};
maybe_t<wcstring> 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<int> 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<int> 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<int> 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(

View File

@ -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<wcstring> {
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<wcstring> result;
auto expand_abbreviation_in_command = [](const wcstring &cmdline,
size_t cursor_pos) -> maybe_t<wcstring> {
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<highlight_spec_t> colors{cmdline_expanded.size()};
apply_edit(&cmdline_expanded, &colors, edit_t{replacement->range, replacement->text});

View File

@ -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();

View File

@ -788,9 +788,8 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
/// 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<positioned_token_t> 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<abbrs_replacement_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<abbrs_replacement_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<abbrs_replacement_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) {

View File

@ -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<abbrs_replacement_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.

View File

@ -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")