Subject: Re: bug#53894: 27.2; Clipboard copy -> C-y -> M-y -> Same clipboard copy does not push to kill ring

From: Po Lu <[email-masked]>
Date: Tue, 29 Mar 2022 21:00:50 +0800

Ignacio Casso <[email-masked]> writes:

> Ok, here is the final patch, and a suggested commit message. I've
> initiated the paperwork for the copyright assignment, I will let you
> know when it is done.

Thanks.  A last-minute comment:

> +(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))
> +    (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))
> +    (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)))
> +       (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)))
> +       (eq gui--last-selection-timestamp-primary
> +           (gui-get-selection 'PRIMARY 'TIMESTAMP)))))

These checks should use `gui-backend-get-selection' to find the
selection timestamp, not the public function.  Also, the doc
strings of the new functions should explain the meaning of the
argument `text', and the first line of each doc string should be
a complete sentence not more than 70 characters wide, terminated
by a period.

> -    ;; one they wanted.
> +    ;; one they wanted. EDIT: We do have timestamps or selection
> +    ;; counter now (for some window systems), so we can return the
> +    ;; newer.

Perhaps the entire comment could be reworded instead of adding
an edit to the end.  There should also be two spaces between
punctuation and any further text, like this: "wanted.  EDIT: We
do...".

Thanks.