bug#74230: 30.0.92; eshell-emit-prompt clobbers text properties

[Jim Porter <jporterbugs@gmail.com>]

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

On 11/6/2024 10:21 AM, Ashton Wiersdorf wrote:
> In Emacs 29.4, the function did *not* overwrite the text properties of
> the prompt `rear-nonsticky (read-only)`. The new version overwrites the
> rear-nonsticky property so that it is only `(field)`.

Thanks for noticing this.

> If I set `eshell-highlight-prompt' to `t', then I get the read-only
> behavior that I want, but now I cannot get the pleasant component
> highlighting that `fancy-shell' is meant to provide.

If you set the 'face' attribute instead of the 'font-lock-face' 
attribute, you should get the highlighting you want (that's what I do) 
while 'eshell-highlight-prompt' is non-nil.

Here's a patch to fix this though so that we're more careful about not 
clobbering stickiness properties. Eli, what do you think about this 
patch? It fixes a regression from Emacs 29, but the diff is fairly large 
for so late in the Emacs 30 cycle. Unfortunately, I can't think of a 
better solution that doesn't just shuffle the bug around to a different 
spot. On the plus side, this code already has regression tests, and it 
was easy to add a new one for this case.

Personally, I'd be ok with having this be a known bug (there's a 
workaround for Emacs 30) and fixing it on master. But if you think this 
change is ok for the release branch, I'd also be ok with installing 
there; then people don't need to deal with the workaround.

[-- Attachment #2: 0001-Don-t-clobber-stickiness-text-properties-when-printi.patch --]
[-- Type: text/plain, Size: 5857 bytes --]

From 8b8e4830f45e69329018bc87121e12ee6ddfb9d4 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 7 Nov 2024 10:08:33 -0800
Subject: [PATCH] Don't clobber stickiness text properties when printing Eshell
 prompt

* lisp/eshell/em-prompt.el (eshell--append-text-property): New
function...
(eshell-emit-prompt): ... use it.

* test/lisp/eshell/em-prompt-tests.el
(em-prompt-test/field-properties/merge-stickiness): New test.
(em-prompt-test/field-properties, em-prompt-test/after-failure): Reorder
stickiness values (bug#74230).
---
 lisp/eshell/em-prompt.el            | 36 ++++++++++++++++++-----------
 test/lisp/eshell/em-prompt-tests.el | 35 ++++++++++++++++++++++++----
 2 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index de62b5c7d97..37970ac0ba5 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -119,6 +119,19 @@ eshell-prompt-initialize
     (add-hook 'eshell-post-command-hook 'eshell-emit-prompt nil t)
     (eshell-prompt-mode)))
 
+(defun eshell--append-text-property (start end prop value &optional object)
+  "Append to a text property from START to END.
+PROP is the text property to append to, and VALUE is the list of
+property values to append.  OBJECT is the object to propertize, as with
+`put-text-property' (which see)."
+  (let (next)
+    (while (< start end)
+      (setq next (next-single-property-change start prop object end))
+      (put-text-property start next prop
+                         (append (get-text-property start prop object) value)
+                         object)
+      (setq start next))))
+
 (defun eshell-emit-prompt ()
   "Emit a prompt if eshell is being used interactively."
   (when (boundp 'ansi-color-context-region)
@@ -126,19 +139,16 @@ eshell-emit-prompt
   (run-hooks 'eshell-before-prompt-hook)
   (if (not eshell-prompt-function)
       (set-marker eshell-last-output-end (point))
-    (let ((prompt (funcall eshell-prompt-function)))
-      (add-text-properties
-       0 (length prompt)
-       (if eshell-highlight-prompt
-           '( read-only t
-              field prompt
-              font-lock-face eshell-prompt
-              front-sticky (read-only field font-lock-face)
-              rear-nonsticky (read-only field font-lock-face))
-         '( field prompt
-            front-sticky (field)
-            rear-nonsticky (field)))
-       prompt)
+    (let* ((prompt (funcall eshell-prompt-function))
+           (len (length prompt))
+           (sticky-props '(field)))
+      (put-text-property 0 len 'field 'prompt prompt)
+      (when eshell-highlight-prompt
+        (add-text-properties
+         0 len '(read-only t font-lock-face eshell-prompt) prompt)
+        (setq sticky-props `(read-only font-lock-face . ,sticky-props)))
+      (eshell--append-text-property 0 len 'front-sticky sticky-props prompt)
+      (eshell--append-text-property 0 len 'rear-nonsticky sticky-props prompt)
       (eshell-interactive-filter nil prompt)))
   (run-hooks 'eshell-after-prompt-hook))
 
diff --git a/test/lisp/eshell/em-prompt-tests.el b/test/lisp/eshell/em-prompt-tests.el
index fbadade061f..1c6e8e02293 100644
--- a/test/lisp/eshell/em-prompt-tests.el
+++ b/test/lisp/eshell/em-prompt-tests.el
@@ -57,8 +57,8 @@ em-prompt-test/field-properties
                'read-only t
                'field 'prompt
                'font-lock-face 'eshell-prompt
-               'front-sticky '(read-only field font-lock-face)
-               'rear-nonsticky '(read-only field font-lock-face))))
+               'front-sticky '(read-only font-lock-face field)
+               'rear-nonsticky '(read-only font-lock-face field))))
      (should (equal last-input "echo hello\n"))
      (should (equal-including-properties
               last-output
@@ -88,6 +88,33 @@ em-prompt-test/field-properties/no-highlight
                 (apply #'propertize "hello\n"
                        eshell-command-output-properties)))))))
 
+(ert-deftest em-prompt-test/field-properties/merge-stickiness ()
+  "Check that stickiness properties are properly merged on Eshell prompts."
+  (let ((eshell-prompt-function
+         (lambda ()
+           (concat (propertize (eshell/pwd) 'front-sticky '(front))
+                   (propertize "$ " 'rear-nonsticky '(rear))))))
+    (with-temp-eshell
+     (eshell-insert-command "echo hello")
+     (let ((last-prompt (field-string (1- eshell-last-input-start))))
+       (should (equal-including-properties
+                last-prompt
+                (concat
+                 (propertize
+                  (directory-file-name default-directory)
+                  'read-only t
+                  'field 'prompt
+                  'font-lock-face 'eshell-prompt
+                  'front-sticky '(front read-only font-lock-face field)
+                  'rear-nonsticky '(read-only font-lock-face field))
+                 (propertize
+                  "$ "
+                  'read-only t
+                  'field 'prompt
+                  'font-lock-face 'eshell-prompt
+                  'front-sticky '(read-only font-lock-face field)
+                  'rear-nonsticky '(rear read-only font-lock-face field)))))))))
+
 (ert-deftest em-prompt-test/after-failure ()
   "Check that current prompt shows the exit code of the last failed command."
   (with-temp-eshell
@@ -104,8 +131,8 @@ em-prompt-test/after-failure
                'read-only t
                'field 'prompt
                'font-lock-face 'eshell-prompt
-               'front-sticky '(read-only field font-lock-face)
-               'rear-nonsticky '(read-only field font-lock-face)))))))
+               'front-sticky '(read-only font-lock-face field)
+               'rear-nonsticky '(read-only font-lock-face field)))))))
 
 \f
 ;; Prompt navigation
-- 
2.25.1