On 2022-10-20 18:36, Eli Zaretskii wrote: >> Date: Thu, 20 Oct 2022 11:14:50 +1300 >> From: Phil Sainty >> 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. > > It could be, but maybe looking at the Git history of that code will > tell you why we have that part there? I mean, maybe there are use > cases where that is important? > > If nothing comes up, I think you are right, and that move should be > removed. On a GUI frame, a middle click leaves point _at_the_end_ of > the inserted text, not where I click. I don't have my head around all of this, but I've done some digging... The call to `mouse-set-point' in `term-mouse-paste' dates back to its origins in commit 4060e6ee3b796ecac506b1ca54217b100795d73a (by RMS) dated 1994, and then there are a couple of subsequent commits of interest: commit aade4ab28e774bc2d74a6567aae24e805f30e78a by Per Bothner in 1995 removed the call to `mouse-set-point' (amongst other changes), and then in 2004 Richard explicitly added it back again: commit 2d7502b55b52b9668c2c920f1bfa5bd437fb3b96 Author: Richard M. Stallman Date: Fri Jan 30 16:53:11 2004 +0000 (term-mouse-paste): Call mouse-set-point. Richard, I've CC'd you on this basis, as it's not clear to me why the `term-mouse-paste' command (which I would expect to do nothing besides send the primary selection text to the inferior process) should be setting point. Do you recall the use-case for that? (n.b. I've attached the above-mentioned commits to this email.) > But if we aren't sure, it's okay to momentarily disable > select-active-regions here, we just need a comment with the > explanation you wrote above. I'm certainly unsure of much of this. I can add that bug#2449 from 2009 is where (deactivate-mark) in `term-send-raw-string' originates, so I think that will be when the `term-mouse-paste' problem I've reported was effectively introduced, as it's the `deactivate-mark' call which causes the primary selection to be updated. The commits for that were: commit c5f894821d04f6fbaf3b8c62308692439ec598d3 2009-03-08 19:33 commit 212bb1a81ade52dcbcbb1bceb680e3e0e0b6264c 2009-03-08 19:37 commit 1cb6d11e13c5e28f96558b85f15344abd56f5e18 2009-03-13 01:43 I haven't read through that bug report, but as `deactivate-mark' was the solution to that, I don't imagine removing that call would be a viable thing to do now, so let-binding `select-active-regions' still feels like a reasonable option for the current problem. >> 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. > > Indeed, we don't by default treat middle click as yank on GUI frames. > So maybe you are right -- but please note that there are some user > options which perhaps do cause the middle click to be treated as yank > as optional behavior, in which case they should do the same in the > term case. This code also dates back to the original implementation of `term-mouse-paste', and bug#6845 from 2010 seems notable for this part. That bug was about `term-mouse-paste' pasting the current kill instead of the primary selection. I don't believe the changes for that are relevant to the *main* problem I've reported, but when the command was pasting from the kill ring the (setq this-command 'yank) call would have made sense, so I suspect this call should have been removed in commit ff98b2dd51e84b812e061859fa8c682d22b2e459 ? All in all: * The (deactivate-mark) in `term-send-raw-string' seems important. * I don't understand the `mouse-set-point' call, but it may be wanted. * Disabling `select-active-regions' in `term-mouse-paste' still seems like the way to go if point is being set (or maybe regardless). * (setq this-command 'yank) looks like an old remnant (now a bug). -Phil (not currently subscribed to the lists; please keep me CC'd)