bug#61310: Eshell modifying and running output regression

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

On 2/7/2023 10:20 AM, miha--- via Bug reports for GNU Emacs, the Swiss 
army knife of text editors wrote:
> You might also be interested in how comint deals with this. It can
> handle the case when the user yanks something in the middle of output as
> well. See function comint--mark-as-output, bug#3735 and bug#18135 for
> more details.

Good call. Here's a patch that (mostly) copies those comint functions 
over to Eshell.

[-- Attachment #2: 0001-Ensure-that-Eshell-users-can-run-lines-of-command-ou.patch --]
[-- Type: text/plain, Size: 8508 bytes --]

From bd3bc93e00056241968cc6710e0864eafe8d709a Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 5 Feb 2023 21:37:08 -0800
Subject: [PATCH] Ensure that Eshell users can run lines of command output as
 input

Previously, this failed to work properly because any additional input
the user entered would have no 'field' property, confusing
'eshell-get-old-input'.  To fix this, we simply ensure that any
user-entered text in the output field retains said output field
(bug#61310).

* lisp/eshell/esh-util.el (eshell-command-output-properties): New
variable.
(eshell--mark-as-output, eshell--mark-yanked-as-output): New
functions, mostly copied from comint.

* lisp/eshell/esh-proc.el (eshell-interactive-process-filter):
* lisp/eshell/esh-mode.el (eshell-interactive-print): Call
'eshell--mark-as-output'.
(eshell-get-old-input): Remove properties from the returned string
just to be safe.

* test/lisp/eshell/eshell-tests.el (eshell-test-value): New variable.
(eshell-test/get-old-input/rerun-command)
(eshell-test/get-old-input/run-output): New tests.

* test/lisp/eshell/em-prompt-tests.el
(em-prompt-test/field-properties)
(em-prompt-test/field-properties/no-highlight): Use
'eshell-command-output-properties'.
---
 lisp/eshell/esh-mode.el             |  6 ++---
 lisp/eshell/esh-proc.el             |  5 ++---
 lisp/eshell/esh-util.el             | 34 +++++++++++++++++++++++++++++
 test/lisp/eshell/em-prompt-tests.el |  8 +++----
 test/lisp/eshell/eshell-tests.el    | 28 ++++++++++++++++++++++++
 5 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 503d9ba1b63..654e26777e0 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -525,9 +525,7 @@ eshell-goto-input-start
 (defun eshell-interactive-print (string)
   "Print STRING to the eshell display buffer."
   (when string
-    (add-text-properties 0 (length string)
-                         '(field command-output rear-nonsticky (field))
-                         string)
+    (eshell--mark-as-output 0 (length string) string)
     (eshell-interactive-filter nil string)))
 
 (defsubst eshell-begin-on-new-line ()
@@ -891,7 +889,7 @@ eshell-get-old-input
       (let ((inhibit-field-text-motion)
             (end (point)))
         (beginning-of-line)
-        (buffer-substring (point) end)))))
+        (buffer-substring-no-properties (point) end)))))
 
 (defun eshell-copy-old-input ()
   "Insert after prompt old input at point as new input to be edited."
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index 27cd521e82e..a86e7502795 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -24,6 +24,7 @@
 ;;; Code:
 
 (require 'esh-io)
+(require 'esh-util)
 
 (defgroup eshell-proc nil
   "When Eshell invokes external commands, it always does so
@@ -411,9 +412,7 @@ eshell-interactive-process-filter
   "Send the output from PROCESS (STRING) to the interactive display.
 This is done after all necessary filtering has been done."
   (when string
-    (add-text-properties 0 (length string)
-                         '(field command-output rear-nonsticky (field))
-                         string)
+    (eshell--mark-as-output 0 (length string) string)
     (require 'esh-mode)
     (declare-function eshell-interactive-filter "esh-mode" (buffer string))
     (eshell-interactive-filter (if process (process-buffer process)
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 9549e7f1a10..c0685757789 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -132,6 +132,19 @@ eshell-user-names
 (defvar eshell-user-timestamp nil
   "A timestamp of when the user file was read.")
 
+(defvar eshell-command-output-properties
+  `( field command-output
+     front-sticky (field)
+     rear-nonsticky (field)
+     ;; Text inserted by a user in the middle of process output
+     ;; should be marked as output.  This is needed for commands
+     ;; such as `yank' or `just-one-space' which don't use
+     ;; `insert-and-inherit' and thus bypass default text property
+     ;; inheritance.
+     insert-in-front-hooks (,#'eshell--mark-as-output
+                            ,#'eshell--mark-yanked-as-output))
+  "A list of text properties to apply to command output.")
+
 ;;; Obsolete variables:
 
 (define-obsolete-variable-alias 'eshell-host-names
@@ -157,6 +170,27 @@ eshell-condition-case
 	 ,@handlers)
     form))
 
+(defun eshell--mark-as-output (start end &optional object)
+  "Mark the text from START to END as Eshell output.
+OBJECT can be a buffer or string.  If nil, mark the text in the
+current buffer."
+  (with-silent-modifications
+    (add-text-properties start end eshell-command-output-properties
+                         object)))
+
+(defun eshell--mark-yanked-as-output (start end)
+  "Mark yanked text from START to END as Eshell output."
+  ;; `yank' removes the field text property from the text it inserts
+  ;; due to `yank-excluded-properties', so arrange for this text
+  ;; property to be reapplied in the `after-change-functions'.
+  (letrec ((hook
+            (lambda (start1 end1 _len1)
+              (remove-hook 'after-change-functions hook t)
+              (when (and (= start start1)
+                         (= end end1))
+                (eshell--mark-as-output start1 end1)))))
+    (add-hook 'after-change-functions hook nil t)))
+
 (defun eshell-find-delimiter
   (open close &optional bound reverse-p backslash-p)
   "From point, find the CLOSE delimiter corresponding to OPEN.
diff --git a/test/lisp/eshell/em-prompt-tests.el b/test/lisp/eshell/em-prompt-tests.el
index db45e2ae3a7..257549e40fb 100644
--- a/test/lisp/eshell/em-prompt-tests.el
+++ b/test/lisp/eshell/em-prompt-tests.el
@@ -54,8 +54,8 @@ em-prompt-test/field-properties
      (should (equal last-input "echo hello\n"))
      (should (equal-including-properties
               last-output
-              (propertize "hello\n" 'rear-nonsticky '(field)
-                          'field 'command-output))))))
+              (apply #'propertize "hello\n"
+                     eshell-command-output-properties))))))
 
 (ert-deftest em-prompt-test/field-properties/no-highlight ()
   "Check that field properties are properly set on Eshell output/prompts.
@@ -77,8 +77,8 @@ em-prompt-test/field-properties/no-highlight
        (should (equal last-input "echo hello\n"))
        (should (equal-including-properties
                 last-output
-                (propertize "hello\n" 'rear-nonsticky '(field)
-                            'field 'command-output)))))))
+                (apply #'propertize "hello\n"
+                       eshell-command-output-properties)))))))
 
 (ert-deftest em-prompt-test/next-previous-prompt ()
   "Check that navigating forward/backward through old prompts works correctly."
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 776cfb9b92f..743cc28b9b5 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -34,6 +34,8 @@
                            (file-name-directory (or load-file-name
                                                     default-directory))))
 
+(defvar eshell-test-value nil)
+
 ;;; Tests:
 
 (ert-deftest eshell-test/pipe-headproc ()
@@ -160,6 +162,32 @@ eshell-test/get-old-input
      (beginning-of-line))
    (should (string= (eshell-get-old-input) "echo alpha"))))
 
+(ert-deftest eshell-test/get-old-input/rerun-command ()
+  "Test that we can rerun an old command when point is on it."
+  (with-temp-eshell
+   (let ((eshell-test-value "first"))
+     (eshell-match-command-output "echo $eshell-test-value" "first"))
+   ;; Go to the previous prompt.
+   (forward-line -2)
+   (let ((inhibit-field-text-motion t))
+     (end-of-line))
+   ;; Rerun the command, but with a different variable value.
+   (let ((eshell-test-value "second"))
+     (eshell-send-input))
+   (eshell-match-output "second")))
+
+(ert-deftest eshell-test/get-old-input/run-output ()
+  "Test that we can run a line of output as a command when point is on it."
+  (with-temp-eshell
+   (eshell-match-command-output "echo \"echo there\"" "echo there")
+   ;; Go to the output, and insert "hello" after "echo".
+   (forward-line -1)
+   (forward-word)
+   (insert " hello")
+   ;; Run the line as a command.
+   (eshell-send-input)
+   (eshell-match-output "(\"hello\" \"there\")")))
+
 (provide 'eshell-tests)
 
 ;;; eshell-tests.el ends here
-- 
2.25.1