Revert changes to restore cursor position after undo

This feature is nice and desirable, but it was implemented in a intrusive way
by modifying the sequence of bytes we emit when running a command; this in
turn requires changing a bunch of tests.

This sequence hasn't changed in decades and the consequences of changing it
are hard to predict, given that it is likely terminal dependent; we've
already found a regression.

It's fine to reintroduce this but it should be done in a less intrusive way
(conceptually that seems straightforward - we're just remembering the cursor
position).

Revert "Fix spurious blank lines when executing scrolled commandline"

This reverts commit 0e512f8033.

Revert "On undo after execute, restore the cursor position "

This reverts commit 610338cc70.

CHANGELOG.rst

@@ -26,7 +26,6 @@ New or improved bindings
 - On non-macOS systems, :kbd:`alt-left`, :kbd:`alt-right`, :kbd:`alt-backspace`, :kbd:`alt-delete` no longer operate on punctuation-delimited words but on whole arguments, possibly including special characters like ``/`` and quoted spaces.
   On macOS, the same corresponding :kbd:`ctrl-` prefixed keys operate on whole arguments.
   Word operations are still available via the other respective modifier, same as in the browser.
-- :kbd:`ctrl-z` (undo) after executing a command will restore the previous cursor position instead of placing the cursor at the end of the command line.
 - The OSC 133 prompt marking feature has learned about kitty's ``click_events=1`` flag, which allows moving fish's cursor by clicking.
 - :kbd:`ctrl-l` now pushes all text located above the prompt to the terminal's scrollback, before clearing and redrawing the screen (via a new special input function ``scrollback-push``).
   This feature depends on the terminal advertising via XTGETTCAP support for the ``indn`` and ``cuu`` terminfo capabilities,

src/reader.rs

@@ -152,8 +152,7 @@
     string_prefixes_string_case_insensitive, StringFuzzyMatch,
 };
 use crate::wildcard::wildcard_has;
-use crate::wutil::wstat;
+use crate::wutil::{fstat, perror, write_to_fd, wstat};
-use crate::wutil::{fstat, perror};
 use crate::{abbrs, event, function};
 
 /// A description of where fish is in the process of exiting.
@@ -2267,8 +2266,11 @@ fn readline(&mut self, nchars: Option<NonZeroUsize>) -> Option<WString> {
             zelf.finish_highlighting_before_exec();
         }
 
-        // Move the cursor so that output is on the line after the command.
+        // Emit a newline so that the output is on the line after the command.
-        zelf.screen.move_to_end();
+        // But do not emit a newline if the cursor has wrapped onto a new line all its own - see #6826.
+        if !zelf.screen.cursor_is_wrapped_to_own_line() {
+            let _ = write_to_fd(b"\n", STDOUT_FILENO);
+        }
 
         // HACK: If stdin isn't the same terminal as stdout, we just moved the cursor.
         // For now, just reset it to the beginning of the line.
@@ -4012,6 +4014,7 @@ fn handle_execute(&mut self) -> bool {
 
         self.add_to_history();
         self.rls_mut().finished = true;
+        self.update_buff_pos(elt, Some(self.command_line_len()));
         true
     }
 

src/screen.rs

@@ -207,9 +207,8 @@ pub fn is_empty(&self) -> bool {
 pub struct Screen {
     /// Whether the last-drawn autosuggestion (if any) is truncated, or hidden entirely.
     pub autosuggestion_is_truncated: bool,
-    /// If the last rendering was so large we could only display part of the command line,
+    /// True if the last rendering was so large we could only display part of the command line.
-    /// this is the number of lines that were pushed to scrollback.
+    pub scrolled: bool,
-    pub scroll_amount: usize,
 
     /// Receiver for our output.
     outp: &'static RefCell<Outputter>,
@@ -245,7 +244,7 @@ pub fn new() -> Self {
         Self {
             outp: Outputter::stdoutput(),
             autosuggestion_is_truncated: Default::default(),
-            scroll_amount: Default::default(),
+            scrolled: Default::default(),
             desired: Default::default(),
             actual: Default::default(),
             actual_left_prompt: Default::default(),
@@ -258,8 +257,9 @@ pub fn new() -> Self {
         }
     }
 
+    /// Return whether the last rendering was so large we could only display part of the command line.
     pub fn scrolled(&self) -> bool {
-        self.scroll_amount != 0
+        self.scrolled
     }
 
     /// This is the main function for the screen output library. It is used to define the desired
@@ -515,7 +515,7 @@ struct ScrolledCursor {
         // Append pager_data (none if empty).
         self.desired.append_lines(&page_rendering.screen_data);
 
-        self.scroll_amount = scrolled_cursor.scroll_amount;
+        self.scrolled = scrolled_cursor.scroll_amount != 0;
 
         self.update(
             vars,
@@ -558,10 +558,6 @@ pub fn reset_line(&mut self, repaint_prompt: bool /* = false */) {
         self.save_status();
     }
 
-    pub fn move_to_end(&mut self) {
-        self.r#move(0, self.actual.line_count() - self.scroll_amount);
-    }
-
     pub fn push_to_scrollback(&mut self, cursor_y: usize) {
         let prompt_y = self.command_line_y_given_cursor_y(cursor_y);
         let trailing_prompt_lines = self

tests/checks/tmux-commandline.fish

@@ -40,23 +40,6 @@ isolated-tmux capture-pane -p
 # CHECK: 10
 # CHECK: scroll_here
 
-# The output is broken here (seems tmux specific).
-isolated-tmux send-keys C-c
-tmux-sleep
-isolated-tmux send-keys C-l 'commandline -i ": \'$(seq $LINES)" A B "C\'"' Enter Enter
-tmux-sleep
-isolated-tmux capture-pane -p
-# CHECK: 4
-# CHECK: 5
-# CHECK: 6
-# CHECK: 7
-# CHECK: 8
-# CHECK: 9
-# CHECK: 10
-# CHECK: prompt 5>
-# CHECK: B
-# CHECK: C'
-
 # Soft-wrapped commandline with omitted right prompt.
 isolated-tmux send-keys C-q '
     function fish_right_prompt
@@ -66,40 +49,7 @@ isolated-tmux send-keys C-q '
 ' Enter
 tmux-sleep
 isolated-tmux capture-pane -p | sed 1,5d
-# CHECK: prompt {{\d+}}> echo 00000000000000000000000000000000000000000000000000000000000000000
+# CHECK: prompt 5> echo 00000000000000000000000000000000000000000000000000000000000000000
 # CHECK: 000000000000000
 # CHECK: 00000000000000000000000000000000000000000000000000000000000000000000000000000000
-# CHECK: prompt {{\d+}}>                                                           right-prompt
+# CHECK: prompt 6>                                                           right-prompt
-
-isolated-tmux send-keys C-q 'echo | echo\;' M-Enter 'another job' C-b C-b C-g
-tmux-sleep
-isolated-tmux capture-pane -p
-# CHECK: prompt {{\d+}}> echo | echo;
-# CHECK:          another job
-# CHECK: another job
-# CHECK: prompt {{\d+}}> echo | echo;
-# CHECK:          another job
-
-isolated-tmux send-keys C-q 'echo foobar' Left Left Left C-t
-tmux-sleep
-# CHECK: prompt {{\d+}}> echo foobar
-# CHECK: cursor is at offset 3 in token
-# CHECK: prompt {{\d+}}> echo foobar
-isolated-tmux capture-pane -p
-
-isolated-tmux send-keys C-a C-k \
-    'bind ctrl-x,a "__fish_echo echo line=(commandline --line) column=(commandline --column)"' \
-    Enter \
-    C-l "echo '1" Enter 2
-tmux-sleep
-isolated-tmux send-keys C-x a C-a Up C-x a
-tmux-sleep
-# CHECK: prompt {{\d+}}> echo '1
-# CHECK: 2
-# CHECK: line=2 column=2
-# CHECK: prompt {{\d+}}> echo '1
-# CHECK: 2
-# CHECK: line=1 column=1
-# CHECK: prompt {{\d+}}> echo '1
-# CHECK: 2
-isolated-tmux capture-pane -p

tests/pexpect_helper.py

@@ -131,12 +131,6 @@ class Message(object):
         return Message(Message.DIR_OUTPUT, text, when)
 
 
-# Sequences for moving the cursor below the commandline. This happens before executing.
-MOVE_TO_END: str = r"(?:\r\n|\x1b\[2 q|)"
-TO_END: str = MOVE_TO_END + r"[^\n]*"
-TO_END_SUFFIX: str = r"[^\n]*" + MOVE_TO_END
-
-
 class SpawnedProc(object):
     """A process, talking to our ptty. This wraps pexpect.spawn.
 

tests/pexpects/bind.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-from pexpect_helper import SpawnedProc, TO_END
+from pexpect_helper import SpawnedProc
 import os
 import platform
 import sys
@@ -44,9 +44,7 @@ expect_prompt("")
 # Start by testing with no delay. This should transpose the words.
 send("echo abc def")
 send("\033t\r")
-expect_prompt(
+expect_prompt("\r\n.*def abc\r\n")  # emacs transpose words, default timeout: no delay
-    TO_END + "def abc\r\n"
-)  # emacs transpose words, default timeout: no delay
 
 # Now test with a delay > 0 and < the escape timeout. This should transpose
 # the words.
@@ -55,7 +53,7 @@ send("\033")
 sleep(0.010)
 send("t\r")
 # emacs transpose words, default timeout: short delay
-expect_prompt(TO_END + "jkl ghi\r\n")
+expect_prompt("\r\n.*jkl ghi\r\n")
 
 # Now test with a delay > the escape timeout. The transposition should not
 # occur and the "t" should become part of the text that is echoed.
@@ -64,11 +62,11 @@ send("\033")
 sleep(0.250)
 send("t\r")
 # emacs transpose words, default timeout: long delay
-expect_prompt(TO_END + "mno pqrt\r\n")
+expect_prompt("\r\n.*mno pqrt\r\n")
 
 # Now test that exactly the expected bind modes are defined
 sendline("bind --list-modes")
-expect_prompt(TO_END + "default", unmatched="Unexpected bind modes")
+expect_prompt("\r\n.*default", unmatched="Unexpected bind modes")
 
 # Test vi key bindings.
 # This should leave vi mode in the insert state.
@@ -78,8 +76,7 @@ expect_prompt()
 # Go through a prompt cycle to let fish catch up, it may be slow due to ASAN
 sendline("echo success: default escape timeout")
 expect_prompt(
-    TO_END + "success: default escape timeout",
+    "\r\n.*success: default escape timeout", unmatched="prime vi mode, default timeout"
-    unmatched="prime vi mode, default timeout",
 )
 
 send("echo fail: default escape timeout")
@@ -93,7 +90,7 @@ sleep(0.250)
 send("ddi")
 sendline("echo success: default escape timeout")
 expect_prompt(
-    TO_END + "success: default escape timeout\r\n",
+    "\r\n.*success: default escape timeout\r\n",
     unmatched="vi replace line, default timeout: long delay",
 )
 
@@ -108,7 +105,7 @@ send("\033")
 sleep(0.400)
 send("hhrAi\r")
 expect_prompt(
-    TO_END + "TAXT\r\n", unmatched="vi mode replace char, default timeout: long delay"
+    "\r\n.*TAXT\r\n", unmatched="vi mode replace char, default timeout: long delay"
 )
 
 # Test deleting characters with 'x'.
@@ -120,7 +117,7 @@ send("xxxxx\r")
 
 # vi mode delete char, default timeout: long delay
 expect_prompt(
-    TO_END + "MORE\r\n", unmatched="vi mode delete char, default timeout: long delay"
+    "\r\n.*MORE\r\n", unmatched="vi mode delete char, default timeout: long delay"
 )
 
 # Test jumping forward til before a character with t
@@ -132,7 +129,7 @@ send("0tTD\r")
 
 # vi mode forward-jump-till character, default timeout: long delay
 expect_prompt(
-    TO_END + "MORE\r\n",
+    "\r\n.*MORE\r\n",
     unmatched="vi mode forward-jump-till character, default timeout: long delay",
 )
 
@@ -145,7 +142,7 @@ expect_prompt(
 # send("TSD\r")
 # # vi mode backward-jump-till character, default timeout: long delay
 # expect_prompt(
-#     TO_END + "MORE-TEXT-IS\r\n",
+#     "\r\n.*MORE-TEXT-IS\r\n",
 #     unmatched="vi mode backward-jump-till character, default timeout: long delay",
 # )
 
@@ -157,7 +154,7 @@ sleep(0.250)
 send("F-;D\r")
 # vi mode backward-jump-to character and repeat, default timeout: long delay
 expect_prompt(
-    TO_END + "MORE-TEXT\r\n",
+    "\r\n.*MORE-TEXT\r\n",
     unmatched="vi mode backward-jump-to character and repeat, default timeout: long delay",
 )
 
@@ -169,7 +166,7 @@ sleep(0.250)
 send("F-F-,D\r")
 # vi mode backward-jump-to character, and reverse, default timeout: long delay
 expect_prompt(
-    TO_END + "MORE-TEXT-IS\r\n",
+    "\r\n.*MORE-TEXT-IS\r\n",
     unmatched="vi mode backward-jump-to character, and reverse, default timeout: long delay",
 )
 
@@ -184,7 +181,7 @@ send("ddi")
 sleep(0.25)
 send("echo success: lengthened escape timeout\r")
 expect_prompt(
-    TO_END + "success: lengthened escape timeout\r\n",
+    "\r\n.*success: lengthened escape timeout\r\n",
     unmatched="vi replace line, 100ms timeout: long delay",
 )
 
@@ -196,7 +193,7 @@ sleep(0.010)
 send("ddi")
 send("inserted\r")
 expect_prompt(
-    TO_END + "fail: no normal modediinserted\r\n",
+    "\r\n.*fail: no normal modediinserted\r\n",
     unmatched="vi replace line, 100ms timeout: short delay",
 )
 
@@ -213,7 +210,7 @@ expect_str("echo TEXT")
 send("\033")
 sleep(0.200)
 send("hhtTrN\r")
-expect_prompt(TO_END + "TENT\r\n", unmatched="Couldn't find expected output 'TENT'")
+expect_prompt("\r\n.*TENT\r\n", unmatched="Couldn't find expected output 'TENT'")
 
 # Test sequence key delay
 send("set -g fish_sequence_key_delay_ms 200\r")
@@ -244,7 +241,7 @@ expect_prompt("foo")
 # send("echo some TExT\033")
 # sleep(0.300)
 # send("hh~~bbve~\r")
-# expect_prompt(TO_END + "SOME TeXT\r\n", unmatched="Couldn't find expected output 'SOME TeXT")
+# expect_prompt("\r\n.*SOME TeXT\r\n", unmatched="Couldn't find expected output 'SOME TeXT")
 
 send("echo echo")
 send("\033")
@@ -272,7 +269,7 @@ expect_prompt()
 # Verify the custom escape timeout set earlier is still in effect.
 sendline("echo fish_escape_delay_ms=$fish_escape_delay_ms")
 expect_prompt(
-    TO_END + "fish_escape_delay_ms=50\r\n",
+    "\r\n.*fish_escape_delay_ms=50\r\n",
     unmatched="default-mode custom timeout not set correctly",
 )
 
@@ -287,8 +284,7 @@ send("echo abc def")
 send("\033")
 send("t\r")
 expect_prompt(
-    TO_END + "def abc\r\n",
+    "\r\n.*def abc\r\n", unmatched="emacs transpose words fail, 200ms timeout: no delay"
-    unmatched="emacs transpose words fail, 200ms timeout: no delay",
 )
 
 # Verify special characters, such as \cV, are not intercepted by the kernel
@@ -319,7 +315,7 @@ expect_prompt()
 send("foo ")
 expect_str("echo foonanana")
 send(" banana\r")
-expect_str(" banana")
+expect_str(" banana\r")
 expect_prompt("foonanana banana")
 
 # Ensure that nul can be bound properly (#3189).
@@ -347,7 +343,7 @@ expect_prompt()
 send("a b c d\x01")  # ctrl-a, move back to the beginning of the line
 send("\x07")  # ctrl-g, kill bigword
 sendline("echo")
-expect_prompt(TO_END + "b c d")
+expect_prompt("\n.*b c d")
 
 # Test that overriding the escape binding works
 # and does not inhibit other escape sequences (up-arrow in this case).
@@ -363,7 +359,7 @@ expect_prompt()
 send("    a b c d\x01")  # ctrl-a, move back to the beginning of the line
 send("\x07")  # ctrl-g, kill bigword
 sendline("echo")
-expect_prompt(TO_END + "b c d")
+expect_prompt("\n.*b c d")
 
 # Check that ctrl-z can be bound
 sendline('bind ctrl-z "echo bound ctrl-z"')

tests/pexpects/bind_mode_events.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-from pexpect_helper import SpawnedProc, TO_END
+from pexpect_helper import SpawnedProc
 import os
 import sys
 import signal
@@ -16,7 +16,7 @@ send("set -g fish_key_bindings fish_vi_key_bindings\r")
 expect_prompt()
 
 send("echo ready to go\r")
-expect_prompt(TO_END + f"ready to go\r\n")
+expect_prompt(f"\r\n.*ready to go\r\n")
 send(
     "function add_change --on-variable fish_bind_mode ; set -g MODE_CHANGES $MODE_CHANGES $fish_bind_mode ; end\r"
 )
@@ -42,7 +42,7 @@ send("i")
 sleep(10 if "CI" in os.environ else 1)
 
 send("echo mode changes: $MODE_CHANGES\r")
-expect_prompt(TO_END + "mode changes: default insert default insert\r\n")
+expect_prompt("\r\n.*mode changes: default insert default insert\r\n")
 
 # Regression test for #8125.
 # Control-C should return us to insert mode.
@@ -70,4 +70,4 @@ sleep(timeout * 2)
 
 # We should be back in insert mode now.
 send("echo mode changes: $MODE_CHANGES\r")
-expect_prompt(TO_END + "mode changes: default insert\r\n")
+expect_prompt("\r\n.*mode changes: default insert\r\n")

tests/pexpects/complete.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-from pexpect_helper import SpawnedProc, TO_END
+from pexpect_helper import SpawnedProc
 
 sp = SpawnedProc()
 send, sendline, sleep, expect_prompt, expect_re, expect_str = (
@@ -75,6 +75,6 @@ send("echo fo\t")
 expect_re("foooo")
 send("\x07")
 sendline("echo bar")
-expect_re(TO_END + "bar")
+expect_re("\n.*bar")
 sendline("echo fo\t")
 expect_re("foooo")

tests/pexpects/history.py

@@ -11,7 +11,7 @@
 
 # The history function might pipe output through the user's pager. We don't
 # want something like `less` to complicate matters so force the use of `cat`.
-from pexpect_helper import SpawnedProc, TO_END, TO_END_SUFFIX
+from pexpect_helper import SpawnedProc
 import os
 
 os.environ["PAGER"] = "cat"
@@ -107,16 +107,13 @@ expect_prompt("count hello 0\r\n")
 # delete the first entry matched by the prefix search (the most recent command
 # sent above that matches).
 sendline("history delete -p 'echo hello'")
-expect_re("history delete -p 'echo hello'" + TO_END_SUFFIX)
+expect_re("history delete -p 'echo hello'\r\n")
-expect_re("\\[1\\] echo hello AGAIN" + TO_END_SUFFIX)
+expect_re("\[1\] echo hello AGAIN\r\n")
-expect_re("\\[2\\] echo hello again" + TO_END_SUFFIX)
+expect_re("\[2\] echo hello again\r\n\r\n")
-expect_re("Enter nothing to cancel the delete, or\r\n")
 expect_re(
-    "Enter one or more of the entry IDs or ranges like '5..12', separated by a space.\r\n"
+    "Enter nothing to cancel the delete, or\r\nEnter one or more of the entry IDs or ranges like '5..12', separated by a space.\r\nFor example '7 10..15 35 788..812'.\r\nEnter 'all' to delete all the matching entries.\r\n"
 )
-expect_re("For example '7 10..15 35 788..812'.\r\n")
+expect_re("Delete which entries\? ")
-expect_re("Enter 'all' to delete all the matching entries.\r\n")
-expect_re("Delete which entries\\? ")
 sendline("1")
 expect_prompt('Deleting history entry 1: "echo hello AGAIN"\r\n')
 
@@ -176,7 +173,7 @@ expect_prompt()
 sendline("history clear-session")
 expect_prompt()
 sendline("history search --exact 'echo after' | cat")
-expect_prompt()
+expect_prompt("\r\n")
 
 # Check history filtering
 # We store anything that starts with "echo ephemeral".

tests/pexpects/read.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-from pexpect_helper import SpawnedProc, TO_END
+from pexpect_helper import SpawnedProc
 
 sp = SpawnedProc()
 send, sendline, sleep, expect_prompt, expect_re, expect_str = (
@@ -17,7 +17,7 @@ def expect_read_prompt():
 
 
 def expect_marker(text):
-    expect_prompt(TO_END + "@MARKER:" + str(text) + "@\\r\\n")
+    expect_prompt("\r\n.*@MARKER:" + str(text) + "@\\r\\n")
 
 
 def print_var_contents(varname, expected):

tests/pexpects/status.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-from pexpect_helper import SpawnedProc, TO_END
+from pexpect_helper import SpawnedProc
 
 sp = SpawnedProc()
 send, sendline, sleep, expect_prompt, expect_re, expect_str = (
@@ -22,11 +22,11 @@ expect_prompt("")
 
 # Validate standalone behavior
 sendline("status current-commandline")
-expect_prompt(TO_END + "status current-commandline\r\n")
+expect_prompt("\r\n.*status current-commandline\r\n")
 
 # Validate behavior as part of a command chain
 sendline("true 7 && status current-commandline")
-expect_prompt(TO_END + "true 7 && status current-commandline\r\n")
+expect_prompt("\r\n.*true 7 && status current-commandline\r\n")
 
 # Validate behavior when used in a function
 sendline("function report; set -g last_cmdline (status current-commandline); end")
@@ -34,7 +34,7 @@ expect_prompt("")
 sendline("report 27")
 expect_prompt("")
 sendline("echo $last_cmdline")
-expect_prompt(TO_END + "report 27\r\n")
+expect_prompt("\r\n.*report 27\r\n")
 
 # Exit
 send("\x04")  # <c-d>