Vi-mode poor autosuggestion behavior / Invalid cursor placement and length in normal mode

[johnpyp]

fish, version 3.7.0
3.7.0
OS: Linux 6.6.14 NixOS (but happens on any OS)
Term: Kitty (but any terminal does this, over ssh, etc.)

---

In vi mode, I noticed an autocomplete issue, say you start here:

If you then press ESC to enter normal mode, then l to attempt to complete the autosuggestion, it doesn't work. You have to press l twice to complete.

It seems that the reason for this issue with the autosuggestion or the fact that ESC moves you back, but that vi mode lets you move your normal mode cursor to after the last character, which isn't actually a valid vi place to put the cursor (or a "non-extant character").

Then, when autosuggestion logic checks if you're at the last possible cursor position when you call forward-char with l, it should consider you at the last possible normal mode position, but instead it thinks that there is that one extra place for the cursor to be.

[faho]

I'm disagreeing that this is a good first issue, it cuts to a fundamental issue with vi-mode: The cursor placement isn't the same as in emacs mode.

That needs some careful consideration, because it would probably touch a lot more than the one bind function.

See also #8189 or #3005, which both mention it as an issue with $, and #3006, which tried to fix it exclusively for $.

I'm going to close both of those and leave the issue at hand to be tracked here.

[johnpyp]

Until this gets fixed fundamentally, is there any intermediary fish script conditional hack that could fix this specific pattern (normal mode forward-char accepting suggestions behavior)? Or are the apis necessary just not available?

[krobelus]

It's not worth to add a hack.
Better change Vi mode to always put the cursor on a character (except maybe if the commandline is empty).
We can start from this and fix any remaining issues.

diff --git a/src/reader.rs b/src/reader.rs
index 45dbe69fd..5fd0425c1 100644
--- a/src/reader.rs
+++ b/src/reader.rs
@@ -1181,13 +1181,26 @@ impl ReaderData {
     }
 
     /// Update the cursor position.
-    fn update_buff_pos(&mut self, elt: EditableLineTag, new_pos: Option<usize>) {
-        if let Some(pos) = new_pos {
+    fn update_buff_pos(&mut self, elt: EditableLineTag, new_pos: Option<usize>) -> bool {
+        if let Some(mut pos) = new_pos {
+            if self
+                .vars()
+                .get_unless_empty(L!("fish_cursor_selection_mode"))
+                .is_some_and(|mode| mode.as_list()[0] == "inclusive")
+            {
+                let el = self.edit_line(elt);
+                if !el.is_empty() && pos == el.len() {
+                    pos = el.len() - 1;
+                    if el.position() == pos {
+                        return false;
+                    }
+                }
+            }
             self.edit_line_mut(elt).set_position(pos);
         }
 
         if elt != EditableLineTag::Commandline {
-            return;
+            return true;
         }
         let buff_pos = self.command_line.position();
         let target_char = if self.cursor_selection_mode == CursorSelectionMode::Inclusive {
@@ -1196,7 +1209,7 @@ impl ReaderData {
             0
         };
         let Some(selection) = self.selection.as_mut() else {
-            return;
+            return true;
         };
         if selection.begin <= buff_pos {
             selection.start = selection.begin;
@@ -1205,6 +1218,7 @@ impl ReaderData {
             selection.start = buff_pos;
             selection.stop = selection.begin + target_char;
         }
+        true
     }
 }
 
@@ -2240,7 +2254,9 @@ impl ReaderData {
                     }
                     position
                 };
-                self.update_buff_pos(self.active_edit_line_tag(), Some(position + 1));
+                if !self.update_buff_pos(self.active_edit_line_tag(), Some(position + 1)) {
+                    break;
+                }
             },
             rl::BeginningOfBuffer => {
                 self.update_buff_pos(EditableLineTag::Commandline, Some(0));
@@ -2644,9 +2660,9 @@ impl ReaderData {
                 let (elt, el) = self.active_edit_line();
                 if self.is_navigating_pager_contents() {
                     self.select_completion_in_direction(SelectionMotion::East, false);
-                } else if el.position() != el.len() {
-                    self.update_buff_pos(elt, Some(el.position() + 1));
-                } else {
+                } else if el.position() == el.len()
+                    || !self.update_buff_pos(elt, Some(el.position() + 1))
+                {
                     self.accept_autosuggestion(
                         /*full=*/ c != rl::ForwardSingleChar,
                         /*single=*/ c == rl::ForwardSingleChar,