Subject: Re: bug#18135: bug#3735: shell-mode editing, movement, and command entry broken by prompt/output changes

From: <[email-masked]>
Date: Tue, 07 Dec 2021 17:57:34 +0100

Lars Ingebrigtsen <[email-masked]> writes:

> [email-masked] writes:
>
>> Reproducer:
>>
>> - copy " -la" into kill ring
>> - type "echo ls" into M-x shell
>> - (Shell gives output line "ls".)
>> - Go up to output line "ls" (C-p).
>> - yank " -la"
>> - press C-b
>> - the line now "ls -la" with point after "l"
>> - press RET
>>
>> Only " -la" is sent to the process instead of "ls -la"
>
> Thanks; with that I'm also able to reproduce the problem.
>
>> I am willing to write up a patch to fix this. I'm thinking of having
>> comint-mode register a function in after-change-functions to mark text
>> inserted in the 'output' field as 'output'. Would this be okay or are
>> there any obvious flaws with this approach?
>
> I think that sounds like a promising approach, but I wonder whether
> something could be done with rear-nonsticky here. `field' isn't
> rear-sticky here for probably good reasons, but perhaps something
> could be tweaked there? (I haven't actually tried, though.)

Yeah I thought about that too. However, commands such as 'yank' or
'just-one-space' use insert rather than insert-and-inherit and thus
always bypass inheritance of surrounding text properties regardless of
it's stickiness. That is why I settled for using the
'insert-in-front-hooks' text property, patch attached.

From 7914fe85ba096f93d4b6817bc0d7b7976f3d316a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <[email-masked]>
Date: Tue, 7 Dec 2021 16:59:39 +0100
Subject: [PATCH] Improve yanking in the middle of comint process output

* lisp/comint.el
(comint--unmark-string-as-output): New function to remove unwanted
properties from text yanked from comint buffers.

(comint-mode): Use it as a 'filter-buffer-substring-function'.

(comint-output-filter): Set 'insert-in-front-hooks' text property on
process output such that text yanked by the user in the middle of
process output is marked as process output (Bug#3735).

(comint--mark-as-output): New function.
(comint--mark-yanked-as-output): New function.
---
 lisp/comint.el | 54 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 544f0b8b82..5f99f560cf 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -730,6 +730,8 @@ comint-mode
               (or (file-remote-p default-directory) ""))
   (setq-local comint-accum-marker (make-marker))
   (setq-local font-lock-defaults '(nil t))
+  (add-function :filter-return (local 'filter-buffer-substring-function)
+                #'comint--unmark-string-as-output)
   (add-hook 'change-major-mode-hook 'font-lock-defontify nil t)
   (add-hook 'isearch-mode-hook 'comint-history-isearch-setup nil t)
   (add-hook 'completion-at-point-functions 'comint-completion-at-point nil t)
@@ -1815,7 +1817,8 @@ comint-add-to-input-history
     (ring-insert comint-input-ring cmd)))
 
 (defconst comint--prompt-rear-nonsticky
-  '(field inhibit-line-move-field-capture read-only font-lock-face)
+  '( field inhibit-line-move-field-capture read-only font-lock-face
+     insert-in-front-hooks)
   "Text properties we set on the prompt and don't want to leak past it.")
 
 (defun comint-send-input (&optional no-newline artificial)
@@ -2152,14 +2155,7 @@ comint-output-filter
 	    (goto-char (process-mark process)) ; In case a filter moved it.
 
 	    (unless comint-use-prompt-regexp
-              (with-silent-modifications
-                (add-text-properties comint-last-output-start (point)
-                                     `(rear-nonsticky
-				       ,comint--prompt-rear-nonsticky
-				       front-sticky
-				       (field inhibit-line-move-field-capture)
-				       field output
-				       inhibit-line-move-field-capture t))))
+              (comint--mark-as-output comint-last-output-start (point)))
 
 	    ;; Highlight the prompt, where we define `prompt' to mean
 	    ;; the most recent output that doesn't end with a newline.
@@ -2191,6 +2187,46 @@ comint-output-filter
 	                             ,comint--prompt-rear-nonsticky)))
 	    (goto-char saved-point)))))))
 
+(defun comint--mark-as-output (beg end)
+  (with-silent-modifications
+    (add-text-properties
+     beg end
+     `(rear-nonsticky
+       ,comint--prompt-rear-nonsticky
+       front-sticky
+       (field inhibit-line-move-field-capture)
+       field output
+       inhibit-line-move-field-capture t
+       ;; 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
+       (,#'comint--mark-as-output ,#'comint--mark-yanked-as-output)))))
+
+(defun comint--mark-yanked-as-output (beg end)
+  ;; `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'.
+  (let (fun)
+    (setq
+     fun
+     (lambda (beg1 end1 _len1)
+       (remove-hook 'after-change-functions fun t)
+       (when (and (= beg beg1)
+                  (= end end1))
+         (comint--mark-as-output beg1 end1))))
+    (add-hook 'after-change-functions fun nil t)))
+
+(defun comint--unmark-string-as-output (string)
+  (remove-list-of-text-properties
+   0 (length string)
+   '( rear-nonsticky front-sticky field
+      inhibit-line-move-field-capture insert-in-front-hooks)
+   string)
+  string)
+
 (defun comint-preinput-scroll-to-bottom ()
   "Go to the end of buffer in all windows showing it.
 Movement occurs if point in the selected window is not after the process mark,
-- 
2.34.1