bug#61310: Eshell modifying and running output regression

[Jim Porter <jporterbugs@gmail.com>]

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

On 2/5/2023 8:18 PM, Morgan Smith wrote:
> eshell allows you to modify the output of a command and run it.  It is a
> beautiful thing.

Interesting. I didn't realize this was possible in Eshell (I only 
thought you could re-run old *inputs*).

> I don't know much about fields but it looks like it is not necessary to
> have a command-output field at all.  I propose we get rid of that.  If
> you want to keep the field, can we make it sticky or something? 

The output field is actually necessary (or else Eshell would need to be 
cleverer about some things). The main issue is that if a command doesn't 
output a newline, the command's output can end up on the same line as 
the prompt:

   ~ $ *echo -n [output]
   [output]~ $

If the output had no field, C-a would move to the very beginning of the 
line, not to the beginning of the input field. Maybe this is a bug in 
how fields are handled, but changing field handling in general is 
probably too risky.

So instead, let's make the output field sticky as you say. Here's a fix 
for that plus regression tests so this won't break in the future.

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

From a0f49108386500f66e55a605497dee94dbd788d6 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...
* lisp/eshell/esh-proc.el (eshell-interactive-process-filter):
* lisp/eshell/esh-mode.el (eshell-interactive-print): ... use it.
(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.
---
 lisp/eshell/esh-mode.el          |  5 ++---
 lisp/eshell/esh-proc.el          |  4 ++--
 lisp/eshell/esh-util.el          |  4 ++++
 test/lisp/eshell/eshell-tests.el | 28 ++++++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 503d9ba1b63..e7ccf2b3aef 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -526,8 +526,7 @@ eshell-interactive-print
   "Print STRING to the eshell display buffer."
   (when string
     (add-text-properties 0 (length string)
-                         '(field command-output rear-nonsticky (field))
-                         string)
+                         eshell-command-output-properties string)
     (eshell-interactive-filter nil string)))
 
 (defsubst eshell-begin-on-new-line ()
@@ -891,7 +890,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..02de619864a 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
@@ -412,8 +413,7 @@ eshell-interactive-process-filter
 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-command-output-properties 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..fc0aee22f33 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -132,6 +132,10 @@ 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))
+  "A list of text properties to apply to command output.")
+
 ;;; Obsolete variables:
 
 (define-obsolete-variable-alias 'eshell-host-names
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 776cfb9b92f..f6a688b1b56 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-and-inherit " 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