* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
@ 2017-09-28 8:02 Tino Calancha
2017-10-12 0:52 ` Tino Calancha
2017-10-12 2:06 ` Noam Postavsky
0 siblings, 2 replies; 13+ messages in thread
From: Tino Calancha @ 2017-09-28 8:02 UTC (permalink / raw)
To: 28631
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@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))
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-09-28 8:02 bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection Tino Calancha
@ 2017-10-12 0:52 ` Tino Calancha
2017-10-12 2:06 ` Noam Postavsky
1 sibling, 0 replies; 13+ messages in thread
From: Tino Calancha @ 2017-10-12 0:52 UTC (permalink / raw)
To: 28631; +Cc: tino.calancha
Tino Calancha <tino.calancha@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?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-09-28 8:02 bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection Tino Calancha
2017-10-12 0:52 ` Tino Calancha
@ 2017-10-12 2:06 ` Noam Postavsky
2017-10-12 2:53 ` Tino Calancha
1 sibling, 1 reply; 13+ messages in thread
From: Noam Postavsky @ 2017-10-12 2:06 UTC (permalink / raw)
To: Tino Calancha; +Cc: 28631
Tino Calancha <tino.calancha@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?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-12 2:06 ` Noam Postavsky
@ 2017-10-12 2:53 ` Tino Calancha
2017-10-13 0:46 ` Noam Postavsky
0 siblings, 1 reply; 13+ messages in thread
From: Tino Calancha @ 2017-10-12 2:53 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 28631, tino.calancha
Noam Postavsky <npostavs@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@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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-12 2:53 ` Tino Calancha
@ 2017-10-13 0:46 ` Noam Postavsky
2017-10-13 1:54 ` Tino Calancha
2017-10-13 6:48 ` Eli Zaretskii
0 siblings, 2 replies; 13+ messages in thread
From: Noam Postavsky @ 2017-10-13 0:46 UTC (permalink / raw)
To: Tino Calancha; +Cc: 28631
Tino Calancha <tino.calancha@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@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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-13 0:46 ` Noam Postavsky
@ 2017-10-13 1:54 ` Tino Calancha
2017-10-13 6:48 ` Eli Zaretskii
1 sibling, 0 replies; 13+ messages in thread
From: Tino Calancha @ 2017-10-13 1:54 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 28631, Tino Calancha
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-13 0:46 ` Noam Postavsky
2017-10-13 1:54 ` Tino Calancha
@ 2017-10-13 6:48 ` Eli Zaretskii
2017-10-13 8:06 ` Tino Calancha
1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2017-10-13 6:48 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 28631, tino.calancha
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Thu, 12 Oct 2017 20:46:54 -0400
> Cc: 28631@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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-13 6:48 ` Eli Zaretskii
@ 2017-10-13 8:06 ` Tino Calancha
2017-10-13 13:32 ` Noam Postavsky
0 siblings, 1 reply; 13+ messages in thread
From: Tino Calancha @ 2017-10-13 8:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 28631, Noam Postavsky
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Thu, 12 Oct 2017 20:46:54 -0400
>> Cc: 28631@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@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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-13 8:06 ` Tino Calancha
@ 2017-10-13 13:32 ` Noam Postavsky
2017-10-14 1:42 ` Tino Calancha
0 siblings, 1 reply; 13+ messages in thread
From: Noam Postavsky @ 2017-10-13 13:32 UTC (permalink / raw)
To: Tino Calancha; +Cc: 28631
Eli Zaretskii <eliz@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@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?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-13 13:32 ` Noam Postavsky
@ 2017-10-14 1:42 ` Tino Calancha
2017-10-14 2:09 ` Noam Postavsky
0 siblings, 1 reply; 13+ messages in thread
From: Tino Calancha @ 2017-10-14 1:42 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 28631, Tino Calancha
On Fri, 13 Oct 2017, Noam Postavsky wrote:
>
> Eli Zaretskii <eliz@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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-14 1:42 ` Tino Calancha
@ 2017-10-14 2:09 ` Noam Postavsky
2017-10-14 6:56 ` Eli Zaretskii
0 siblings, 1 reply; 13+ messages in thread
From: Noam Postavsky @ 2017-10-14 2:09 UTC (permalink / raw)
To: Tino Calancha; +Cc: 28631
Tino Calancha <tino.calancha@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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-14 2:09 ` Noam Postavsky
@ 2017-10-14 6:56 ` Eli Zaretskii
2017-10-17 7:25 ` Tino Calancha
0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2017-10-14 6:56 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 28631, tino.calancha
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, 28631@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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection
2017-10-14 6:56 ` Eli Zaretskii
@ 2017-10-17 7:25 ` Tino Calancha
0 siblings, 0 replies; 13+ messages in thread
From: Tino Calancha @ 2017-10-17 7:25 UTC (permalink / raw)
To: 28631-done
Eli Zaretskii <eliz@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)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-10-17 7:25 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-28 8:02 bug#28631: 25.3.50; Deactivate mark with Ctrl-g mess up the primary-selection Tino Calancha
2017-10-12 0:52 ` Tino Calancha
2017-10-12 2:06 ` Noam Postavsky
2017-10-12 2:53 ` Tino Calancha
2017-10-13 0:46 ` Noam Postavsky
2017-10-13 1:54 ` Tino Calancha
2017-10-13 6:48 ` Eli Zaretskii
2017-10-13 8:06 ` Tino Calancha
2017-10-13 13:32 ` Noam Postavsky
2017-10-14 1:42 ` Tino Calancha
2017-10-14 2:09 ` Noam Postavsky
2017-10-14 6:56 ` Eli Zaretskii
2017-10-17 7:25 ` Tino Calancha
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).