edit_command_buffer: speed up setting cursor position by line/column

alt-e restores the cursor position received from the editor, moving by
one character at a time.  This can be super slow on large commandlines,
even on release builds.  Let's fix that by setting the coordinates
directly.

doc_src/cmds/commandline.rst

@@ -97,7 +97,12 @@ If ``commandline`` is called during a call to complete a given string using ``co
 The following options output metadata about the commandline state:
 
 **-L** or **--line**
-    Print the line that the cursor is on, with the topmost line starting at 1.
+    If no argument is given, print the line that the cursor is on, with the topmost line starting at 1.
+    Otherwise, set the cursor to the given line.
+
+**--column**
+    If no argument is given, print the 1-based offset from the start of the line to the cursor position in Unicode code points.
+    Otherwise, set the cursor to the given code point offset.
 
 **-S** or **--search-mode**
     Evaluates to true if the commandline is performing a history search.

share/functions/edit_command_buffer.fish

@@ -110,13 +110,10 @@ function edit_command_buffer --description 'Edit the command buffer in an extern
             set -l line $pos[2]
             set -l indent (math (string length -- "$raw_lines[$line]") - (string length -- "$unindented_lines[$line]"))
             set -l column (math $pos[3] - $indent)
-            commandline -C 0
+            if not commandline --line $line 2>/dev/null
-            for _line in (seq $line)[2..]
+                commandline -f end-of-buffer
-                commandline -f down-line
+            else
-            end
+                commandline --column $column 2>/dev/null || commandline -f end-of-line
-            commandline -f beginning-of-line
-            for _column in (seq $column)[2..]
-                commandline -f forward-single-char
             end
         end
         command rm $cursor_from_editor

src/builtins/commandline.rs

@@ -7,8 +7,8 @@ use crate::input_common::{CharEvent, ReadlineCmd};
 use crate::operation_context::{no_cancel, OperationContext};
 use crate::parse_constants::ParserTestErrorBits;
 use crate::parse_util::{
-    parse_util_detect_errors, parse_util_job_extent, parse_util_lineno, parse_util_process_extent,
+    parse_util_detect_errors, parse_util_get_offset_from_line, parse_util_job_extent,
-    parse_util_token_extent,
+    parse_util_lineno, parse_util_process_extent, parse_util_token_extent,
 };
 use crate::proc::is_interactive_session;
 use crate::reader::{
@@ -88,7 +88,7 @@ fn replace_part(
     if search_field_mode {
         commandline_set_search_field(out, Some(out_pos));
     } else {
-        commandline_set_buffer(out, Some(out_pos));
+        commandline_set_buffer(Some(out), Some(out_pos));
     }
 }
 
@@ -198,6 +198,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
     let mut selection_start_mode = false;
     let mut selection_end_mode = false;
     let mut line_mode = false;
+    let mut column_mode = false;
     let mut search_mode = false;
     let mut paging_mode = false;
     let mut paging_full_mode = false;
@@ -229,6 +230,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
         wopt(L!("selection-start"), ArgType::NoArgument, 'B'),
         wopt(L!("selection-end"), ArgType::NoArgument, 'E'),
         wopt(L!("line"), ArgType::NoArgument, 'L'),
+        wopt(L!("column"), ArgType::NoArgument, '\x05'),
         wopt(L!("search-mode"), ArgType::NoArgument, 'S'),
         wopt(L!("paging-mode"), ArgType::NoArgument, 'P'),
         wopt(L!("paging-full-mode"), ArgType::NoArgument, 'F'),
@@ -275,6 +277,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
             'B' => selection_start_mode = true,
             'E' => selection_end_mode = true,
             'L' => line_mode = true,
+            '\x05' => column_mode = true,
             'S' => search_mode = true,
             's' => selection_mode = true,
             'P' => paging_mode = true,
@@ -308,6 +311,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
             || token_mode.is_some()
             || cursor_mode
             || line_mode
+            || column_mode
             || search_mode
             || paging_mode
             || selection_start_mode
@@ -363,7 +367,9 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
         return STATUS_INVALID_ARGS;
     }
 
-    if (search_mode || line_mode || cursor_mode || paging_mode) && positional_args > 1 {
+    if (search_mode || line_mode || column_mode || cursor_mode || paging_mode)
+        && positional_args > 1
+    {
         streams
             .err
             .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd));
@@ -372,7 +378,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
     }
 
     if (buffer_part.is_some() || token_mode.is_some() || cut_at_cursor || search_field_mode)
-        && (cursor_mode || line_mode || search_mode || paging_mode || paging_full_mode)
+        && (cursor_mode || line_mode||column_mode || search_mode || paging_mode || paging_full_mode)
         // Special case - we allow to get/set cursor position relative to the process/job/token.
         && ((buffer_part.is_none() && !search_field_mode) || !cursor_mode)
     {
@@ -407,11 +413,75 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
 
     let buffer_part = buffer_part.unwrap_or(TextScope::String);
 
-    if line_mode {
+    if line_mode || column_mode {
-        streams.out.append(sprintf!(
+        if positional_args != 0 {
-            "%d\n",
+            let arg = w.argv[w.wopt_index];
-            parse_util_lineno(&rstate.text, rstate.cursor_pos)
+            let new_coord = match fish_wcstol(arg) {
-        ));
+                Err(_) => {
+                    streams
+                        .err
+                        .append(wgettext_fmt!(BUILTIN_ERR_NOT_NUMBER, cmd, arg));
+                    builtin_print_error_trailer(parser, streams.err, cmd);
+                    0
+                }
+                Ok(num) => num - 1,
+            };
+            let Ok(new_coord) = usize::try_from(new_coord) else {
+                streams
+                    .err
+                    .append(wgettext_fmt!("%ls: line/column index starts at 1", cmd));
+                builtin_print_error_trailer(parser, streams.err, cmd);
+                return STATUS_INVALID_ARGS;
+            };
+
+            let new_pos = if line_mode {
+                let Some(offset) = parse_util_get_offset_from_line(
+                    &rstate.text,
+                    i32::try_from(new_coord).unwrap(),
+                ) else {
+                    streams
+                        .err
+                        .append(wgettext_fmt!("%ls: there is no line %ls\n", cmd, arg));
+                    builtin_print_error_trailer(parser, streams.err, cmd);
+                    return STATUS_INVALID_ARGS;
+                };
+                offset
+            } else {
+                let line_index =
+                    i32::try_from(parse_util_lineno(&rstate.text, rstate.cursor_pos)).unwrap() - 1;
+                let line_offset =
+                    parse_util_get_offset_from_line(&rstate.text, line_index).unwrap_or_default();
+                let next_line_offset =
+                    parse_util_get_offset_from_line(&rstate.text, line_index + 1)
+                        .unwrap_or(rstate.text.len());
+                if line_offset + new_coord > next_line_offset {
+                    streams.err.append(wgettext_fmt!(
+                        "%ls: column %ls exceeds line length\n",
+                        cmd,
+                        arg
+                    ));
+                    builtin_print_error_trailer(parser, streams.err, cmd);
+                    return STATUS_INVALID_ARGS;
+                }
+                line_offset + new_coord
+            };
+            commandline_set_buffer(None, Some(new_pos));
+        } else {
+            streams.out.append(sprintf!(
+                "%d\n",
+                if line_mode {
+                    parse_util_lineno(&rstate.text, rstate.cursor_pos)
+                } else {
+                    rstate.cursor_pos + 1
+                        - parse_util_get_offset_from_line(
+                            &rstate.text,
+                            i32::try_from(parse_util_lineno(&rstate.text, rstate.cursor_pos))
+                                .unwrap(),
+                        )
+                        .unwrap_or_default()
+                }
+            ));
+        }
         return STATUS_CMD_OK;
     }
 
@@ -561,7 +631,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
                     .saturating_add_signed(isize::try_from(new_pos).unwrap()),
                 current_buffer.len(),
             );
-            commandline_set_buffer(current_buffer.to_owned(), Some(new_pos));
+            commandline_set_buffer(None, Some(new_pos));
         } else {
             streams.out.append(sprintf!("%lu\n", current_cursor_pos));
         }

src/builtins/read.rs

@@ -233,7 +233,7 @@ fn read_interactive(
 
     // Keep in-memory history only.
     reader_push(parser, L!(""), conf);
-    commandline_set_buffer(commandline.to_owned(), None);
+    commandline_set_buffer(Some(commandline.to_owned()), None);
 
     let mline = {
         let _interactive = scoped_push_replacer(

src/parse_util.rs

@@ -532,7 +532,6 @@ pub fn parse_util_get_offset_from_line(s: &wstr, line: i32) -> Option<usize> {
         return Some(0);
     }
 
-    // let mut pos = -1 as usize;
     let mut count = 0;
     for (pos, _) in s.chars().enumerate().filter(|(_, c)| *c == '\n') {
         count += 1;

src/reader.rs

@@ -1009,11 +1009,13 @@ pub fn commandline_get_state(sync: bool) -> CommandlineState {
 
 /// Set the command line text and position. This may be called on a background thread; the reader
 /// will pick it up when it is done executing.
-pub fn commandline_set_buffer(text: WString, cursor_pos: Option<usize>) {
+pub fn commandline_set_buffer(text: Option<WString>, cursor_pos: Option<usize>) {
     {
         let mut state = commandline_state_snapshot();
-        state.cursor_pos = cmp::min(cursor_pos.unwrap_or(usize::MAX), text.len());
+        if let Some(text) = text {
-        state.text = text;
+            state.text = text;
+        }
+        state.cursor_pos = cmp::min(cursor_pos.unwrap_or(usize::MAX), state.text.len());
     }
     current_data().map(|data| data.apply_commandline_state_changes());
 }