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.
This commit is contained in:
ridiculousfish 2022-11-27 11:29:16 -08:00
parent 695cc74c88
commit 5841e9f712
8 changed files with 22 additions and 156 deletions

View file

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

View file

@ -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 <configuration>`. Abbreviations are only expanded for typed-in commands, not in scripts.
@ -41,21 +39,19 @@ Abbreviations may be added to :ref:`config.fish <configuration>`. 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.
::

View file

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

View file

@ -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<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, 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<int> 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<int> 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(

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_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());
}

View file

@ -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<size_t>::max()};
};
@ -886,10 +882,9 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
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<positioned_token_t> 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<wcstring> 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<wcstring> 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<wcstring>{std::move(rls.cmd)} : none();
return rls.finished ? maybe_t<wcstring>{command_line.text()} : none();
}
bool reader_data_t::jump(jump_direction_t dir, jump_precision_t precision, editable_line_t *el,

View file

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

View file

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