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

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Tino Calancha-2
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 <[hidden email]>
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))



Reply | Threaded
Open this post in threaded view
|

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

Tino Calancha-2
Tino Calancha <[hidden email]> 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?



Reply | Threaded
Open this post in threaded view
|

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

Noam Postavsky-2
In reply to this post by Tino Calancha-2
Tino Calancha <[hidden email]> 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?



Reply | Threaded
Open this post in threaded view
|

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

Tino Calancha-2
Noam Postavsky <[hidden email]> 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 <[hidden email]>
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



Reply | Threaded
Open this post in threaded view
|

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

Noam Postavsky-2
Tino Calancha <[hidden email]> 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 <[hidden email]>
> 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.



Reply | Threaded
Open this post in threaded view
|

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

Tino Calancha-2


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.




Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
In reply to this post by Noam Postavsky-2
> From: Noam Postavsky <[hidden email]>
> Date: Thu, 12 Oct 2017 20:46:54 -0400
> Cc: [hidden email]
>
> --- 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.



Reply | Threaded
Open this post in threaded view
|

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

Tino Calancha-2
Eli Zaretskii <[hidden email]> writes:

>> From: Noam Postavsky <[hidden email]>
>> Date: Thu, 12 Oct 2017 20:46:54 -0400
>> Cc: [hidden email]
>>
>> --- 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 <[hidden email]>
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



Reply | Threaded
Open this post in threaded view
|

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

Noam Postavsky-2

Eli Zaretskii <[hidden email]> 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 <[hidden email]> 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?



Reply | Threaded
Open this post in threaded view
|

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

Tino Calancha-2


On Fri, 13 Oct 2017, Noam Postavsky wrote:

>
> Eli Zaretskii <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|

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

Noam Postavsky-2
Tino Calancha <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: Noam Postavsky <[hidden email]>
> Cc: Eli Zaretskii <[hidden email]>,  [hidden email]
> 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.



Reply | Threaded
Open this post in threaded view
|

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

Tino Calancha-2
Eli Zaretskii <[hidden email]> 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)