bug#58608: 29.0.50; Nasty bug with pasting primary selection in term buffers

[Phil Sainty <psainty@orcon.net.nz>]

On 2022-10-20 00:05, Eli Zaretskii wrote:
> From: Phil Sainty <psainty@orcon.net.nz>
>> Looking at the docstring for `deactivate-mark' told me about
>> `select-active-regions'.  I don't know whether this is the correct
>> solution, but I can confirm that this change to `term-mouse-paste'
>> appears (after only cursory testing) to fix the bug:
>> 
>> -  (term-send-raw-string (gui-get-primary-selection)))
>> +  (let ((select-active-regions nil))
>> +    (term-send-raw-string (gui-get-primary-selection))))
> 
> I think it could be important to understand why select-active-regions
> causes this problem in your case.

I think mechanically it's because the middle click moves point to the
click position, and the call to `deactivate-mark' then causes the
primary selection to be updated based on the current point and mark
(unless we mess with select-active-regions).

You've made me wonder, though... this command is intended only for
term char mode, so should a middle click *really* be setting point?
If all we're trying to do is send the selection text to the inferior
process, that bit might be wrong.

There's an explicit (mouse-set-point click) there, but if I comment
out both that and my interim binding of `select-active-regions' then
this also seems to do the right thing (once more with only very cursory
testing).

(defun term-mouse-paste (click)
   "Insert the primary selection at the position clicked on."
   (interactive "e")
   ;; Give temporary modes such as isearch a chance to turn off.
   (run-hooks 'mouse-leave-buffer-hook)
   (setq this-command 'yank)
   ;; (mouse-set-point click)
   ;; (let ((select-active-regions nil))
   (term-send-raw-string (gui-get-primary-selection)))
   ;;)

That's more of a change, but perhaps it's the correct thing to do.

I'm now looking at that (setq this-command 'yank) as well, and
wondering whether it's important for anything under the impression
that a `yank' just happened to also see point at the location of
the yank.  I'm not sure whether a middle click in a terminal to
send the primary selection directly to the inferior process *should*
be treated as `yank' though -- maybe that code is also wrong.

These changes are more nebulous to me, as they represent more of
a functional change than binding `select-active-regions' does.


-Phil