Expand abbr explicitly (#5762)

* Add "expand-abbr" bind function

This can be used to explictly allow expanding abbreviations.

* Make expanding abbr explicit

NOTE: This accepts them for space only, we currently also do it for \n
and \r.

* Remove now dead code

We no longer trigger an abbr implicitly, so we can remove the code
that does it.

* Fix comment

[ci skip]

share/functions/fish_default_key_bindings.fish

@@ -31,6 +31,9 @@ function fish_default_key_bindings -d "Default (Emacs-like) key bindings for fis
     bind --preset $argv "" self-insert
     or exit # protect against invalid $argv
 
+    # Space expands abbrs _and_ inserts itself.
+    bind --preset $argv " " 'commandline -i " "; commandline -f expand-abbr'
+
     bind --preset $argv \n execute
     bind --preset $argv \r execute
 

share/functions/fish_vi_key_bindings.fish

@@ -54,6 +54,9 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish'
     bind -s --preset -M insert \n execute
 
     bind -s --preset -M insert "" self-insert
+    # Space expands abbrs _and_ inserts itself.
+    bind -s --preset -M insert " " 'commandline -i " "; commandline -f expand-abbr'
+
 
     # Add way to kill current command line while in insert mode.
     bind -s --preset -M insert \cc __fish_cancel_commandline

sphinx_doc_src/cmds/bind.rst

@@ -112,6 +112,8 @@ The following special input functions are available:
 
 - ``end-selection``, end selecting text
 
+- ``expand-abbr`` expands any abbreviation currently under the cursor
+
 - ``forward-bigword``, move one whitespace-delimited word to the right
 
 - ``forward-char``, move one character to the right

src/input.cpp

@@ -129,6 +129,7 @@ static const input_function_metadata_t input_function_metadata[] = {
     {readline_cmd_t::repeat_jump, L"repeat-jump"},
     {readline_cmd_t::reverse_repeat_jump, L"repeat-jump-reverse"},
     {readline_cmd_t::func_and, L"and"},
+    {readline_cmd_t::expand_abbr, L"expand-abbr"},
     {readline_cmd_t::cancel, L"cancel"}};
 
 static_assert(sizeof(input_function_metadata) / sizeof(input_function_metadata[0]) ==

src/input_common.h

@@ -65,6 +65,7 @@ enum class readline_cmd_t {
     forward_jump_till,
     backward_jump_till,
     func_and,
+    expand_abbr,
     cancel,
     repeat_jump,
     reverse_repeat_jump,

src/reader.cpp

@@ -430,13 +430,12 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
     void update_buff_pos(editable_line_t *el, size_t buff_pos);
     void repaint();
     void kill(editable_line_t *el, size_t begin_idx, size_t length, int mode, int newv);
-    bool insert_string(editable_line_t *el, const wcstring &str,
-                       bool allow_expand_abbreviations = false);
+    bool insert_string(editable_line_t *el, const wcstring &str);
 
     /// Insert the character into the command line buffer and print it to the screen using syntax
     /// highlighting, etc.
-    bool insert_char(editable_line_t *el, wchar_t c, bool allow_expand_abbreviations = false) {
-        return insert_string(el, wcstring{c}, allow_expand_abbreviations);
+    bool insert_char(editable_line_t *el, wchar_t c) {
+        return insert_string(el, wcstring{c});
     }
 
     void move_word(editable_line_t *el, bool move_right, bool erase, enum move_word_style_t style,
@@ -1134,43 +1133,21 @@ void reader_data_t::remove_backward() {
 }
 
 /// Insert the characters of the string into the command line buffer and print them to the screen
-/// using syntax highlighting, etc. Optionally also expand abbreviations, after space characters.
+/// using syntax highlighting, etc.
 /// Returns true if the string changed.
-bool reader_data_t::insert_string(editable_line_t *el, const wcstring &str,
-                                  bool allow_expand_abbreviations) {
+bool reader_data_t::insert_string(editable_line_t *el, const wcstring &str) {
     size_t len = str.size();
     if (len == 0) return false;
 
-    // Start inserting. If we are expanding abbreviations, we have to do this after every space (see
-    // #1434), so look for spaces. We try to do this efficiently (rather than the simpler character
-    // at a time) to avoid expensive work in command_line_changed().
+    // Start inserting.
     size_t cursor = 0;
     while (cursor < len) {
-        // Determine the position of the next expansion-triggering char (possibly none), and the end
-        // of the range we wish to insert.
-        const wchar_t *expansion_triggering_chars = L" ;|&^><";
-        size_t char_triggering_expansion_pos =
-            allow_expand_abbreviations ? str.find_first_of(expansion_triggering_chars, cursor)
-                                       : wcstring::npos;
-        bool has_expansion_triggering_char = (char_triggering_expansion_pos != wcstring::npos);
-        size_t range_end =
-            (has_expansion_triggering_char ? char_triggering_expansion_pos + 1 : len);
-
         // Insert from the cursor up to but not including the range end.
-        assert(range_end > cursor);
-        el->insert_string(str, cursor, range_end - cursor);
+        el->insert_string(str, cursor, len - cursor);
 
         update_buff_pos(el, el->position);
         command_line_changed(el);
-
-        // If we got an expansion trigger, then the last character we inserted was it (i.e. was a
-        // space). Expand abbreviations.
-        if (has_expansion_triggering_char && allow_expand_abbreviations) {
-            assert(range_end > 0);
-            assert(std::wcschr(expansion_triggering_chars, str.at(range_end - 1)));
-            expand_abbreviation_as_necessary(1);
-        }
-        cursor = range_end;
+        cursor = len;
     }
 
     if (el == &command_line) {
@@ -2471,7 +2448,7 @@ maybe_t<char_event_t> reader_data_t::read_normal_chars(readline_loop_state_t &rl
     }
 
     editable_line_t *el = active_edit_line();
-    insert_string(el, arr, true);
+    insert_string(el, arr);
 
     // End paging upon inserting into the normal command line.
     if (el == &command_line) {
@@ -3193,6 +3170,17 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
             break;
         }
 
+        case rl::expand_abbr: {
+            editable_line_t *el = active_edit_line();
+            if (expand_abbreviation_as_necessary(1)) {
+                super_highlight_me_plenty();
+                mark_repaint_needed();
+                input_function_set_status(true);
+            } else {
+                input_function_set_status(false);
+            }
+            break;
+        }
             // Some commands should have been handled internally by input_readch().
         case rl::self_insert: {
             DIE("self-insert should have been handled by input_readch");
@@ -3317,8 +3305,7 @@ maybe_t<wcstring> reader_data_t::readline(int nchars_or_0) {
                        c != 0x7F) {
                 // Regular character.
                 editable_line_t *el = active_edit_line();
-                bool allow_expand_abbreviations = (el == &command_line);
-                insert_char(active_edit_line(), c, allow_expand_abbreviations);
+                insert_char(active_edit_line(), c);
 
                 // End paging upon inserting into the normal command line.
                 if (el == &command_line) {