bug#8996: Set PRIMARY from last selection, not last selected window

bug#8996: Set PRIMARY from last selection, not last selected window

[Stefan Monnier]

With focus-follows-mouse (and/or mouse-autoselect-window), I often get
surprising results in my PRIMARY.  E.g.:

1- in window/frame 1 I select a piece of text "test" with the mouse.
2- I move the mouse to an xterm
3- I middle click, and lo and behold rather than "test" I see some
   unrelated selection inserted.

What happened is that at step 2 I happened to move the mouse over some
other Emacs window which happens to also have an active region, and my
focus-follows-mouse window manager (and/or mouse-autoselect-window
setting) caused Emacs to temporarily select this other region, changing
the PRIMARY selection from the "test" I just selected to something
completely different.

For the last few months I attributed this "erratic middle-click in xterm"
to the fact that I'm not used to have to distinguish the PRIMARY from
the CLIPBOARD, and only yesterday did I figure out where it's coming from.


        Stefan

  


In GNU Emacs 24.0.50.1 (i686-pc-linux-gnu, GTK+ Version 2.24.4)
 of 2011-06-25 on pastel
Windowing system distributor `The X.Org Foundation', version 11.0.11002000
configured using `configure  'CFLAGS=-Wall -Wno-pointer-sign -DUSE_LISP_UNION_TYPE -DSYNC_INPUT -DENABLE_CHECKING -DXASSERTS -DFONTSET_DEBUG -g -O0 -I/usr/include/GNUstep''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: fr_CH.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: InactiveMinibuffer

Minor modes in effect:
  shell-dirtrack-mode: t
  diff-auto-refine-mode: t
  electric-pair-mode: t
  electric-indent-mode: t
  url-handler-mode: t
  global-reveal-mode: t
  reveal-mode: t
  auto-insert-mode: t
  savehist-mode: t
  minibuffer-electric-default-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-x v = <switch-frame> <down-mouse-1> <mouse-movement> 
<mouse-1> <backspace> <backspace> C-x C-s <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> C-x C-w 
C-g C-g C-x v = C-x h M-w <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> C-s 1 7 1 0 C-s C-a <left> <right> <up> 
<left> <right> <down> <left> <right> <right> <right> 
<right> <right> <right> <right> <right> <right> <right> 
<right> <right> <right> <right> <right> <right> <right> 
<right> M-d <down> <down> <down> <down> <down> <up> 
<up> <up> <up> <up> <up> a l t <down> <down> <down> 
<down> <down> <left> <left> <left> C-d C-e <left> <left> 
<left> <left> <left> <left> C-d C-a C-x C-s <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> M-x r 
e p o - e m - b u <tab> <return>

Recent messages:
Warning: special-display-p is obsolete!
Warning: same-window-p is obsolete!
Finding changes in /home/monnier/src/emacs/work/src/textprop.c...done
Mark set [2 times]
Warning: interactive-p is obsolete!
Mark set [5 times]
Mark saved where search started
Saving file /home/monnier/src/emacs/work/src/textprop.c...
Wrote /home/monnier/src/emacs/work/src/textprop.c
Warning: interactive-p is obsolete!

Load-path shadows:
/usr/share/emacs23/site-lisp/bbdb/bbdb hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb
/usr/share/emacs23/site-lisp/bbdb/bbdb-mhe hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-mhe
/usr/share/emacs23/site-lisp/bbdb/bbdb-gnus hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-gnus
/usr/share/emacs23/site-lisp/bbdb/bbdb-migrate hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-migrate
/usr/share/emacs23/site-lisp/bbdb/bbdb-sc hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-sc
/usr/share/emacs23/site-lisp/bbdb/bbdb-snarf hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-snarf
/usr/share/emacs23/site-lisp/bbdb/bbdb-w3 hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-w3
/usr/share/emacs23/site-lisp/bbdb/bbdb-gui hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-gui
/usr/share/emacs23/site-lisp/bbdb/bbdb-print hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-print
/usr/share/emacs23/site-lisp/bbdb/bbdb-rmail hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-rmail
/usr/share/emacs23/site-lisp/bbdb/bbdb-vm hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-vm
/usr/share/emacs23/site-lisp/bbdb/bbdb-ftp hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-ftp
/usr/share/emacs23/site-lisp/bbdb/bbdb-merge hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-merge
/usr/share/emacs23/site-lisp/bbdb/bbdb-whois hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-whois
/usr/share/emacs23/site-lisp/bbdb/bbdb-com hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-com
/usr/share/emacs23/site-lisp/bbdb/bbdb-hooks hides /usr/share/emacs/site-lisp/bbdb/lisp/bbdb-hooks

Features:
(mail-extr message sendmail format-spec rfc822 mml mml-sec mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums
mailabbrev mail-utils mailheader emacsbug sort network-stream starttls
tls mpc gud doc-view jka-compr image-mode dired completion vc-annotate
shell pcomplete grep compile hideif cpp cmacexp cc-mode cc-fonts
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs dabbrev
cal-china lunar solar cal-dst cal-bahai cal-islam cal-hebrew holidays
hol-loaddefs cal-french diary-lib diary-loaddefs mule-util cal-move
cal-menu calendar cal-loaddefs smerge-mode octave-mod skeleton derived
smie cl-specs log-view log-edit pcvs-util vc-sccs vc-svn vc-cvs vc-rcs
vc-dir add-log whitespace diff-mode vc ediff-merg ediff-diff ediff-wind
ediff-help ediff-util ediff-mult ediff-init ediff vc-dispatcher
executable copyright multi-isearch xscheme trace testcover scheme
unsafep re-builder shadow inf-lisp ielm pp comint ring gmm-utils ert
find-func ewoc debug elp edebug cust-print cus-edit cus-start cus-load
wid-edit vc-bzr filecache server noutline outline easy-mmode flyspell
ispell eldoc checkdoc regexp-opt thingatpt help-mode easymenu view
prog-mode electric url-handlers url-parse auth-source warnings eieio
byte-opt bytecomp byte-compile cconv macroexp assoc gnus-util time-date
password-cache url-vars mm-util mail-prsvr reveal autoinsert uniquify
advice help-fns advice-preload savehist minibuf-eldef disp-table cl
cl-loaddefs proof-site proof-autoloads pg-vars bbdb-autoloads agda2
tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar
dnd fontset image fringe lisp-mode register page newcomment menu-bar
rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax
font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan
thai tai-viet lao korean japanese hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer loaddefs button faces
cus-face files text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget hashtable-print-readable backquote
make-network-process dbusbind dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 04/07/11 18:19, Stefan Monnier wrote:
> With focus-follows-mouse (and/or mouse-autoselect-window), I often get
> surprising results in my PRIMARY.  E.g.:
>
> 1- in window/frame 1 I select a piece of text "test" with the mouse.
> 2- I move the mouse to an xterm
> 3- I middle click, and lo and behold rather than "test" I see some
>     unrelated selection inserted.
>
> What happened is that at step 2 I happened to move the mouse over some
> other Emacs window which happens to also have an active region, and my
> focus-follows-mouse window manager (and/or mouse-autoselect-window
> setting) caused Emacs to temporarily select this other region, changing
> the PRIMARY selection from the "test" I just selected to something
> completely different.
>

Erk. Sorry, yes, this AFAIK sounds like a known (to some people...) 
problem that is apparently still present in at least some circumstances. 
I actually meant to check if it was still occurring after the 
customization vs. binding thing was decided and if so file it as a bug.

I had previously mentioned the subtlety under #6774 (at least) [1], and 
ISTR Chong Yidong also being aware of it, though I'm having some trouble 
finding the relevant email, so hopefully I'm not unjustly saying that.

A fair bit of the implementation has AFAIK changed since #6774, but I 
guess the issue was carried forward.


[1]
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6774#32

"""
Try it between two kate windows both with selected text, say - note how
the selection doesn't change depending on which window you're currently
in, it depends on the last text the user actively selected.
"""

bug#8996: Set PRIMARY from last selection, not last selected window

[Chong Yidong]

David De La Harpe Golden <david@harpegolden.net> writes:

> Erk. Sorry, yes, this AFAIK sounds like a known (to some people...)
> problem that is apparently still present in at least some
> circumstances. I actually meant to check if it was still occurring
> after the customization vs. binding thing was decided and if so file
> it as a bug.
>
> I had previously mentioned the subtlety under #6774 (at least) [1],
> and ISTR Chong Yidong also being aware of it, though I'm having some
> trouble finding the relevant email, so hopefully I'm not unjustly
> saying that.

I can't reproduce this bug with Metacity with focus-follows-mouse on.

I'm surprised this issue has reappeared; the whole point of the current
active selection implementation is that Emacs grabs the primary
selection only after executing a command (in the command loop).  Simply
switching to an Emacs frame should not trigger it---could it be that
your Emacs is customized to do something funky with the command loop?

bug#8996: Set PRIMARY from last selection, not last selected window

[Stefan Monnier]

>> Erk. Sorry, yes, this AFAIK sounds like a known (to some people...)
>> problem that is apparently still present in at least some
>> circumstances. I actually meant to check if it was still occurring
>> after the customization vs. binding thing was decided and if so file
>> it as a bug.
>> 
>> I had previously mentioned the subtlety under #6774 (at least) [1],
>> and ISTR Chong Yidong also being aware of it, though I'm having some
>> trouble finding the relevant email, so hopefully I'm not unjustly
>> saying that.

> I can't reproduce this bug with Metacity with focus-follows-mouse on.
> I'm surprised this issue has reappeared; the whole point of the current
> active selection implementation is that Emacs grabs the primary
> selection only after executing a command (in the command loop).  Simply
> switching to an Emacs frame should not trigger it---could it be that
> your Emacs is customized to do something funky with the command loop?

Here's my recipe with no funky anything (just plain old trunk):

% trunk/src/emacs -Q
C-x 2
M-: (setq mouse-autoselect-window t) RET
Select a word with a double-mouse-1 click on a word in one of the two windows.
Move the mouse over the other window and then outside Emacs frame to an xterm.
Hit mouse-2 in the xterm.

In my case, the mouse-2 does not paste the selected word but the other
part of the buffer that happened to be the selected region in the
other window.

The window manager is ctwm rather than Metacity, in case it matters.


        Stefan

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 05/07/11 04:26, Stefan Monnier wrote:

> The window manager is ctwm rather than Metacity, in case it matters.
>
It may be relevant - focus-follows-mouse may be implemented somewhat 
gesturally in some window managers, i.e. you may have to dwell a 
fraction of a second in a window before focus actually shifts, even if 
you don't have to click to focus, so that "just passing through" windows 
get skipped.

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 05/07/11 04:26, Stefan Monnier wrote:


> The window manager is ctwm rather than Metacity, in case it matters.
>

I can replicate this (not just with ctwm)

The issue I was thinking of is still present too, and is likely
closely related / really the same thing.

emacs -Q

C-x 2

point is at end of scratch buffer in the window you're in.

Now select word "create" with M-SPC and arrow keys, mark at start of 
word, point at end of word.

C-x o

create to the end of buffer is highlighted in current window.*

select word "visit" in window you're now in with M-SPC and arrow keys, 
mark at start of word, point at end of word.

C-x o

Note " a file, " is highlighted.*

hit mouse-2 on the end of the word "enter" in current window.

++ " a file, " is inserted, whereas user expectation is probably 
"visit", as the most recently actively selected by the user.

If we made it insert "visit", and (unlikely) the user did actually want 
" a file, ", they could hit C-x C-x [C-x C-x], say, to make that region 
the last actively selected by the user.


* Notionally pretty separate issue, best considered a separate bug or 
wishlist item: Point position is saved/restored on a per-window basis by 
select-window, but the mark position (and active status) is not.  IMO 
both point and mark (and active status) could do with being saved 
per-window and restored on window switches. That would be a noticeable 
behavioural change if done by default (could be optional though), and is 
presumably not a runner even as an option right at this very moment 
(feature freeze).  And of course, an ability to set the mark in one 
window open on a large buffer and the point in another window open onto 
a different place in the same large buffer is sortof a feature (though 
it's also possible to imagine both switch-window and, uh, 
switch-window-keeping-current-mark-position being available on different 
bindings).

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

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

On 08/07/11 06:54, David De La Harpe Golden wrote:
> On 05/07/11 04:26, Stefan Monnier wrote:
>
>
>> The window manager is ctwm rather than Metacity, in case it matters.
>>
>
> I can replicate this (not just with ctwm)
>
> The issue I was thinking of is still present too, and is likely
> closely related / really the same thing.

Hmm. bug #6872 was addressed by trunk r101176, which suppresses the 
primary update on handle_switch_frame, but, well, that only covers that 
immediate frame switch case.  Kinda need to address switching between 
individual windows too (or actually instead since handle_switch_frame 
will always wind up calling select_window itself, I think).  However, 
select-window itself is not a command afaiui, and enumerating all 
potential window-switching commands is probably not viable.  I can't say 
I like the attached solution (modelled on the deactivate-mark variable) 
very much, though nor did I have any especially better ideas.

















[-- Attachment #2: select-active-regions_noupdate_on_select_window_r1.diff --]
[-- Type: text/x-patch, Size: 2965 bytes --]

=== modified file 'lisp/simple.el'
--- lisp/simple.el	2011-07-06 19:44:09 +0000
+++ lisp/simple.el	2011-07-11 02:44:14 +0000
@@ -3701,6 +3701,7 @@
     (when (and (if (eq select-active-regions 'only)
 		   (eq (car-safe transient-mark-mode) 'only)
 		 select-active-regions)
+	       (not skip-active-region-selection)
 	       (region-active-p)
 	       (display-selections-p))
       ;; The var `saved-region-selection', if non-nil, is the text in

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2011-07-02 23:40:04 +0000
+++ src/keyboard.c	2011-07-11 02:43:15 +0000
@@ -1337,6 +1337,7 @@
   KVAR (current_kboard, Vprefix_arg) = Qnil;
   KVAR (current_kboard, Vlast_prefix_arg) = Qnil;
   Vdeactivate_mark = Qnil;
+  Vskip_active_region_selection = Qnil;
   waiting_for_input = 0;
   cancel_echoing ();
 
@@ -1388,6 +1389,7 @@
 	display_malloc_warning ();
 
       Vdeactivate_mark = Qnil;
+      Vskip_active_region_selection = Qnil;
 
       /* If minibuffer on and echo area in use,
 	 wait a short time and redraw minibuffer.  */
@@ -1512,6 +1514,7 @@
       /* Process filters and timers may have messed with deactivate-mark.
 	 reset it before we execute the command. */
       Vdeactivate_mark = Qnil;
+      Vskip_active_region_selection = Qnil;
 
       /* Remap command through active keymaps */
       Vthis_original_command = cmd;
@@ -1648,7 +1651,7 @@
 		      ? EQ (CAR_SAFE (Vtransient_mark_mode), Qonly)
 		      : (!NILP (Vselect_active_regions)
 			 && !NILP (Vtransient_mark_mode)))
-		  && !EQ (Vthis_command, Qhandle_switch_frame))
+                  && NILP (Vskip_active_region_selection))
 		{
 		  EMACS_INT beg =
 		    XINT (Fmarker_position (BVAR (current_buffer, mark)));
@@ -12212,6 +12215,16 @@
 `deactivate-mark' call uses this to set the window selection.  */);
   Vsaved_region_selection = Qnil;
 
+  DEFVAR_LISP ("skip-active-region-selection", Vskip_active_region_selection,
+	       doc: /* If an editing command sets this to t, don't update the primary selection.
+The command loop sets this to nil before each command,
+and tests the value when the command returns. Certain atypical commands
+(e.g. window and frame switching) may change the current active region,
+yet it is incorrect to update the system selection from that active
+region at that time, as would normally be done if select-active-regions
+is non-nil */);
+  Vskip_active_region_selection = Qnil;
+
   DEFVAR_LISP ("debug-on-event",
                Vdebug_on_event,
                doc: /* Enter debugger on this event.  When Emacs

=== modified file 'src/window.c'
--- src/window.c	2011-07-02 10:36:48 +0000
+++ src/window.c	2011-07-11 02:23:31 +0000
@@ -339,6 +339,9 @@
   if (EQ (window, selected_window) && !inhibit_point_swap)
     return window;
 
+  /* frame/window switches are not grounds to update the primary selection */
+  Vskip_active_region_selection = Qt;
+
   sf = SELECTED_FRAME ();
   if (XFRAME (WINDOW_FRAME (w)) != sf)
     {

bug#8996: Set PRIMARY from last selection, not last selected window

[Stefan Monnier]

severity 8996 important
thanks

> Hmm. bug #6872 was addressed by trunk r101176, which suppresses the primary
> update on handle_switch_frame, but, well, that only covers that immediate
> frame switch case.  Kinda need to address switching between individual
> windows too (or actually instead since handle_switch_frame will always wind
> up calling select_window itself, I think).  However, select-window itself is
> not a command afaiui, and enumerating all potential window-switching
> commands is probably not viable.  I can't say I like the attached solution
> (modelled on the deactivate-mark variable) very much, though nor did I have
> any especially better ideas.

I think this approach isn't as terrible as it sounds (tho I don't much like
the name you chose, sorry).  We'd want to let-bind that new var in
things like save-selected-window, with-selected-window, ... which is
kind of ugly.

Maybe a better approach is to record the selected window before running
a command, and only do the select-active-regions dance if the command did
not change the selected window.

Along the same lines, another approach that doesn't pay attention to
windows is to record the current buffer and the region's status
before running the command, and only do the select-active-regions is the
buffer is the same and the region's status has changed.


        Stefan

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 13/03/12 13:23, Stefan Monnier wrote:

> I think this approach isn't as terrible as it sounds (tho I don't much like
> the name you chose, sorry).  We'd want to let-bind that new var in
> things like save-selected-window, with-selected-window, ... which is
> kind of ugly.

I dunno, maybe if 'tis getting time ordering right we're worried about, 
we should stop trying to make an ordering emergent from global state and 
impose one, probably more robust - save an actual timestamp or sequence 
number with saved positions (and restore it on position restore), and if 
the current selection postdates, it just shouldn't be reset (x11 
selections are already timestamped anyway IIRC, dunno about other 
platforms).

> Maybe a better approach is to record the selected window before running
> a command, and only do the select-active-regions dance if the command did
> not change the selected window.

Not convinced myself that covers commands that actually legitimately 
change the selected window and also set a new region, like maybe a mouse 
gesture. though turns out I'm now rusty in the area and may not have 
thought it through fully.

> and only do the select-active-regions is the
> buffer is the same and the region's status has changed.

I don't think that one can work - two different windows can routinely be 
open on the same buffer?

bug#8996: Set PRIMARY from last selection, not last selected window

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Hmm. bug #6872 was addressed by trunk r101176, which suppresses the
>> primary update on handle_switch_frame, but, well, that only covers
>> that immediate frame switch case.  Kinda need to address switching
>> between individual windows too (or actually instead since
>> handle_switch_frame will always wind up calling select_window itself,
>> I think).  However, select-window itself is not a command afaiui, and
>> enumerating all potential window-switching commands is probably not
>> viable.  I can't say I like the attached solution (modelled on the
>> deactivate-mark variable) very much, though nor did I have any
>> especially better ideas.
>
> I think this approach isn't as terrible as it sounds (tho I don't much
> like the name you chose, sorry).  We'd want to let-bind that new var
> in things like save-selected-window, with-selected-window, ... which
> is kind of ugly.
>
> Maybe a better approach is to record the selected window before
> running a command, and only do the select-active-regions dance if the
> command did not change the selected window.

Why not just extend the Bug#6872 approach to handle-select-window too?

=== modified file 'src/keyboard.c'
*** src/keyboard.c	2012-02-24 08:34:09 +0000
--- src/keyboard.c	2012-03-24 11:09:56 +0000
***************
*** 241,246 ****
--- 241,247 ----
  Time last_event_timestamp;
  
  static Lisp_Object Qx_set_selection, Qhandle_switch_frame;
+ static Lisp_Object Qhandle_select_window;
  Lisp_Object QPRIMARY;
  
  static Lisp_Object Qself_insert_command;
***************
*** 1647,1653 ****
  		      ? EQ (CAR_SAFE (Vtransient_mark_mode), Qonly)
  		      : (!NILP (Vselect_active_regions)
  			 && !NILP (Vtransient_mark_mode)))
! 		  && !EQ (Vthis_command, Qhandle_switch_frame))
  		{
  		  EMACS_INT beg =
  		    XINT (Fmarker_position (BVAR (current_buffer, mark)));
--- 1648,1655 ----
  		      ? EQ (CAR_SAFE (Vtransient_mark_mode), Qonly)
  		      : (!NILP (Vselect_active_regions)
  			 && !NILP (Vtransient_mark_mode)))
! 		  && !EQ (Vthis_command, Qhandle_switch_frame)
! 		  && !EQ (Vthis_command, Qhandle_select_window))
  		{
  		  EMACS_INT beg =
  		    XINT (Fmarker_position (BVAR (current_buffer, mark)));
***************
*** 11649,11654 ****
--- 11651,11657 ----
    DEFSYM (Qx_set_selection, "x-set-selection");
    DEFSYM (QPRIMARY, "PRIMARY");
    DEFSYM (Qhandle_switch_frame, "handle-switch-frame");
+   DEFSYM (Qhandle_select_window, "handle-select-window");
  
    DEFSYM (Qinput_method_function, "input-method-function");
    DEFSYM (Qinput_method_exit_on_first_char, "input-method-exit-on-first-char");

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 24/03/12 11:10, Chong Yidong wrote:

> Why not just extend the Bug#6872 approach to handle-select-window too?
>

Well, that only catches cases that happen owing to things that trigger 
the <select-window> event that handle-select-window is bound to.

But that event isn't fired when you C-x o - other-window just straight 
calls the select-window function.  So, with your patch applied, it 
probably fixes the visible issue in Stefan's focus-follows-mouse case, 
but my keyboard recipe previously given still yields the "wrong"* 
selection.

So, you say, "then why not just add other-window too with the same 
approach?" Well, maybe, but then how many more need to be added after 
that?  That's what I was getting at when I previously remarked 
"enumerating all potential window-switching commands is probably not 
viable.".   Or maybe it is, though I guess in principle users could also 
define arbitrary new commands that called select-window (not sure many 
would. Maybe there could be an extensible select-inhibit-commands-list, 
and users who do such things could be told to push their command onto it...)


* there is probably an approach that can allow both behaviours as a 
switchable user customization, in case you think the selection yielded 
via the keyboard recipe is "right"...

bug#8996: Set PRIMARY from last selection, not last selected window

[Chong Yidong]

David De La Harpe Golden <david@harpegolden.net> writes:

> Well, that only catches cases that happen owing to things that trigger
> the <select-window> event that handle-select-window is bound to.
>
> But that event isn't fired when you C-x o - other-window just straight
> calls the select-window function.  So, with your patch applied, it
> probably fixes the visible issue in Stefan's focus-follows-mouse case,
> but my keyboard recipe previously given still yields the "wrong"*
> selection.

OTOH, keyboard commands stealing the selection seems a little less
problematic, because the user is specifically doing an Emacs command
rather than just moving the mouse.  I'm not sure it's wrong for C-x o to
get the selection if the window switched to has an active region.

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 25/03/12 04:42, Chong Yidong wrote:
> David De La Harpe Golden<david@harpegolden.net>  writes:
>
>> Well, that only catches cases that happen owing to things that trigger
>> the<select-window>  event that handle-select-window is bound to.
>>
>> But that event isn't fired when you C-x o - other-window just straight
>> calls the select-window function.  So, with your patch applied, it
>> probably fixes the visible issue in Stefan's focus-follows-mouse case,
>> but my keyboard recipe previously given still yields the "wrong"*
>> selection.
>
> OTOH, keyboard commands stealing the selection seems a little less
> problematic, because the user is specifically doing an Emacs command
> rather than just moving the mouse.  I'm not sure it's wrong for C-x o to
> get the selection if the window switched to has an active region.

Hmm, well, subjectively it did feel wrong to me, it is after all the 
context I spotted the issue in.  But I'm now presumably also influenced 
by months of using my locally munged emacs.

In terms of current practice of one of our "competitors" (loosely) that 
has fairly similar functionality, kde kate, it does _not_ appear to 
reset x11 primary to the older but still visibly highlighted region when 
you just switch between windows, even with the keyboard (of course it 
doesn't use emacs terminology for the entities in question, but 
functionally similar), it sticks to the temporal order:

launch kate, enter a new session
enter "The quick brown fox jumps over the lazy dog."
hit Ctrl-Shift-T  (split window)
select (shift-arrow keys) "fox" in window #1
hit F8 (switch window)
select (shift-arrow keys) "dog" in window #2
hit F8 (switch window)
-> current window #1 again, and has "fox" region still visibly 
highlighted (and it is again current in terms of intra-kate ops like 
overtyping it)
hit mouse-2
->  "dog" is inserted from primary.

In gvim, given modality there are probably too many differences for the 
comparison to be fair, anyway it doesn't reset primary to the older (and 
unlike emacs/kate, not visible anyway) region when you switch windows. 
Visible region highlighting apparently disappears once you exit visual 
mode, so the question of visual vs. temporal correspondence that arises 
in emacs/kate is irrelevant, temporal is the natural option. Visual mode 
seems to be exited when you switch windows to a window onto a different 
buffer, it's only maintained if both windows are onto the same buffer 
and in which case they apparently have the same visual region, so the 
same-buffer/different-window/different-visible-region case of emacs or 
kate probably can't arise (not a vim expert, mind).

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 25/03/12 04:32, David De La Harpe Golden wrote:

> Maybe there could be an extensible select-inhibit-commands-list,
> and users who do such things could be told to push their command onto
> it...)

I suppose you'd prefer to settle on a solution this weekend given the 
announced regressions-only, and I'm now unlikely to finish a 
probably-overengineered stab at a temporal-because-actually-timestamped 
approach today, and, well, the old approach I sent was a tad ugly too, so...

If such a postcommand-select-inhibit-list* defvar** was introduced and 
checked against instead of just hardcoding the two handle-select-window 
and handle-switch-frame, then whether or not you decide other-window 
also belongs on such a list by default, I could at least push 
other-window (and any other commands I encounter where it seems 
desirable) onto the list in my own ~/.emacs and reduce my local patching 
load...


* or whatever you want to call it.

** Making something so niche a visible defcustom might be a bit much.

bug#8996: Set PRIMARY from last selection, not last selected window

[Chong Yidong]

David De La Harpe Golden <david@harpegolden.net> writes:

> If such a postcommand-select-inhibit-list* defvar** was introduced and
> checked against instead of just hardcoding the two
> handle-select-window and handle-switch-frame, then whether or not you
> decide other-window also belongs on such a list by default, I could at
> least push other-window (and any other commands I encounter where it
> seems desirable) onto the list in my own ~/.emacs and reduce my local
> patching load...

OK, I've committed a variable selection-inhibit-update-commands.

bug#8996: Set PRIMARY from last selection, not last selected window

[David De La Harpe Golden]

On 26/03/12 05:06, Chong Yidong wrote:

> OK, I've committed a variable selection-inhibit-update-commands.

Right so, thanks. Seems to work fine, and it is useful to me to be able
to add entries, anyway:

FWIW, in the past day of emacs use, apart from 'handle-select-window and 
'handle-switch-frame, 'other-window was indeed the main case I ran into, 
but 'other-frame got added too.

'delete-window (and 'delete-frame too) also got pushed onto the list 
quickly: I was sometimes selecting something in a window then deleting 
that window immediately afterward, and then of course, without 
'delete-window on the list, primary would then be replaced by whatever 
region in whatever remaining window got (re)activated, losing whatever I 
had expected to be in primary.

Haven't felt an urge to add anything else as yet, but I guess I only
use a fairly small working set of emacs commands, and of course in turn 
the issue will only apply to a few of them. I suppose more 
emacs-idiomatic use of the kill-ring would have meant I didn't encounter 
as many problems, so my own usage quirks aren't blameless.