GNU bug report logs - #28631
25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28631 in the body.
You can then email your comments to 28631 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
Date: Thu, 28 Sep 2017 17:02:39 +0900

Tags: patch

I am biten by this problem from time to time.  When winer-mode
is enabled and i deactivate the mark with C-g, then i observe
funny behaviour with `yank'.

Usually i visit another buffer BUF-B to copy a string STR; then
i do `winner-undo' to comeback to the initial buffer, BUF-A,  and `yank'
STR.  Sometimes i found that `yank' insert a string different than STR.
That's happen if i previously deactivated the mark in BUF-A with C-g.

To reproduce the problem:
emacs  -Q -eval '(winner-mode 1)' \
-eval "(customize-set-variable 'select-enable-clipboard nil)" \
-eval "(customize-set-variable 'select-enable-primary t)"

< C-TAB M-f M-f ; `yank' would insert ";; This buffer"
C-x C-b C-x 0
C-x h ; mark whole buffer
;; `yank' would insert the content of *Buffer List*
C-g
RET ; `yank' would insert  ";; This buffer"
C-c <left> ; winner-undo
;; `yank' would insert the content of *Buffer List*

Note that the primary selection doesn't change if you visit
*Buffer List* with C-x b RET.  That is, if you change above
C-c <left>
by:
C-x b RET

then, the `yank' will insert ";;This buffer".

I don't see a good reason why visiting a buffer with `C-x b' or
`winner-undo' makes a difference in the primary selection.

How about the following patch?
--8<-----------------------------cut here---------------start------------->8---
commit 74e5a589b762388baadbb2ac2f146bbe66765deb
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Thu Sep 28 16:34:52 2017 +0900

    Set mark at point after keyboard-quit
    
    * lisp/simple.el (deactivate-mark): In transient-mark-mode
    always set the mark at point after keyboard-quit (Bug#28631).

diff --git a/lisp/simple.el b/lisp/simple.el
index 469557713d..1f809d8964 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5349,6 +5349,11 @@ deactivate-mark
       (kill-local-variable 'transient-mark-mode)))
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
+    ;; Set mark at point after `keyboard-quit' (Bug#28631).
+    (when (and transient-mark-mode
+               (eq this-command 'keyboard-quit)
+               (/= (mark 'force) (point)))
+      (push-mark nil 'nomsg))
     (redisplay--update-region-highlight (selected-window))))
 
 (defun activate-mark (&optional no-tmm)

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 11, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-09-28
Repository revision: 1f02ae39310f15bf683642b9aee1cf162bd391e6


In GNU Emacs 25.3.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-09-20 built on calancha-pc
Repository revision: c3ff6712ad24fcf45874dc0665a8606e9b2208a4
Windowing system distributor 'The X.Org Foundation', version 11.0.11902000
System Description:	Debian GNU/Linux 9.1 (stretch)

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Buffer Menu

Minor modes in effect:
  winner-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set [4 times]
Quit
Winner undo (1 / 4)
Quit

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode mail-prsvr
mail-utils cus-edit easymenu cus-start cus-load wid-edit cl-loaddefs
pcase cl-lib winner ring time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 104842 3683)
 (symbols 48 21319 0)
 (miscs 40 108 146)
 (strings 32 17876 5250)
 (string-bytes 1 487777)
 (vectors 16 12687)
 (vector-slots 8 439460 5000)
 (floats 8 174 21)
 (intervals 56 281 10)
 (buffers 976 20))

Message #8 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 28631 <at> debbugs.gnu.org
Cc: tino.calancha <at> gmail.com
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Thu, 12 Oct 2017 09:52:28 +0900

Tino Calancha <tino.calancha <at> gmail.com> writes:

> I am biten by this problem from time to time.

> Usually I visit another buffer BUF-B to copy a string STR; then
> I do `winner-undo' to comeback to the initial buffer, BUF-A,  and `yank'
> STR.  Sometimes I found that `yank' insert a string different than STR.
> That's happen if I previously deactivated the mark in BUF-A with C-g.

Since I reported this issue I added the patch into my .emacs and
I don't get this problem anymore.
Maybe someone prefer to fix this issue in another way.
Any comment or suggestion?

Message #11 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 28631 <at> debbugs.gnu.org
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Wed, 11 Oct 2017 22:06:22 -0400

Tino Calancha <tino.calancha <at> gmail.com> writes:

> I am biten by this problem from time to time.  When winer-mode
> is enabled and i deactivate the mark with C-g, then i observe
> funny behaviour with `yank'.
>
> Usually i visit another buffer BUF-B to copy a string STR; then
> i do `winner-undo' to comeback to the initial buffer, BUF-A,  and `yank'
> STR.  Sometimes i found that `yank' insert a string different than STR.
> That's happen if i previously deactivated the mark in BUF-A with C-g.
>
> To reproduce the problem:
> emacs  -Q -eval '(winner-mode 1)' \
> -eval "(customize-set-variable 'select-enable-clipboard nil)" \
> -eval "(customize-set-variable 'select-enable-primary t)"
>
> < C-TAB M-f M-f ; `yank' would insert ";; This buffer"

That should be 'M-< C-SPC M-f M-f' I guess.

> C-x C-b C-x 0

C-x o, not C-x 0.

> C-x h ; mark whole buffer
> ;; `yank' would insert the content of *Buffer List*
> C-g
> RET ; `yank' would insert  ";; This buffer"
> C-c <left> ; winner-undo
> ;; `yank' would insert the content of *Buffer List*
>
> Note that the primary selection doesn't change if you visit
> *Buffer List* with C-x b RET.  That is, if you change above
> C-c <left>
> by:
> C-x b RET
>
> then, the `yank' will insert ";;This buffer".
>
> I don't see a good reason why visiting a buffer with `C-x b' or
> `winner-undo' makes a difference in the primary selection.

It's because winner-undo tries to restore marks.

(defun winner-active-region ()
  (declare (gv-setter (lambda (store) ...
                          `(if ,store (activate-mark) (deactivate-mark))))))
  (region-active-p))

...

;; Make sure point does not end up in the minibuffer and delete
;; windows displaying dead or boring buffers
;; (c.f. `winner-boring-buffers').  Return nil if all the windows
;; should be deleted.  Preserve correct points and marks.
(defun winner-set (conf)
  ...
      ;; Restore marks
      (save-current-buffer
        (cl-loop for buf in buffers
                 for entry = (cadr (assq buf winner-point-alist))
                 do (progn (set-buffer buf)
                           (set-mark (car entry))
                           (setf (winner-active-region) (cdr entry)))))
  ...


Lisp Backtrace:
"x-own-selection-internal" (0xffffb560)
0x1364810 PVEC_COMPILED
"apply" (0xffffbbe0)
"gui-backend-set-selection" (0xffffc110)
"gui-set-selection" (0xffffc628)
"deactivate-mark" (0xffffcb60)
"winner-set" (0xffffd100)
"winner-undo-this" (0xffffd600)
"winner-undo" (0xffffddb0)
"funcall-interactively" (0xffffdda8)
"call-interactively" (0xffffe010)
"command-execute" (0xffffe588)
(gdb) p selection_value
$10 = XIL(0x31a5f54)
(gdb) xpr
Lisp_String
$11 = (struct Lisp_String *) 0x31a5f50
". * *scratch*", ' ' <repeats 15 times>, "266 Lisp Interaction \n %* *Messages*", ' ' <repeats 14 times>, "133 Messages         \n"

> How about the following patch?
>
>     Set mark at point after keyboard-quit
>     
>     * lisp/simple.el (deactivate-mark): In transient-mark-mode
>     always set the mark at point after keyboard-quit (Bug#28631).

That seems quite disruptive.  Perhaps instead deactivate-mark could be
changed so that it would not update the selection on winner-undo?

Message #14 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: 28631 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Thu, 12 Oct 2017 11:53:47 +0900

Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
Thank you very much for the comments.  I have updated
the patch following your suggestion (see below).

>> To reproduce the problem:
>> emacs  -Q -eval '(winner-mode 1)' \
>> -eval "(customize-set-variable 'select-enable-clipboard nil)" \
>> -eval "(customize-set-variable 'select-enable-primary t)"
>>
>> < C-TAB M-f M-f ; `yank' would insert ";; This buffer"
>
> That should be 'M-< C-SPC M-f M-f' I guess.
Absolutely.

>> C-x C-b C-x 0
>
> C-x o, not C-x 0.
I think is OK.  After C-x C-b you are in *scratch* buffer and the window
is splited horizontally.  C-x 0 brings you at *Buffer List*.

>> I don't see a good reason why visiting a buffer with `C-x b' or
>> `winner-undo' makes a difference in the primary selection.
>
> It's because winner-undo tries to restore marks.
I see, thank you.

>
>> How about the following patch?
>>
>>     Set mark at point after keyboard-quit
>>     
>>     * lisp/simple.el (deactivate-mark): In transient-mark-mode
>>     always set the mark at point after keyboard-quit (Bug#28631).
>
> That seems quite disruptive.  Perhaps instead deactivate-mark could be
> changed so that it would not update the selection on winner-undo?
Yeah, my patch is kinda of lazy.  What you are suggesting sounds better.
Following one works to me

--8<-----------------------------cut here---------------start------------->8---
commit 6e4e47062daf54923928f6db096d4578bcecd6e2
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Thu Oct 12 11:41:44 2017 +0900

    Dont update primary selection with winner-undo
    
    * lisp/simple.el (deactivate-mark):
    Dont update primary selection if deactivate-mark is
    called by winner-undo (Bug#28631).

diff --git a/lisp/simple.el b/lisp/simple.el
index 5ef511ce0a..faedad4675 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5332,6 +5332,8 @@ deactivate-mark
 	     (if (gui-backend-selection-owner-p 'PRIMARY)
 		 (gui-set-selection 'PRIMARY saved-region-selection))
 	     (setq saved-region-selection nil))
+            ;; `winner-undo' shouldn't update the selection (Bug#28631).
+            ((eq this-command 'winner-undo) nil)
 	    ;; If another program has acquired the selection, region
 	    ;; deactivation should not clobber it (Bug#11772).
 	    ((and (/= (region-beginning) (region-end))

--8<-----------------------------cut here---------------end--------------->8---

In GNU Emacs 27.0.50 (build 14, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-10-12
Repository revision: 349c1f93021e49c63f076b602d3d228324105fd6

Message #17 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 28631 <at> debbugs.gnu.org
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Thu, 12 Oct 2017 20:46:54 -0400

Tino Calancha <tino.calancha <at> gmail.com> writes:

>> C-x o, not C-x 0.
> I think is OK.  After C-x C-b you are in *scratch* buffer and the window
> is splited horizontally.  C-x 0 brings you at *Buffer List*.

Oops, you're right, I was confused.

> Following one works to me
>
> commit 6e4e47062daf54923928f6db096d4578bcecd6e2
> Author: Tino Calancha <tino.calancha <at> gmail.com>
> Date:   Thu Oct 12 11:41:44 2017 +0900
>
>     Dont update primary selection with winner-undo
>     
>     * lisp/simple.el (deactivate-mark):
>     Dont update primary selection if deactivate-mark is
>     called by winner-undo (Bug#28631).
>
> diff --git a/lisp/simple.el b/lisp/simple.el
> index 5ef511ce0a..faedad4675 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -5332,6 +5332,8 @@ deactivate-mark
>  	     (if (gui-backend-selection-owner-p 'PRIMARY)
>  		 (gui-set-selection 'PRIMARY saved-region-selection))
>  	     (setq saved-region-selection nil))
> +            ;; `winner-undo' shouldn't update the selection (Bug#28631).
> +            ((eq this-command 'winner-undo) nil)
>  	    ;; If another program has acquired the selection, region
>  	    ;; deactivation should not clobber it (Bug#11772).
>  	    ((and (/= (region-beginning) (region-end))

I think I would rather put it with the next condition:

--- i/lisp/simple.el
+++ w/lisp/simple.el
@@ -5336,7 +5336,9 @@ deactivate-mark
 	    ;; deactivation should not clobber it (Bug#11772).
 	    ((and (/= (region-beginning) (region-end))
 		  (or (gui-backend-selection-owner-p 'PRIMARY)
-		      (null (gui-backend-selection-exists-p 'PRIMARY))))
+                      (null (gui-backend-selection-exists-p 'PRIMARY)))
+                  ;; `winner-undo' shouldn't update the selection (Bug#28631).
+                  (not (eq this-command 'winner-undo)))
 	     (gui-set-selection 'PRIMARY
                                 (funcall region-extract-function nil)))))
     (when mark-active (force-mode-line-update)) ;Refresh toolbar (bug#16382).

Either way, I think it's okay to push emacs-26, but wait a bit in case
someone else thinks otherwise.

Message #20 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: 28631 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the
 primary-selection
Date: Fri, 13 Oct 2017 10:54:41 +0900 (JST)

On Thu, 12 Oct 2017, Noam Postavsky wrote:

> I think I would rather put it with the next condition:
>
> --- i/lisp/simple.el
> +++ w/lisp/simple.el
> @@ -5336,7 +5336,9 @@ deactivate-mark
> 	    ;; deactivation should not clobber it (Bug#11772).
> 	    ((and (/= (region-beginning) (region-end))
> 		  (or (gui-backend-selection-owner-p 'PRIMARY)
> -		      (null (gui-backend-selection-exists-p 'PRIMARY))))
> +                      (null (gui-backend-selection-exists-p 'PRIMARY)))
> +                  ;; `winner-undo' shouldn't update the selection (Bug#28631).
> +                  (not (eq this-command 'winner-undo)))
> 	     (gui-set-selection 'PRIMARY
>                                 (funcall region-extract-function nil)))))
>     (when mark-active (force-mode-line-update)) ;Refresh toolbar (bug#16382).
>
> Either way, I think it's okay to push emacs-26, but wait a bit in case
> someone else thinks otherwise.
OK.
Thank you very much.

Message #23 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: 28631 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Fri, 13 Oct 2017 09:48:15 +0300

> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Thu, 12 Oct 2017 20:46:54 -0400
> Cc: 28631 <at> debbugs.gnu.org
> 
> --- i/lisp/simple.el
> +++ w/lisp/simple.el
> @@ -5336,7 +5336,9 @@ deactivate-mark
>  	    ;; deactivation should not clobber it (Bug#11772).
>  	    ((and (/= (region-beginning) (region-end))
>  		  (or (gui-backend-selection-owner-p 'PRIMARY)
> -		      (null (gui-backend-selection-exists-p 'PRIMARY))))
> +                      (null (gui-backend-selection-exists-p 'PRIMARY)))
> +                  ;; `winner-undo' shouldn't update the selection (Bug#28631).
> +                  (not (eq this-command 'winner-undo)))
>  	     (gui-set-selection 'PRIMARY
>                                  (funcall region-extract-function nil)))))
>      (when mark-active (force-mode-line-update)) ;Refresh toolbar (bug#16382).
> 
> Either way, I think it's okay to push emacs-26, but wait a bit in case
> someone else thinks otherwise.

Is there really no way to solve this in winner?  It seems like a
winner bug/misfeature, and I'm worried by the possible effect of this
patch on use cases that have nothing to do with the specific scenario
of this bug.  deactivate-mark is used a lot in places and ways we
cannot possibly predict.

So I think a solution for emacs-26 should be safer than that.

Thanks.

Message #26 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 28631 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> users.sourceforge.net>
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Fri, 13 Oct 2017 17:06:53 +0900

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
>> Date: Thu, 12 Oct 2017 20:46:54 -0400
>> Cc: 28631 <at> debbugs.gnu.org
>> 
>> --- i/lisp/simple.el
>> +++ w/lisp/simple.el
>> @@ -5336,7 +5336,9 @@ deactivate-mark
>>  	    ;; deactivation should not clobber it (Bug#11772).
>>  	    ((and (/= (region-beginning) (region-end))
>>  		  (or (gui-backend-selection-owner-p 'PRIMARY)
>> -		      (null (gui-backend-selection-exists-p 'PRIMARY))))
>> +                      (null (gui-backend-selection-exists-p 'PRIMARY)))
>> +                  ;; `winner-undo' shouldn't update the selection (Bug#28631).
>> +                  (not (eq this-command 'winner-undo)))
>>  	     (gui-set-selection 'PRIMARY
>>                                  (funcall region-extract-function nil)))))
>>      (when mark-active (force-mode-line-update)) ;Refresh toolbar (bug#16382).
>> 
>> Either way, I think it's okay to push emacs-26, but wait a bit in case
>> someone else thinks otherwise.
>
> Is there really no way to solve this in winner?  It seems like a
> winner bug/misfeature, and I'm worried by the possible effect of this
> patch on use cases that have nothing to do with the specific scenario
> of this bug.  deactivate-mark is used a lot in places and ways we
> cannot possibly predict.
I agree it's better if it is handled inside winner.
I am not a winner guru, just an user so sorry if the following patch
is not right.

--8<-----------------------------cut here---------------start------------->8---
commit 3420c8089f3c3d2dc22472dddfa938a2bd044a27
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Fri Oct 13 16:56:55 2017 +0900

    Dont update primary selection with winner-undo
    
    * lisp/winner.el (winner-set):
    Dont update primary selection when select-enable-primary
    is non-nil (Bug#28631).

diff --git a/lisp/winner.el b/lisp/winner.el
index 61ea4d40e7..6bc27484a7 100644
--- a/lisp/winner.el
+++ b/lisp/winner.el
@@ -304,12 +304,15 @@ winner-set
           (push win xwins)))            ; delete this window
 
       ;; Restore marks
-      (save-current-buffer
-	(cl-loop for buf in buffers
-                 for entry = (cadr (assq buf winner-point-alist))
-                 do (progn (set-buffer buf)
-                           (set-mark (car entry))
-                           (setf (winner-active-region) (cdr entry)))))
+      ;; `winner-undo' shouldn't update the selection (Bug#28631) when
+      ;; select-enable-primary is non-nil.
+      (unless select-enable-primary
+        (save-current-buffer
+	  (cl-loop for buf in buffers
+                   for entry = (cadr (assq buf winner-point-alist))
+                   do (progn (set-buffer buf)
+                             (set-mark (car entry))
+                             (setf (winner-active-region) (cdr entry))))))
       ;; Delete windows, whose buffers are dead or boring.
       ;; Return t if this is still a possible configuration.
       (or (null xwins)

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 7, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-10-13
Repository revision: 7dc037e39e6bbfa8964d0040e8141dbcf70d726d

Message #29 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 28631 <at> debbugs.gnu.org
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Fri, 13 Oct 2017 09:32:28 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>>  	    ((and (/= (region-beginning) (region-end))
>>  		  (or (gui-backend-selection-owner-p 'PRIMARY)
>> -		      (null (gui-backend-selection-exists-p 'PRIMARY))))
>> +                      (null (gui-backend-selection-exists-p 'PRIMARY)))
>> +                  ;; `winner-undo' shouldn't update the selection (Bug#28631).
>> +                  (not (eq this-command 'winner-undo)))
>>  	     (gui-set-selection 'PRIMARY
>>                                  (funcall region-extract-function nil)))))

> Is there really no way to solve this in winner?  It seems like a
> winner bug/misfeature, and I'm worried by the possible effect of this
> patch on use cases that have nothing to do with the specific scenario
> of this bug.  deactivate-mark is used a lot in places and ways we
> cannot possibly predict.

That patch only has affect during winner-undo, no?  Probably cleaner to
avoid relying on `this-command' if possible though.

Tino Calancha <tino.calancha <at> gmail.com> writes:

> I agree it's better if it is handled inside winner.
> I am not a winner guru, just an user so sorry if the following patch
> is not right.

> +      ;; `winner-undo' shouldn't update the selection (Bug#28631) when
> +      ;; select-enable-primary is non-nil.
> +      (unless select-enable-primary
> +        (save-current-buffer
> +	  (cl-loop for buf in buffers
> +                   for entry = (cadr (assq buf winner-point-alist))
> +                   do (progn (set-buffer buf)
> +                             (set-mark (car entry))
> +                             (setf (winner-active-region) (cdr entry))))))

Maybe only the (setf (winner-active-region) (cdr entry)) part should be
skipped?

Message #32 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 28631 <at> debbugs.gnu.org,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the
 primary-selection
Date: Sat, 14 Oct 2017 10:42:15 +0900 (JST)

On Fri, 13 Oct 2017, Noam Postavsky wrote:

>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>>  	    ((and (/= (region-beginning) (region-end))
>>>  		  (or (gui-backend-selection-owner-p 'PRIMARY)
>>> -		      (null (gui-backend-selection-exists-p 'PRIMARY))))
>>> +                      (null (gui-backend-selection-exists-p 'PRIMARY)))
>>> +                  ;; `winner-undo' shouldn't update the selection (Bug#28631).
>>> +                  (not (eq this-command 'winner-undo)))
>>>  	     (gui-set-selection 'PRIMARY
>>>                                  (funcall region-extract-function nil)))))
>
>> Is there really no way to solve this in winner?  It seems like a
>> winner bug/misfeature, and I'm worried by the possible effect of this
>> patch on use cases that have nothing to do with the specific scenario
>> of this bug.  deactivate-mark is used a lot in places and ways we
>> cannot possibly predict.
>
> That patch only has affect during winner-undo, no?
I think so.

>> +      ;; `winner-undo' shouldn't update the selection (Bug#28631) when
>> +      ;; select-enable-primary is non-nil.
>> +      (unless select-enable-primary
>> +        (save-current-buffer
>> +	  (cl-loop for buf in buffers
>> +                   for entry = (cadr (assq buf winner-point-alist))
>> +                   do (progn (set-buffer buf)
>> +                             (set-mark (car entry))
>> +                             (setf (winner-active-region) (cdr entry))))))
>
> Maybe only the (setf (winner-active-region) (cdr entry)) part should be
> skipped?
We need to ban
(set-mark (car entry))
as well, because it updates the primary selection.

emacs  -Q -eval '(winner-mode 1)' \
-eval "(customize-set-variable 'select-enable-clipboard nil)" \
-eval "(customize-set-variable 'select-enable-primary t)"

M-<
M-: (set-mark 15) RET
M-: C-y ; It inserts in minibuffer ";; This buffer"

I prefer this patch because:
1) Make the fix inside winner.el
2) The fix takes effect just if user has enabled select-enable-primary

I don't fully understand the purpose of that part of the code, but
I assume if someone have set select-enable-primary, then she
probably doesn't want winner-undo to change her selection.

Message #35 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 28631 <at> debbugs.gnu.org
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Fri, 13 Oct 2017 22:09:49 -0400

Tino Calancha <tino.calancha <at> gmail.com> writes:

> On Fri, 13 Oct 2017, Noam Postavsky wrote:
>
>>> Is there really no way to solve this in winner?  It seems like a
>>> winner bug/misfeature, and I'm worried by the possible effect of this
>>> patch on use cases that have nothing to do with the specific scenario
>>> of this bug.  deactivate-mark is used a lot in places and ways we
>>> cannot possibly predict.
>>
>> That patch only has affect during winner-undo, no?
> I think so.

Actually, I thought about it a bit more, and realized it could also
affect things like post-command hooks that are run after winner-undo.

>> Maybe only the (setf (winner-active-region) (cdr entry)) part should be
>> skipped?
> We need to ban
> (set-mark (car entry))
> as well, because it updates the primary selection.

Ah, okay.

> I assume if someone have set select-enable-primary, then she
> probably doesn't want winner-undo to change her selection.

Seems reasonable.

Message #38 received at 28631 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: 28631 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Sat, 14 Oct 2017 09:56:20 +0300

> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  28631 <at> debbugs.gnu.org
> Date: Fri, 13 Oct 2017 22:09:49 -0400
> 
> >> That patch only has affect during winner-undo, no?
> > I think so.
> 
> Actually, I thought about it a bit more, and realized it could also
> affect things like post-command hooks that are run after winner-undo.
> 
> >> Maybe only the (setf (winner-active-region) (cdr entry)) part should be
> >> skipped?
> > We need to ban
> > (set-mark (car entry))
> > as well, because it updates the primary selection.
> 
> Ah, okay.
> 
> > I assume if someone have set select-enable-primary, then she
> > probably doesn't want winner-undo to change her selection.
> 
> Seems reasonable.

Please push to emacs-26 in a few days, if no comments or objections
surface.

Thanks.

Message #43 received at 28631-done <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 28631-done <at> debbugs.gnu.org
Subject: Re: bug#28631: 25.3.50;
 Deactivate mark with Ctrl-g mess up the primary-selection
Date: Tue, 17 Oct 2017 16:25:54 +0900

Eli Zaretskii <eliz <at> gnu.org> writes:

> Please push to emacs-26 in a few days, if no comments or objections
> surface.
Pushed into emacs-26 branch as commit
"Dont update primary selection with winner-undo"
(2c3e6f1ddc90335249f1a7f56f5f7b377c873fb7)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.