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

@@ -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_map<wcstring, env_var_t
         }
     }
 }
+
+// static
+abbrs_replacement_t abbrs_replacement_t::from(source_range_t range, wcstring text,
+                                              const abbrs_replacer_t &replacer) {
+    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 (pos != wcstring::npos) {
+            result.text.erase(pos, replacer.set_cursor_indicator->size());
+            result.cursor = pos + range.start;
+        }
+    }
+    return result;
+}

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<wcstring> 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<wcstring> set_cursor_indicator;
 };
 using abbrs_replacer_list_t = std::vector<abbrs_replacer_t>;
 
+/// 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<size_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.

src/builtins/abbr.cpp

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

src/fish_tests.cpp

@@ -2506,11 +2506,11 @@ 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 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<highlight_spec_t> 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();

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
+/// Expand an abbreviation replacer, which may mean running its function.
-/// its function. \return the replacement string, or none to skip it. This may run fish script!
+/// \return the replacement, or none to skip it. This may run fish script!
-maybe_t<wcstring> expand_replacer(const wcstring &token, const abbrs_replacer_t &repl,
+maybe_t<abbrs_replacement_t> expand_replacer(source_range_t range, const wcstring &token,
-                                  parser_t &parser) {
+                                             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<wcstring> 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<positioned_token_t> extract_tokens(const wcstring &str) {
     return result;
 }
 
-/// Expand abbreviations at the given cursor position. Does NOT inspect 'data'.
+/// Expand abbreviations in the given phase at the given cursor position.
-maybe_t<edit_t> reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos,
+/// cursor. \return the replacement. This does NOT inspect the current reader data.
-                                                     abbrs_phase_t phase, parser_t &parser) {
+maybe_t<abbrs_replacement_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<edit_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)) {
+        if (auto replacement = expand_replacer(range, token_str, replacer, parser)) {
-            return edit_t{range.start, range.length, replacement.acquire()};
+            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 =
+        if (auto replacement = reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, phase,
-                reader_expand_abbreviation_at_cursor(el->text(), cursor_pos, phase, parser())) {
+                                                                    this->parser())) {
-            push_edit(el, std::move(*edit));
+            push_edit(el, edit_t{replacement->range, std::move(replacement->text)});
-            update_buff_pos(el);
+            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);
+            const auto replacement = expand_replacer(orig_range, token, replacer, parser);
-            if (replacement && *replacement != token) {
+            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;
             }
         }

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
+/// Expand at most one abbreviation at the given cursor position, updating the position if the
-/// abbreviations which want function calls.
+/// abbreviation wants to move the cursor. Use the parser to run any abbreviations which want
-/// \return none if no abbreviations were expanded, otherwise the resulting edit.
+/// function calls. \return none if no abbreviations were expanded, otherwise the resulting edit.
 enum class abbrs_phase_t : uint8_t;
-maybe_t<edit_t> reader_expand_abbreviation_at_cursor(const wcstring &cmdline, size_t cursor_pos,
+struct abbrs_replacement_t;
-                                                     abbrs_phase_t phase, parser_t &parser);
+maybe_t<abbrs_replacement_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,

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