bug#49484: 27.2; [PATCH] Undoing a 'RET' in comint and eshell

[miha]

1) M-x shell
2) echo foo RET
3) C-/ to undo this 'RET'
   The buffer now contain the shell's prompt and "echo foo".
   However, the process mark is located at eob after "echo foo"
4) Type bar
   The buffer now contain the shell's prompt and "echo foobar"
5) RET
   Shell will output "bar: command not found", because the process mark
   is located before "bar" after "foo".

Similar behaviour can be observed with C-c SPC (comint-accumulate) and
with eshell.

My idea to solve this is to record process mark and related marker
positions as `apply' entries in the undo list. Attached patch implements
this for comint and eshell.

>From fde3b5ce8964e001a9019feff83e267b2cf367dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <miha@kamnitnik.top>
Date: Fri, 9 Jul 2021 10:57:11 +0200
Subject: [PATCH] Improve undo in comint and eshell

* lisp/simple.el (marker-record-undo): New function.
* etc/NEWS:
* doc/lispref/markers.texi (Moving Markers): Document it.
* lisp/comint.el (comint-send-input):
(comint-accumulate):
(comint-set-process-mark):
* lisp/eshell/esh-mode.el (eshell-reset):
(eshell-update-markers): Use it to record adjustments to various
marker positions in undo list.
---
 doc/lispref/markers.texi | 17 +++++++++++++++++
 etc/NEWS                 |  5 +++++
 lisp/comint.el           | 14 ++++++++++----
 lisp/eshell/esh-mode.el  | 15 ++++++++++-----
 lisp/simple.el           | 18 ++++++++++++++++++
 5 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/doc/lispref/markers.texi b/doc/lispref/markers.texi
index 80f79b67e5..b0c454be8d 100644
--- a/doc/lispref/markers.texi
+++ b/doc/lispref/markers.texi
@@ -395,6 +395,23 @@ Moving Markers
 
 @defun move-marker marker position &optional buffer
 This is another name for @code{set-marker}.
+@end defun
+
+  Function @code{set-marker} does not record marker movement in the
+undo list.  Before moving a marker, you can explicitly record its
+original position as an undo list entry with
+@code{marker-record-undo}.
+
+@defun marker-record-undo &rest markers
+This function records the current position and buffer of each marker
+in MARKERS as an entry in the undo list.  Undoing it will relocate
+these markers to point back to their recorded positions.  Passing
+markers that currently point nowhere is allowed and undoing will
+simply make them point nowhere again.
+
+Undo in region will always ignore entries made with this function.
+Also, this function doesn't do anything if undo is disabled in the
+current buffer.
 @end defun
 
 @node The Mark
diff --git a/etc/NEWS b/etc/NEWS
index da5524a555..2e0e7abc47 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2945,6 +2945,11 @@ The former is now declared obsolete.
 
 * Lisp Changes in Emacs 28.1
 
++++
+** New function 'marker-record-undo'.
+To make marker movement undoable, use this function to store a
+marker's current position in the undo list before moving the marker.
+
 ---
 *** ':safe' settings in 'defcustom' are now propagated to the loaddefs files.
 
diff --git a/lisp/comint.el b/lisp/comint.el
index 9e406614b9..f464ecbbe4 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1931,9 +1931,12 @@ comint-send-input
         (setq comint-input-ring-index nil)
         ;; Update the markers before we send the input
         ;; in case we get output amidst sending the input.
+        (marker-record-undo
+         pmark comint-last-input-start comint-last-input-end
+         comint-accum-marker)
         (set-marker comint-last-input-start pmark)
         (set-marker comint-last-input-end (point))
-        (set-marker (process-mark proc) (point))
+        (set-marker pmark (point))
         ;; clear the "accumulation" marker
         (set-marker comint-accum-marker nil)
         (let ((comint-input-sender-no-newline no-newline))
@@ -3490,6 +3493,7 @@ comint-accumulate
 when you send it."
   (interactive)
   (insert "\n")
+  (marker-record-undo comint-accum-marker)
   (set-marker comint-accum-marker (point))
   (if comint-input-ring-index
       (setq comint-save-input-ring-index
@@ -3525,9 +3529,11 @@ comint-bol-or-process-mark
 (defun comint-set-process-mark ()
   "Set the process mark at point."
   (interactive)
-  (let ((proc (or (get-buffer-process (current-buffer))
-                 (user-error "Current buffer has no process"))))
-    (set-marker (process-mark proc) (point))
+  (let* ((proc (or (get-buffer-process (current-buffer))
+                   (user-error "Current buffer has no process")))
+         (pmark (process-mark proc)))
+    (marker-record-undo pmark)
+    (set-marker pmark (point))
     (message "Process mark set")))
 
 
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index f9dbce9770..9aa00016c0 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -534,11 +534,14 @@ eshell-reset
   "Output a prompt on a new line, aborting any current input.
 If NO-HOOKS is non-nil, then `eshell-post-command-hook' won't be run."
   (goto-char (point-max))
-  (setq eshell-last-input-start (point-marker)
-       eshell-last-input-end (point-marker)
-       eshell-last-output-start (point-marker)
-       eshell-last-output-block-begin (point)
-       eshell-last-output-end (point-marker))
+  (marker-record-undo
+   eshell-last-input-start eshell-last-input-end
+   eshell-last-output-start eshell-last-output-end)
+  (set-marker eshell-last-input-start (point))
+  (set-marker eshell-last-input-end (point))
+  (set-marker eshell-last-output-start (point))
+  (set-marker eshell-last-output-end (point))
+  (setq eshell-last-output-block-begin (point))
   (eshell-begin-on-new-line)
   (unless no-hooks
     (run-hooks 'eshell-post-command-hook)
@@ -568,6 +571,8 @@ eshell-parse-command-input
 
 (defun eshell-update-markers (pmark)
   "Update the input and output markers relative to point and PMARK."
+  (marker-record-undo eshell-last-input-start eshell-last-input-end
+                      eshell-last-output-end)
   (set-marker eshell-last-input-start pmark)
   (set-marker eshell-last-input-end (point))
   (set-marker eshell-last-output-end (point)))
diff --git a/lisp/simple.el b/lisp/simple.el
index f746d738a6..337cfe6234 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -3466,6 +3466,24 @@ undo-adjust-pos
             ;; comments.
             (max (car d) (- pos (cdr d)))))))
 
+(defun marker-record-undo (&rest markers)
+  "Record positions of MARKERS in the undo list.
+Undoing this entry will make each marker in MARKERS point to its
+recorded position and buffer, or nowhere if it currently points
+nowhere.  Undo in region will always ignore these entries.
+
+If undo is disabled in the current buffer, this function does
+nothing."
+  (let ((undo-list buffer-undo-list))
+    (unless (eq undo-list t)
+      (dolist (marker markers)
+        (push (list 'apply #'set-marker marker
+                    (marker-position marker) (marker-buffer marker))
+              undo-list))
+      (setq buffer-undo-list
+            `((apply ,#'marker-record-undo ,@markers)
+              ,@undo-list)))))
+
 ;; Return the first affected buffer position and the delta for an undo element
 ;; delta is defined as the change in subsequent buffer positions if we *did*
 ;; the undo.
-- 
2.32.0