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

[Phil Sainty]

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