> --- > lisp/menu-bar.el | 2 +- > lisp/select.el | 91 ++++++++++++++++++++++++++++++++++----------- > lisp/term/pc-win.el | 8 ++++ > 3 files changed, 78 insertions(+), 23 deletions(-) > > diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el > index ab64928fe7..7ff5439f7c 100644 > --- a/lisp/menu-bar.el > +++ b/lisp/menu-bar.el > @@ -606,7 +606,7 @@ clipboard-yank > "Insert the clipboard contents, or the last stretch of killed text." > (interactive "*") > (let ((select-enable-clipboard t) > - ;; Ensure that we defeat the DWIM login in `gui-selection-value'. > + ;; Ensure that we defeat the DWIM logic in `gui-selection-value'. > (gui--last-selected-text-clipboard nil)) > (yank))) - DWIM login -> DWIM logic (typo) - (let ((gui--last-selected-text-clipboard nil))) still achieves the same after the changes > diff --git a/lisp/select.el b/lisp/select.el > index 42b50c44e6..708034f6bd 100644 > --- a/lisp/select.el > +++ b/lisp/select.el > @@ -25,9 +25,10 @@ > ;; Based partially on earlier release by Lucid. > > ;; The functionality here is divided in two parts: > -;; - Low-level: gui-get-selection, gui-set-selection, gui-selection-owner-p, > -;; gui-selection-exists-p are the backend-dependent functions meant to access > -;; various kinds of selections (CLIPBOARD, PRIMARY, SECONDARY). > +;; - Low-level: gui-backend-get-selection, gui-backend-set-selection, > +;; gui-backend-selection-owner-p, gui-backend-selection-exists-p are > +;; the backend-dependent functions meant to access various kinds of > +;; selections (CLIPBOARD, PRIMARY, SECONDARY). > ;; - Higher-level: gui-select-text and gui-selection-value go together to > ;; access the general notion of "GUI selection" for interoperation with other > ;; applications. This can use either the clipboard or the primary selection, - gui-selection-owner-p -> gui-backend-selection-owner-p (the former does not exist) - gui-selection-exists-p -> gui-backend-selection-exists-p (the former does not exist) - gui-get-selection -> gui-backend-get-selection (the former does exist, but the later is lower lever, and I assumed that if the previous needed to be updated this one probably too) - gui-set-selection -> gui-backend-set-selection (same as above) > @@ -108,9 +109,10 @@ select-enable-primary > :group 'killing > :version "25.1") > > -;; We keep track of the last text selected here, so we can check the > -;; current selection against it, and avoid passing back our own text > -;; from gui-selection-value. We track both > +;; We keep track of the last selection here, so we can check the > +;; current selection against it, and avoid passing back with > +;; gui-selection-value the same text we previously killed or > +;; yanked. We track both > ;; separately in case another X application only sets one of them > ;; we aren't fooled by the PRIMARY or CLIPBOARD selection staying the same. > - Updated comment > @@ -119,6 +121,50 @@ gui--last-selected-text-clipboard > (defvar gui--last-selected-text-primary nil > "The value of the PRIMARY selection last seen.") > > +(defvar gui--last-selection-timestamp-clipboard nil > + "The timestamp of the CLIPBOARD selection last seen.") > +(defvar gui--last-selection-timestamp-primary nil > + "The timestamp of the PRIMARY selection last seen.") > + > +(defun gui--set-last-clipboard-selection (text) > + "Save last clipboard selection, to be able to check later whether > +it has changed. Save the selected text, and for window systems > +that support it, the selection timestamp." > + (setq gui--last-selected-text-clipboard text) > + (when (memq window-system '(x haiku)) > + (setq gui--last-selection-timestamp-clipboard > + (gui-get-selection 'CLIPBOARD 'TIMESTAMP)))) > + > +(defun gui--set-last-primary-selection (text) > + "Save last primary selection, to be able to check later whether > +it has changed. Save the selected text, and for window systems > +that support it, the selection timestamp." > + (setq gui--last-selected-text-primary text) > + (when (memq window-system '(x haiku)) > + (setq gui--last-selection-timestamp-primary > + (gui-get-selection 'PRIMARY 'TIMESTAMP)))) > + > +(defun gui--clipboard-selection-unchanged-p (text) > + "Check whether the clipboard selection is the same as the last > +time it was read, by comparing the selected text, and for window > +systems that support it, the selection timestamp." > + (and > + (equal text gui--last-selected-text-clipboard) > + (or (not (memq window-system '(x haiku))) > + (eq gui--last-selection-timestamp-clipboard > + (gui-get-selection 'CLIPBOARD 'TIMESTAMP))))) > + > +(defun gui--primary-selection-unchanged-p (text) > + "Check whether the primary selection is the same as the last > +time it was read, by comparing the selected text, and for window > +systems that support it, the selection timestamp." > + (and > + (equal text gui--last-selected-text-primary) > + (or (not (memq window-system '(x haiku))) > + (eq gui--last-selection-timestamp-primary > + (gui-get-selection 'PRIMARY 'TIMESTAMP))))) > + > + > (defun gui-select-text (text) > "Select TEXT, a string, according to the window system. > if `select-enable-clipboard' is non-nil, copy TEXT to the system's clipboard. - New variables gui--last-selection-timestamp-clipboard and gui--last-selection-timestamp-primary. - New functions gui--set-last-clipboard-selection and gui--set-last-primary-selection. - New functions gui--clipboard-selection-unchanged-p and gui--primary-selection-unchanged-p. > @@ -127,14 +173,14 @@ gui-select-text > MS-Windows does not have a \"primary\" selection." > (when select-enable-primary > (gui-set-selection 'PRIMARY text) > - (setq gui--last-selected-text-primary text)) > + (gui--set-last-primary-selection text)) > (when select-enable-clipboard > ;; When cutting, the selection is cleared and PRIMARY > ;; set to the empty string. Prevent that, PRIMARY > ;; should not be reset by cut (Bug#16382). > (setq saved-region-selection text) > (gui-set-selection 'CLIPBOARD text) > - (setq gui--last-selected-text-clipboard text))) > + (gui--set-last-clipboard-selection text))) > (define-obsolete-function-alias 'x-select-text 'gui-select-text "25.1") > > (defcustom x-select-request-type nil Using new functions instead > @@ -175,6 +221,7 @@ gui--selection-value-internal > ;; some other window systems. > (memq window-system '(x haiku)) > (eq type 'CLIPBOARD) > + ;; Should we unify this with the DWIM logic? > (gui-backend-selection-owner-p type)) > (let ((request-type (if (memq window-system '(x pgtk haiku)) > (or x-select-request-type I consider that check to be conceptually part of the same DWIM logic, and feel that maybe both checks should go together in the code. However I decided to preserve the code structure for now, since I wanted to make as little changes as possible, having the ownership check there can save unnecessary computations, and the check is mostly redundant now otherwise. > @@ -197,19 +244,18 @@ gui-selection-value > (let ((text (gui--selection-value-internal 'CLIPBOARD))) > (when (string= text "") > (setq text nil)) > - ;; When `select-enable-clipboard' is non-nil, > - ;; killing/copying text (with, say, `C-w') will push the > - ;; text to the clipboard (and store it in > - ;; `gui--last-selected-text-clipboard'). We check > - ;; whether the text on the clipboard is identical to this > - ;; text, and if so, we report that the clipboard is > - ;; empty. See (bug#27442) for further discussion about > - ;; this DWIM action, and possible ways to make this check > - ;; less fragile, if so desired. > + ;; Check the CLIPBOARD selection for 'newness', i.e., > + ;; whether it is different from the last time we did a > + ;; yank operation or whether it was set by Emacs itself > + ;; with a kill operation, since in both cases the text > + ;; will already be in the kill ring. See (bug#27442) and > + ;; (bug#53894) for further discussion about this DWIM > + ;; action, and possible ways to make this check less > + ;; fragile, if so desired. Updated comment > (prog1 > - (unless (equal text gui--last-selected-text-clipboard) > + (unless (gui--clipboard-selection-unchanged-p text) > text) > - (setq gui--last-selected-text-clipboard text))))) > + (gui--set-last-clipboard-selection text))))) > (primary-text > (when select-enable-primary > (let ((text (gui--selection-value-internal 'PRIMARY))) Using new functions instead > @@ -218,9 +264,9 @@ gui-selection-value > ;; from what we remembered them to be last time we did a > ;; cut/paste operation. > (prog1 > - (unless (equal text gui--last-selected-text-primary) > + (unless (gui--primary-selection-unchanged-p text) > text) > - (setq gui--last-selected-text-primary text)))))) > + (gui--set-last-primary-selection text)))))) > > ;; As we have done one selection, clear this now. > (setq next-selection-coding-system nil) Using new functions instead > @@ -239,7 +285,8 @@ gui-selection-value > ;; timestamps there is no way to know what the 'correct' value to > ;; return is. The nice thing to do would be to tell the user we > ;; saw multiple possible selections and ask the user which was the > - ;; one they wanted. > + ;; one they wanted. EDIT: We do have timestamps now (for most > + ;; window systems), so we can return the newer. > (or clip-text primary-text) > )) > > diff --git a/lisp/term/pc-win.el b/lisp/term/pc-win.el > index 327d51f275..289a1414c6 100644 > --- a/lisp/term/pc-win.el > +++ b/lisp/term/pc-win.el An unrelated issue which could be solved with timestamps, but the comment must be old and says we don't have them. Just updated the comment saying we do have them now. > @@ -246,6 +246,14 @@ w16-selection-owner-p > ;; if it does not exist, or exists and compares > ;; equal with the last text we've put into the > ;; Windows clipboard. > + ;; EDIT: that variable is actually the last text any program > + ;; (not just Emacs) has put into the windows clipboard (up > + ;; until the last time Emacs read or set the clipboard), so > + ;; it's not suitable for checking actual selection > + ;; ownership. This does not result in a bug for any of the > + ;; current uses of gui-backend-selection-owner however, since > + ;; they don't actually care about selection ownership, but > + ;; about the selected text having changed. > (cond > ((not text) t) > ((equal text gui--last-selected-text-clipboard) text) > -- A comment on the only other use of gui--last-selected-text-clipboard I found, since it is and was incorrect. It behaves exactly the same after the new changes though.