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

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

[Ignacio Casso]

Hello,

I've noticed that if you copy something to the clipboard outside of
Emacs, then rotate the kill ring in Emacs without adding anything new to
it, and then copy the same text again to the clipboard outside of Emacs
(not necessarily in the same place of application, just the same string
anywhere), that text is not pushed again to the top of the kill ring.

I'm not sure if it's intentional or not, or even if Emacs can do
anything about it (maybe since the text is the same, the clipboard is not
actually modified and Emacs has no way to know). I just wanted to report
it in case it is a bug.

This happens starting Emacs with "emacs -Q", in an Ubuntu 20.04.3 with
GNOME as window manager (although that's probably not enough information
to reproduce it if it's the clipboard's manager fault).

Thanks for your time,

Ignacio



In GNU Emacs 27.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20)
 of 2022-01-16 built on ignacio-IdeaPad-3-15ADA05
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Ubuntu 20.04.3 LTS

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

[Lars Ingebrigtsen]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> I'm not sure if it's intentional or not, or even if Emacs can do
> anything about it (maybe since the text is the same, the clipboard is not
> actually modified and Emacs has no way to know). I just wanted to report
> it in case it is a bug.

It's intentional:

(defun gui-selection-value ()
  (let ((clip-text
         (when select-enable-clipboard
           (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.
             (prog1
                 (unless (equal text gui--last-selected-text-clipboard)
                   text)
               (setq gui--last-selected-text-clipboard text)))))

But it's pretty fragile DWIM action, and is frequently not what I mean,
so I often resort to `clipboard-yank' instead to ensure that I really
get the clipboard contents.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Ignacio Casso]

Hi Lars,

I read the debbugs.gnu.org thread about bug#27442 where you try to
figure out the reason behind that logic, and I think the conclusion you
arrive at is incomplete and that the bug I reported might give some more
insight about it. I might be wrong since I have not given that much
though to this, but here is what I mean:

Every time we yank, interprogram-paste-function is called. If it returns
nil we yank from the kill-ring, otherwise the returned string is pushed
to the kill-ring and yanked.

If we then rotate the kill ring, we do not want to paste from the
clipboard anymore, so interprogram-paste-function must return nil if
nothing newer has been copied to the clipboard. The way this is checked
is by just checking whether the clipboard content has changed.

If we do not rotate the kill ring, it doesn't matter if
interprogram-paste-function returns nil since the clipboard content is
already at the top of the kill-ring. In fact we need it to return nil in
order not to duplicate it in the kill ring.

So basically, while the content of the clipboard remains the same, we
want gui-selection-value to return it only the first time we call it
(for this use case at least), and from then onwards the kill ring will
handle it. That would be another reason behind that
logic aside from the one you thought of: ignoring clipboard content if it
was put there by Emacs itself with kill commands (since it's already in
the kill ring too).

This would also answer my original question: unless there was a
timestamp in the clipboard Emacs has indeed no way to know right now if
the exact same string was copied to the clipboard again in-between (in
which case I would expect it to be pushed to the top of the kill-ring
again)

Does it make sense?

Sorry if you had already figured it out or if I am wrong, I did not
understand the whole email thread nor debug this issue in depth. Or if
it's no longer relevant, but since I saw the email thread was less than
one year old I thought that you might want to know.

Regards,

Ignacio


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> I'm not sure if it's intentional or not, or even if Emacs can do
>> anything about it (maybe since the text is the same, the clipboard is not
>> actually modified and Emacs has no way to know). I just wanted to report
>> it in case it is a bug.
>
> It's intentional:
>
> (defun gui-selection-value ()
>   (let ((clip-text
>          (when select-enable-clipboard
>            (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.
>              (prog1
>                  (unless (equal text gui--last-selected-text-clipboard)
>                    text)
>                (setq gui--last-selected-text-clipboard text)))))
>
> But it's pretty fragile DWIM action, and is frequently not what I mean,
> so I often resort to `clipboard-yank' instead to ensure that I really
> get the clipboard contents.

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

[Lars Ingebrigtsen]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> This would also answer my original question: unless there was a
> timestamp in the clipboard Emacs has indeed no way to know right now if
> the exact same string was copied to the clipboard again in-between (in
> which case I would expect it to be pushed to the top of the kill-ring
> again)

On X (at the very least), the clipboard does have a timestamp, though.
But we don't use it, which seems like the bug here.  Probably.

Po Lu's done some work in this area lately; added to the CCs.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> This would also answer my original question: unless there was a
>> timestamp in the clipboard Emacs has indeed no way to know right now if
>> the exact same string was copied to the clipboard again in-between (in
>> which case I would expect it to be pushed to the top of the kill-ring
>> again)
>
> On X (at the very least), the clipboard does have a timestamp, though.
> But we don't use it, which seems like the bug here.  Probably.
>
> Po Lu's done some work in this area lately; added to the CCs.

Every platform except MS-Windows has `x-selection-owner-p' or something
to that effect which tells Emacs whether or not it was the last program
to have saved to a selection (i.e., the clipboard, or the primary
selection.)

On the platforms where it does work reliably, Emacs uses it to determine
whether or not to return nil from the interprogram-paste-function, so I
don't see a problem here.

Eli says MS-Windows also does something similar, but using a different
mechanism.

Thanks.

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

[Eli Zaretskii]

> Cc: Ignacio Casso <ignaciocasso@hotmail.com>, 53894@debbugs.gnu.org
> Date: Thu, 10 Feb 2022 09:56:08 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Every platform except MS-Windows has `x-selection-owner-p' or something
> to that effect which tells Emacs whether or not it was the last program
> to have saved to a selection (i.e., the clipboard, or the primary
> selection.)
> 
> On the platforms where it does work reliably, Emacs uses it to determine
> whether or not to return nil from the interprogram-paste-function, so I
> don't see a problem here.
> 
> Eli says MS-Windows also does something similar, but using a different
> mechanism.

Yes.  But (a) I don't think this report is about MS-Windows, and (b) I
haven't seen a complete reproduction recipe to try this on MS-Windows
and see how Emacs behaves there.  In general, on MS-Windows we do
internally something that closely emulates the ownership test, but it
cannot be 100% compatible for obvious reasons.

In addition, I believe the OP claims that the algorithm we use to
decide (on X) when to return nil is flawed, or at least ruins his use
case.  I don't think I succeed to understand the rationale for that
claim, FWIW.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Yes.  But (a) I don't think this report is about MS-Windows, and (b) I
> haven't seen a complete reproduction recipe to try this on MS-Windows
> and see how Emacs behaves there.  In general, on MS-Windows we do
> internally something that closely emulates the ownership test, but it
> cannot be 100% compatible for obvious reasons.
>
> In addition, I believe the OP claims that the algorithm we use to
> decide (on X) when to return nil is flawed, or at least ruins his use
> case.  I don't think I succeed to understand the rationale for that
> claim, FWIW.

I didn't get to read all of the big report in very much detail until
after replying to Lars' question, but I believe what was installed on
master in the past week has solved the OP's problem.

Thanks.

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

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> In addition, I believe the OP claims that the algorithm we use to
> decide (on X) when to return nil is flawed, or at least ruins his use
> case.  I don't think I succeed to understand the rationale for that
> claim, FWIW.

Here's the case to reproduce:

In external program, "copy" a text
In Emacs: C-y M-y
In external program, "copy" the same text
In Emacs: C-y

This C-y will not yank the text.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> I didn't get to read all of the big report in very much detail until
> after replying to Lars' question, but I believe what was installed on
> master in the past week has solved the OP's problem.

No, the problem is still present on the trunk.  But I suspect that after
you introduced the `gui-backend-selection-owner-p' test in
gui--selection-value-internal, the logic surrounding
gui--last-selected-text-clipboard in gui-selection-value is no longer
necessary, and should just be removed now.

But I haven't analysed the problem in depth, and I don't know how this
works on non-X systems -- perhaps that DWIM logic in gui-selection-value
is still necessary on some systems.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> No, the problem is still present on the trunk.  But I suspect that after
> you introduced the `gui-backend-selection-owner-p' test in
> gui--selection-value-internal, the logic surrounding
> gui--last-selected-text-clipboard in gui-selection-value is no longer
> necessary, and should just be removed now.

Indeed, it should be removed for the platforms where
`gui-backend-selection-owner-p' does work.

> But I haven't analysed the problem in depth, and I don't know how this
> works on non-X systems -- perhaps that DWIM logic in gui-selection-value
> is still necessary on some systems.

It is, `gui-backend-selection-owner-p' does not work reliably on some
other platforms such as PGTK and NS.

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Po Lu <luangruo@yahoo.com>,  ignaciocasso@hotmail.com,
>   53894@debbugs.gnu.org
> Date: Thu, 10 Feb 2022 07:39:51 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > In addition, I believe the OP claims that the algorithm we use to
> > decide (on X) when to return nil is flawed, or at least ruins his use
> > case.  I don't think I succeed to understand the rationale for that
> > claim, FWIW.
> 
> Here's the case to reproduce:
> 
> In external program, "copy" a text
> In Emacs: C-y M-y
> In external program, "copy" the same text
> In Emacs: C-y
> 
> This C-y will not yank the text.

If by "the text" you mean the text copied the second time from another
application, then Emacs on MS-Windows behaves the same as on X in this
scenario.

So now the only issue left (for me) is that I don't think I understand
why the OP doesn't like this: after all, the original text from the
clipboard is still available in the kill ring, and can be pasted given
enough M-y, or using the Edit menu-bar menu, or using the new feature
of M-y when it is invoked not after C-y or another M-y.

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

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

>> But I haven't analysed the problem in depth, and I don't know how this
>> works on non-X systems -- perhaps that DWIM logic in gui-selection-value
>> is still necessary on some systems.
>
> It is, `gui-backend-selection-owner-p' does not work reliably on some
> other platforms such as PGTK and NS.

Do we have a list of these platforms?  Perhaps we should have a function
that says whether the `gui-backend-selection-owner-p' results are
reliable...?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> If by "the text" you mean the text copied the second time from another
> application, then Emacs on MS-Windows behaves the same as on X in this
> scenario.

Yes.

> So now the only issue left (for me) is that I don't think I understand
> why the OP doesn't like this: after all, the original text from the
> clipboard is still available in the kill ring, and can be pasted given
> enough M-y, or using the Edit menu-bar menu, or using the new feature
> of M-y when it is invoked not after C-y or another M-y.

But it's surprising (and frustrating) to click on "Copy text" in a
different application and then (sometimes) not being able to yank it
with `C-y'.  So if we can fix that (and it sounds like we can (on most
platforms) now, since Po Lu introduced the
`gui-backend-selection-owner-p' function), we should.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> It is, `gui-backend-selection-owner-p' does not work reliably on some
>> other platforms such as PGTK and NS.

> Do we have a list of these platforms?  Perhaps we should have a function
> that says whether the `gui-backend-selection-owner-p' results are
> reliable...?

IME, it's PGTK on Wayland and GNUstep where it doesn't work reliably.
It seems to work fine on macOS, and it's guaranteed to work on X (if it
doesn't, then selections won't work at all) and Haiku.

Thanks.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> platforms) now, since Po Lu introduced the
> `gui-backend-selection-owner-p' function), we should.

Minor correction: I didn't introduce that function, I only made
`gui-selection-value' take advantage of it on platforms where it works.

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

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

>> platforms) now, since Po Lu introduced the
>> `gui-backend-selection-owner-p' function), we should.
>
> Minor correction: I didn't introduce that function, I only made
> `gui-selection-value' take advantage of it on platforms where it works.

Ah, right.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: luangruo@yahoo.com,  ignaciocasso@hotmail.com,  53894@debbugs.gnu.org
> Date: Thu, 10 Feb 2022 09:43:58 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So now the only issue left (for me) is that I don't think I understand
> > why the OP doesn't like this: after all, the original text from the
> > clipboard is still available in the kill ring, and can be pasted given
> > enough M-y, or using the Edit menu-bar menu, or using the new feature
> > of M-y when it is invoked not after C-y or another M-y.
> 
> But it's surprising (and frustrating) to click on "Copy text" in a
> different application and then (sometimes) not being able to yank it
> with `C-y'.  So if we can fix that (and it sounds like we can (on most
> platforms) now, since Po Lu introduced the
> `gui-backend-selection-owner-p' function), we should.

Using paste from the clipboard mixed with C-y/M-y will always be
tricky, no matter what we do.  That's why we have
save-interprogram-paste-before-kill.

But if this can be made less tricky, I don't mind doing that, of
course.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Using paste from the clipboard mixed with C-y/M-y will always be
> tricky, no matter what we do.  That's why we have
> save-interprogram-paste-before-kill.
>
> But if this can be made less tricky, I don't mind doing that, of
> course.

If nobody minds, I'll install a change that disables the DWIM behaviour
when the ownership information is reliable.

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

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  ignaciocasso@hotmail.com,
>   53894@debbugs.gnu.org
> Date: Thu, 10 Feb 2022 20:14:36 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Using paste from the clipboard mixed with C-y/M-y will always be
> > tricky, no matter what we do.  That's why we have
> > save-interprogram-paste-before-kill.
> >
> > But if this can be made less tricky, I don't mind doing that, of
> > course.
> 
> If nobody minds, I'll install a change that disables the DWIM behaviour
> when the ownership information is reliable.

I do mind: having different behaviors on different platforms is not
ideal, to say the least, especially for maintenance.  But I'm not sure
this is strong enough a reason not to make the change in this case.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> I do mind: having different behaviors on different platforms is not
> ideal, to say the least, especially for maintenance.  But I'm not sure
> this is strong enough a reason not to make the change in this case.

Hmm, then I'll wait a bit to see what the others think.

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

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

>> I do mind: having different behaviors on different platforms is not
>> ideal, to say the least, especially for maintenance.  But I'm not sure
>> this is strong enough a reason not to make the change in this case.
>
> Hmm, then I'll wait a bit to see what the others think.

We'll have the same behaviour on all the major platforms, so I think
it's worth changing.  If it turns out to be annoying in practice, we can
always revert later.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Ignacio Casso]

Hi all,

Did you read my last email? I think you got some things wrong. I'll
explain it again inline.

First, about my original concern:

>> In addition, I believe the OP claims that the algorithm we use to
>> decide (on X) when to return nil is flawed, or at least ruins his use
>> case.  I don't think I succeed to understand the rationale for that
>> claim, FWIW.
>
> Here's the case to reproduce:
>
> In external program, "copy" a text
> In Emacs: C-y M-y
> In external program, "copy" the same text
> In Emacs: C-y
>
> This C-y will not yank the text.

Lars is right here

>> So now the only issue left (for me) is that I don't think I understand
>> why the OP doesn't like this: after all, the original text from the
>> clipboard is still available in the kill ring, and can be pasted given
>> enough M-y, or using the Edit menu-bar menu, or using the new feature
>> of M-y when it is invoked not after C-y or another M-y.
>
> But it's surprising (and frustrating) to click on "Copy text" in a
> different application and then (sometimes) not being able to yank it
> with `C-y'.

And here too. But I don't really care that much, I was just reporting
it. As you said, you can find it in the kill ring or use clipboard-yank,
and it's a really uncommon scenario.

And now about the DWIM logic in gui-selection-value:

> No, the problem is still present on the trunk.  But I suspect that after
> you introduced the `gui-backend-selection-owner-p' test in
> gui--selection-value-internal, the logic surrounding
> gui--last-selected-text-clipboard in gui-selection-value is no longer
> necessary, and should just be removed now.
>
> But I haven't analysed the problem in depth, and I don't know how this
> works on non-X systems -- perhaps that DWIM logic in gui-selection-value
> is still necessary on some systems.

That logic is still necessary. gui-selection-value is called every time
we yank, and we only want it to return non-nil when there is something
new there. If there is, it's pushed to the top of the kill ring and
yanked, and from then onwards the kill ring handles it. Otherwise, i.e.,
if its the same as gui--last-selected-text-clipboard, it returns nil. So
gui-selection-value will almost always return nil for that reason, and
it's in fact gui-backend-selection-owner-p what is probably unnecessary,
since it achieves the same as setting gui--last-selected-text-clipboard
to the string being killed in the first place, which gui-select-text
already does.

In particular, removing that logic would not solve the problem I
reported. The only way to solve it would be with clipboard
timestamps, which are probably not worth to use if this is the only
problem they solve (although I saw a comment about them being needed
also to resolve discrepancies between clipboard and primary selection).

Regards,

--Ignacio

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

[Ignacio Casso]

Hello again,

After thinking a little bit more about it I realize I have jumped to
conclusions. Sorry about that.

Since in the thread of bug #27442 you focused on the
problem of Emacs being the author of the last clipboard copy, I assumed
that gui-backend-selection-owner-p meant just that. But for that a
variable would suffice, and I see now that it is a low level function,
which probably means that the concept of ownership is at a Window
Manager level. If that's the case, if as soon as Emacs reads the
clipboard it asserts ownership of it without modifying it, that can be
used to know when there is something new in the clipboard, since the
owner will have changed to the application who put it there. Then
gui-backend-selection-owner-p would indeed solve all the problems,
gui--last-selected-text-clipboard would no longer be needed, and if it
was removed it would solve the bug I reported.

I should have known better than to think I could be right and you wrong
about some Emacs code I've just started looking at. Sorry about that.

Regards,

--Ignacio


Ignacio Casso <ignaciocasso@hotmail.com> writes:

> Hi all,
>
> Did you read my last email? I think you got some things wrong. I'll
> explain it again inline.
>
> First, about my original concern:
>
>>> In addition, I believe the OP claims that the algorithm we use to
>>> decide (on X) when to return nil is flawed, or at least ruins his use
>>> case.  I don't think I succeed to understand the rationale for that
>>> claim, FWIW.
>>
>> Here's the case to reproduce:
>>
>> In external program, "copy" a text
>> In Emacs: C-y M-y
>> In external program, "copy" the same text
>> In Emacs: C-y
>>
>> This C-y will not yank the text.
>
> Lars is right here
>
>>> So now the only issue left (for me) is that I don't think I understand
>>> why the OP doesn't like this: after all, the original text from the
>>> clipboard is still available in the kill ring, and can be pasted given
>>> enough M-y, or using the Edit menu-bar menu, or using the new feature
>>> of M-y when it is invoked not after C-y or another M-y.
>>
>> But it's surprising (and frustrating) to click on "Copy text" in a
>> different application and then (sometimes) not being able to yank it
>> with `C-y'.
>
> And here too. But I don't really care that much, I was just reporting
> it. As you said, you can find it in the kill ring or use clipboard-yank,
> and it's a really uncommon scenario.
>
> And now about the DWIM logic in gui-selection-value:
>
>> No, the problem is still present on the trunk.  But I suspect that after
>> you introduced the `gui-backend-selection-owner-p' test in
>> gui--selection-value-internal, the logic surrounding
>> gui--last-selected-text-clipboard in gui-selection-value is no longer
>> necessary, and should just be removed now.
>>
>> But I haven't analysed the problem in depth, and I don't know how this
>> works on non-X systems -- perhaps that DWIM logic in gui-selection-value
>> is still necessary on some systems.
>
> That logic is still necessary. gui-selection-value is called every time
> we yank, and we only want it to return non-nil when there is something
> new there. If there is, it's pushed to the top of the kill ring and
> yanked, and from then onwards the kill ring handles it. Otherwise, i.e.,
> if its the same as gui--last-selected-text-clipboard, it returns nil. So
> gui-selection-value will almost always return nil for that reason, and
> it's in fact gui-backend-selection-owner-p what is probably unnecessary,
> since it achieves the same as setting gui--last-selected-text-clipboard
> to the string being killed in the first place, which gui-select-text
> already does.
>
> In particular, removing that logic would not solve the problem I
> reported. The only way to solve it would be with clipboard
> timestamps, which are probably not worth to use if this is the only
> problem they solve (although I saw a comment about them being needed
> also to resolve discrepancies between clipboard and primary selection).
>
> Regards,
>
> --Ignacio

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> That logic is still necessary. gui-selection-value is called every time
> we yank, and we only want it to return non-nil when there is something
> new there. If there is, it's pushed to the top of the kill ring and
> yanked, and from then onwards the kill ring handles it. Otherwise, i.e.,
> if its the same as gui--last-selected-text-clipboard, it returns nil. So
> gui-selection-value will almost always return nil for that reason, and
> it's in fact gui-backend-selection-owner-p what is probably unnecessary,
> since it achieves the same as setting gui--last-selected-text-clipboard
> to the string being killed in the first place, which gui-select-text
> already does.
>
> In particular, removing that logic would not solve the problem I
> reported. The only way to solve it would be with clipboard
> timestamps, which are probably not worth to use if this is the only
> problem they solve (although I saw a comment about them being needed
> also to resolve discrepancies between clipboard and primary selection).

You have the wrong mental model of how X selections work.  You can't
"set" a selection, but instead you assert ownership of it, which means
other clients will then ask you for its contents.  (Which are always
kept by you, and never sent to the X server, unless you exit and a
clipboard manager is willing to take ownership of the selection.)

If Emacs wants to get the value of CLIPBOARD, then it has to first find
out which client has ownership of that selection, and to ask it for the
value of CLIPBOARD.  If that ownership information is unavailable, then
selections will not work at all, so the selection ownership information
is guaranteed to be reliable as long as selections are working.

Other clipboard systems typically have a counter that is atomically
incremented each time something is saved to the clipboard.  The client
can save the value of the counter before it has saved something to the
clipboard, and compare that to the current value of the counter, to
determine whether or not it was the last client to have set the value of
the clipboard.

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

[Lars Ingebrigtsen]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> I should have known better than to think I could be right and you wrong
> about some Emacs code I've just started looking at. Sorry about that.

No problem.  It's one of the many joys of working on a code base that's
up to almost 40 years old: First you have to figure out what the (no
doubt smart) programmer meant to achieve with the code, and then try to
figure out whether it ever even did that, and then whether it's still
working the same way, and then whether it's still relevant due to
changes elsewhere, and then finally whether it can be improved without
breaking odd edge cases on obscure systems you don't have access to.  🙃

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Po Lu <luangruo@yahoo.com>,  Eli Zaretskii <eliz@gnu.org>,
>   53894@debbugs.gnu.org
> Date: Fri, 11 Feb 2022 07:15:44 +0100
> 
> Ignacio Casso <ignaciocasso@hotmail.com> writes:
> 
> > I should have known better than to think I could be right and you wrong
> > about some Emacs code I've just started looking at. Sorry about that.
> 
> No problem.  It's one of the many joys of working on a code base that's
> up to almost 40 years old: First you have to figure out what the (no
> doubt smart) programmer meant to achieve with the code, and then try to
> figure out whether it ever even did that, and then whether it's still
> working the same way, and then whether it's still relevant due to
> changes elsewhere, and then finally whether it can be improved without
> breaking odd edge cases on obscure systems you don't have access to.  🙃

Thanks, this is now added to DEVEL.HUMOR.

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

[Ignacio Casso]

Thanks for clarifying, Po Lu!

One final question. Does then Emacs assert ownership of the selection
also when it reads it (C-y) and not only when it sets it (C-k), as I
assumed in my last email? Because otherwise I think my point still
holds, and I didn't see any use of gui-backend-set-selection inside
gui-selection-value (and I could not test it since I'm running Emacs
27).

But never mind, I'll leave it here since I stole too much of your time
already. Just one last minor thing: there is a very minor typo in the
comments on select.el in case you want to fix it (or at least it is in
the github mirror which seems to be up to date). It says:

;; 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).

But the last two functions are actually gui-backend-selection-owner-p and
gui-backend-selection-exists-p, the others do not exist. For the first
two, they exist for both prefixes gui- and gui-backend-, the second
being lower level.

Regards,

Ignacio

Po Lu <luangruo@yahoo.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> That logic is still necessary. gui-selection-value is called every time
>> we yank, and we only want it to return non-nil when there is something
>> new there. If there is, it's pushed to the top of the kill ring and
>> yanked, and from then onwards the kill ring handles it. Otherwise, i.e.,
>> if its the same as gui--last-selected-text-clipboard, it returns nil. So
>> gui-selection-value will almost always return nil for that reason, and
>> it's in fact gui-backend-selection-owner-p what is probably unnecessary,
>> since it achieves the same as setting gui--last-selected-text-clipboard
>> to the string being killed in the first place, which gui-select-text
>> already does.
>>
>> In particular, removing that logic would not solve the problem I
>> reported. The only way to solve it would be with clipboard
>> timestamps, which are probably not worth to use if this is the only
>> problem they solve (although I saw a comment about them being needed
>> also to resolve discrepancies between clipboard and primary selection).
>
> You have the wrong mental model of how X selections work.  You can't
> "set" a selection, but instead you assert ownership of it, which means
> other clients will then ask you for its contents.  (Which are always
> kept by you, and never sent to the X server, unless you exit and a
> clipboard manager is willing to take ownership of the selection.)
>
> If Emacs wants to get the value of CLIPBOARD, then it has to first find
> out which client has ownership of that selection, and to ask it for the
> value of CLIPBOARD.  If that ownership information is unavailable, then
> selections will not work at all, so the selection ownership information
> is guaranteed to be reliable as long as selections are working.
>
> Other clipboard systems typically have a counter that is atomically
> incremented each time something is saved to the clipboard.  The client
> can save the value of the counter before it has saved something to the
> clipboard, and compare that to the current value of the counter, to
> determine whether or not it was the last client to have set the value of
> the clipboard.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> One final question. Does then Emacs assert ownership of the selection
> also when it reads it (C-y)

Not that I know of, no; the ownership remains unchanged.

> and not only when it sets it (C-k), as I assumed in my last email?
> Because otherwise I think my point still holds

AFAIU the problem is that if you do this:

  - Kill some text in Emacs
  - C-y
  - Cut the same text in another program

then that same text is not pushed onto the kill ring again, when you
press C-y.  I don't understand why Emacs would have to own the clipboard
in order to fix that.

Thanks.

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

[Ignacio Casso]

Well my point in this case was gui--last-selected-text-clipboard being
still necessary, since you were suggesting to remove it in the email
thread:

>> No, the problem is still present on the trunk.  But I suspect that after
>> you introduced the `gui-backend-selection-owner-p' test in
>> gui--selection-value-internal, the logic surrounding
>> gui--last-selected-text-clipboard in gui-selection-value is no longer
>> necessary, and should just be removed now.
>
> Indeed, it should be removed for the platforms where
> `gui-backend-selection-owner-p' does work.


I'll explain what I mean one final time since you have not really
addressed it yet, so I don't know if I'm wrong or you didn't understood
what I meant. Otherwise I'll leave it at that, since there is no point
in having this discussion while I'm running Emacs 27 and have to look at
the most recent code in Github. I'll go step by step:

- gui-selection-value is called every time we yank (of course, since we
  need to know if anything new was added to the clipboard)

- When it returns non-nil, the text is pushed to the kill ring,
  according to C-h v interprogram-paste-function. From then onwards we
  want the kill ring to handle it.

- Therefore, the next time we yank, we don't want gui-selection-value to
  return the clipboard again, since it's already in the kill-ring, and
  it would be pushed again, duplicating the top of the kill-ring, or if
  it had rotated, pushing down the current top.

- Thus we need a mechanism to ensure we return nil if the clipboard has
  not changed. I understand that originally it was checking if the text
  was the same with gui--last-selected-text-clipboard.

- It was my impression that you though in the email thread of bug #27442
  that the only reason behind that variable was checking that it was not
  Emacs itself the one that had copied to the clipboard, in which case
  we also would want get-selection-value to return nil since the text
  would also be in the kill ring already. I pointed in my second email
  that the reason I just explained may be another, equally important
  reason, and insisted in the third.

- In my last emails I though, wrongly, that in Emacs 28 that mechanism
  had been replaced by asserting selection ownership (without altering
  the selection text, so other programs don't know the difference) as
  soon as the selection was read, and checking whether ownership had
  changed. This assumption was reinforced by you suggesting that
  gui--last-selected-text-clipboard was no longer necessary, which would
  indeed be true in that case, but not now, and you suggesting that
  removing it would fix the bug I originally posted, which again would
  be true in that case, but it is not now (it would, but it would
  introduce new, worse bugs).

I hope it is clearer now what I mean. Of course it all could be that my
mental model of clipboards or my understanding of
gui-backend-selection-value are still wrong: all this depends on my
assumption that repeated calls to gui-selection-value would return the
clipboard text over and over again as long as Emacs does not kill or
other application copies to the clipboard.

Regards,

Ignacio

Po Lu <luangruo@yahoo.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> One final question. Does then Emacs assert ownership of the selection
>> also when it reads it (C-y)
>
> Not that I know of, no; the ownership remains unchanged.
>
>> and not only when it sets it (C-k), as I assumed in my last email?
>> Because otherwise I think my point still holds
>
> AFAIU the problem is that if you do this:
>
>   - Kill some text in Emacs
>   - C-y
>   - Cut the same text in another program
>
> then that same text is not pushed onto the kill ring again, when you
> press C-y.  I don't understand why Emacs would have to own the clipboard
> in order to fix that.
>
> Thanks.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> - It was my impression that you though in the email thread of bug #27442
                                  ^^^^^^

> - In my last emails I though, wrongly, that in Emacs 28 that mechanism
                        ^^^^^^

I think I understand your point now, but is "though" here a typo for
"thought"?  That you repeat it twice makes me unsure.

Thanks.

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

[Ignacio Casso]

It is a typo indeed. Sorry, English is not my native language.

Po Lu <luangruo@yahoo.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> - It was my impression that you though in the email thread of bug #27442
>                                   ^^^^^^
>
>> - In my last emails I though, wrongly, that in Emacs 28 that mechanism
>                         ^^^^^^
>
> I think I understand your point now, but is "though" here a typo for
> "thought"?  That you repeat it twice makes me unsure.
>
> Thanks.

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

[Ignacio Casso]

And also, I realize now that in my second email Eli and you were not CCs
yet, in case that confuses you too. So when I enumerate my emails, I
mean:

- 1º: Bug report to bug-gnu-emacs@gnu.org

- 2º: Wed, 09 Feb to Lars, Cc 53894@debbugs.gnu.org, in which I
  explained most of this.

- 3º: Thu, 10 Feb to Lars, Cc Po Lu, Eli Zaretskii,
  53894@debbugs.gnu.org, in which I insisted with less detail, assuming
  (probably wrongly, I realize now) that Eli and you had read the
  second.

- 4º: The email immediately after, to Lars, Cc Po Lu, Eli Zaretskii,
  53894@debbugs.gnu.org, in which I retract what I said, thinking that
  Emacs now asserts ownership of the selection as soon as it reads it,
  which would invalidate everything I had said.


--Ignacio


Ignacio Casso <ignaciocasso@hotmail.com> writes:

> It is a typo indeed. Sorry, English is not my native language.
>
> Po Lu <luangruo@yahoo.com> writes:
>
>> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>>
>>> - It was my impression that you though in the email thread of bug #27442
>>                                   ^^^^^^
>>
>>> - In my last emails I though, wrongly, that in Emacs 28 that mechanism
>>                         ^^^^^^
>>
>> I think I understand your point now, but is "though" here a typo for
>> "thought"?  That you repeat it twice makes me unsure.
>>
>> Thanks.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> It is a typo indeed. Sorry, English is not my native language.

Thanks, your analysis is indeed correct.  I will look into how other
programs handle this problem and ack.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu <luangruo@yahoo.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> writes:
>
>> It is a typo indeed. Sorry, English is not my native language.
>
> Thanks, your analysis is indeed correct.  I will look into how other
> programs handle this problem and ack.

It is OK to rely on the timestamp always increasing, since the X server
ensures that there will be no races.  The timestamp is in server time,
so there is no need to worry about it moving backwards if the system
time is moved backwards.

So if (x-get-selection 'CLIPBOARD 'TIMESTAMP) is larger than the last
time it was recorded, then it is safe to say that the selection has
"changed".

However, it will still be necessary to keep the DWIM logic on X as a
fallback in the case that the clipboard timestamp has not changed, since
some clients are badly behaved and only change their record of the
current period's selection if they already own it.

On other systems it should work to rely on the clipboard modification
counters.

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

[Ignacio Casso]

Hello again,

Sorry for not answering earlier, my original intention was just to
report the bug and clarify the misunderstanding and then leave you all
to discuss it since you would know better what to do. But I though this
could be a chance to make my first Emacs contribution so I'm reviving
the discussion.

> So if (x-get-selection 'CLIPBOARD 'TIMESTAMP) is larger than the last
> time it was recorded, then it is safe to say that the selection has
> "changed".
>
> However, it will still be necessary to keep the DWIM logic on X as a
> fallback in the case that the clipboard timestamp has not changed, since
> some clients are badly behaved and only change their record of the
> current period's selection if they already own it.

So I guess you mean that if the timestamp has not changed, we can't
assume that the selection has not changed either and we need to confirm
it by checking whether the selection text has changed. In other words,
consider that the selection has changed if and only if either the
timestamp or the text has.

If we implemented that, the only new case we would distinguish would be
when the text has not changed but the timestamp has, which is precisely
the case of the "bug" I reported.

If (x-get-selection 'CLIPBOARD 'TIMESTAMP) works out of the box right
now, maybe it's worth to implement it. And it it's not, we could still
update the documentation that reflects the previous, incomplete
understanding of the issue, e.g., in the last paragraph of the docstring
for interprogram-paste-function or in the comments of the definition of
gui-selection-value.

I volunteer to do both if you agree that it would be an improvement. Not
because it's actually important for me (again, all of this began because
I started reporting the bugs I had written down over my time using
Emacs, most of which I did not really care about), but because I see it
as a chance to get started in contributing to free software and Emacs.

Let me know what you think.

Regards,

Ignacio

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> So I guess you mean that if the timestamp has not changed, we can't
> assume that the selection has not changed either and we need to confirm
> it by checking whether the selection text has changed. In other words,
> consider that the selection has changed if and only if either the
> timestamp or the text has.

Indeed, yes.

> If we implemented that, the only new case we would distinguish would be
> when the text has not changed but the timestamp has, which is precisely
> the case of the "bug" I reported.
>
> If (x-get-selection 'CLIPBOARD 'TIMESTAMP) works out of the box right
> now, maybe it's worth to implement it. And it it's not, we could still
> update the documentation that reflects the previous, incomplete
> understanding of the issue, e.g., in the last paragraph of the docstring
> for interprogram-paste-function or in the comments of the definition of
> gui-selection-value.
>
> I volunteer to do both if you agree that it would be an improvement.

I do agree, any patches would be greatly welcome.  Thanks in advance.

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

[Ignacio Casso]

[-- Attachment #1: Type: text/plain, Size: 12856 bytes --]

>> I volunteer to do both if you agree that it would be an improvement.
>
> I do agree, any patches would be greatly welcome.  Thanks in advance.

I have installed the development version of Emacs and made a first patch
attempt, preserving the structure of the program as much as possible. I
have not tested thoroughly, but it works for all the cases I have
tried. I have attached it to this mail, and comment each change
below. Let me know what you think.

>  lisp/menu-bar.el           |  4 +--
>  lisp/obsolete/mouse-sel.el |  2 +-
>  lisp/select.el             | 70 +++++++++++++++++++++++---------------
>  lisp/term/pc-win.el        |  2 +-
>  5 files changed, 47 insertions(+), 33 deletions(-)
> 
> diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
> index ab64928fe7..01683f5c9f 100644
> --- a/lisp/menu-bar.el
> +++ b/lisp/menu-bar.el
> @@ -606,8 +606,8 @@ 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'.
> -        (gui--last-selected-text-clipboard nil))
> +        ;; Ensure that we defeat the DWIM logic in `gui-selection-value'.
> +        (gui--last-clipboard-selection-fingerprint nil))
>      (yank)))
>  
>  (defun clipboard-kill-ring-save (beg end &optional region)

- DWIM login -> DWIM logic (typo)

- replaced gui--last-selected-text-clipboard variable with the new one I
  have introduced (explained below)

- I have tested that clipboard-yank works for the following cases:
  - (setq select-enable-clipboard nil) -> copy in another program -> clipboard-yank
  another program.
  - copy in another program -> C-y -> M-y -> clipboard-yank

> diff --git a/lisp/obsolete/mouse-sel.el b/lisp/obsolete/mouse-sel.el
> index a9d6bfee60..fc91cc9fc1 100644
> --- a/lisp/obsolete/mouse-sel.el
> +++ b/lisp/obsolete/mouse-sel.el
> @@ -304,7 +304,7 @@ mouse-sel-get-selection-function
>      (if (eq selection 'PRIMARY)
>  	(or (gui-selection-value)
>  	    (bound-and-true-p x-last-selected-text-primary)
> -            gui--last-selected-text-primary)
> +            gui--last-selected-text-primary) ;; this variable no longer exists. Does code in lisp/obsolete/ need to be mantained?
>        (gui-get-selection selection)))
>    "Function to call to get the selection.
>  Called with one argument:

Here I have not replaced the variable, since I have assumed that code
under the obsolete/ directory does not need to be maintained, and C-h f
tells me that function is not loaded by default. However there is a
warning when compiling. Should I fix it?


> diff --git a/lisp/select.el b/lisp/select.el
> index 42b50c44e6..55c409d347 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,16 +109,24 @@ 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.
> +;;
> +;; TODO: add selection owner to fingerprint, since timestamp is not
> +;; always relieable? Probably not worth it, since right now we can't
> +;; get the owner with the low-level functions out of the box, and text
> +;; plus timestamp is probably a strong enough fingerprint already.
> +(defvar gui--last-clipboard-selection-fingerprint nil
> +  "The fingerprint of the CLIPBOARD selection last seen, which is a
> +list of value and timestamp.")
> +(defvar gui--last-primary-selection-fingerprint nil
> +  "The fingerprint of the PRIMARY selection last seen, which is a
> +list of value and timestamp.")
>  
> -(defvar gui--last-selected-text-clipboard nil
> -  "The value of the CLIPBOARD selection last seen.")
> -(defvar gui--last-selected-text-primary nil
> -  "The value of the PRIMARY selection last seen.")
>  
>  (defun gui-select-text (text)
>    "Select TEXT, a string, according to the window system.

- Using new variables gui--last-clipboard-selection-fingerprint and
  gui--last-primary-selection-fingerprint instead of
  gui--last-selected-text-clipboard and
  gui--last-selected-text-primary. They have the same purpose but hold
  the text and the timestamp instead of just the text, and I have
  updated the code everywhere they were referenced to use the
  text-timestamp pair instead of just the text . Better ideas for the
  names are welcome.

- Updated previous comment

- Just threw the idea that a better fingerprint would include the owner
  of the selection, if timestamps are sometimes not updated precisely
  when the owner changes, as Po Lu said.


> @@ -127,14 +136,16 @@ 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))
> +    (setq gui--last-primary-selection-fingerprint
> +          (list text (gui-get-selection 'PRIMARY 'TIMESTAMP))))
>    (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)))
> +    (setq gui--last-clipboard-selection-fingerprint
> +          (list text (gui-get-selection 'CLIPBOARD 'TIMESTAMP)))))
>  (define-obsolete-function-alias 'x-select-text 'gui-select-text "25.1")
>  
>  (defcustom x-select-request-type nil

Saving the text-timestamp pair instead of just text.

> @@ -175,6 +186,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.


> @@ -194,33 +206,34 @@ gui--selection-value-internal
>  (defun gui-selection-value ()
>    (let ((clip-text
>           (when select-enable-clipboard
> -           (let ((text (gui--selection-value-internal 'CLIPBOARD)))
> +           (let ((text (gui--selection-value-internal 'CLIPBOARD))
> +                 (timestamp (gui-get-selection 'CLIPBOARD 'TIMESTAMP)))
>               (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.
>               (prog1
> -                 (unless (equal text gui--last-selected-text-clipboard)
> +                 (unless (equal (list text timestamp)  gui--last-clipboard-selection-fingerprint)
>                     text)
> -               (setq gui--last-selected-text-clipboard text)))))
> +               (setq gui--last-clipboard-selection-fingerprint (list text timestamp))))))
>          (primary-text
>           (when select-enable-primary
> -           (let ((text (gui--selection-value-internal 'PRIMARY)))
> +           (let ((text (gui--selection-value-internal 'PRIMARY))
> +                 (timestamp (gui-get-selection 'PRIMARY 'TIMESTAMP)))
>               (if (string= text "") (setq text nil))
>               ;; Check the PRIMARY selection for 'newness', is it different
>               ;; 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 (equal (list text timestamp)  gui--last-primary-selection-fingerprint)
>                     text)
> -               (setq gui--last-selected-text-primary text))))))
> +               (setq gui--last-primary-selection-fingerprint (list text timestamp)))))))
>  
>      ;; As we have done one selection, clear this now.
>      (setq next-selection-coding-system nil)

- Using the timestamp-text pair instead of just text

- Updated comments

- I have tested the following cases:

  - Copy in another program -> C-y

  - Copy in other program -> C-y -> C-y -> M-y (the copied text is not
    duplicated in the kill ring, i.e., gui-selection-value returns nil
    for the second C-y)

  - C-k -> C-y -> M-y (the killed text is not duplicated in the kill
    ring, i.e., gui-selection-value returns nil)

  - Copy something in other program -> C-y -> M-y -> Copy the same thing
    in other program -> C-y (works as expected, i.e., the bug I reported
    is fixed)


> @@ -239,7 +252,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, so we can
> +    ;; return the newer.
>      (or clip-text primary-text)
>      ))
>  

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.

> diff --git a/lisp/term/pc-win.el b/lisp/term/pc-win.el
> index 327d51f275..2ae3cbd8b2 100644
> --- a/lisp/term/pc-win.el
> +++ b/lisp/term/pc-win.el
> @@ -248,7 +248,7 @@ w16-selection-owner-p
>          ;; Windows clipboard.
>          (cond
>           ((not text) t)
> -         ((equal text gui--last-selected-text-clipboard) text)
> +         ((equal text (car gui--last-clipbaord-selection-fingerprint)) t)
>           (t nil)))))
>  

- Replaced the check

- Returned t instead of text

- Have not tested since I don't use Windows, but it's semantically
  equivalent as before.



[-- Attachment #2: Patch for bug#53894 --]
[-- Type: text/x-diff, Size: 9973 bytes --]

From a81da62327c58c8351c366e8530bf6250a4ffaa2 Mon Sep 17 00:00:00 2001
From: Ignacio <ignacio.decasso@imdea.org>
Date: Mon, 28 Feb 2022 12:13:52 +0100
Subject: [PATCH] fixed bug

---
 etc/DEVEL.HUMOR            |  2 +-
 lisp/menu-bar.el           |  4 +--
 lisp/obsolete/mouse-sel.el |  2 +-
 lisp/select.el             | 70 +++++++++++++++++++++++---------------
 lisp/term/pc-win.el        |  2 +-
 5 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/etc/DEVEL.HUMOR b/etc/DEVEL.HUMOR
index bd51845cb1..e93631c331 100644
--- a/etc/DEVEL.HUMOR
+++ b/etc/DEVEL.HUMOR
@@ -203,4 +203,4 @@ it's still working the same way, and then whether it's still relevant
 due to changes elsewhere, and then finally whether it can be improved
 without breaking odd edge cases on obscure systems you don't have
 access to.  🙃"
-                                -- Ignacio Casso and Lars Ingebrigtsen
+                                -- Ignacio C. and Lars Ingebrigtsen
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index ab64928fe7..01683f5c9f 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -606,8 +606,8 @@ 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'.
-        (gui--last-selected-text-clipboard nil))
+        ;; Ensure that we defeat the DWIM logic in `gui-selection-value'.
+        (gui--last-clipboard-selection-fingerprint nil))
     (yank)))
 
 (defun clipboard-kill-ring-save (beg end &optional region)
diff --git a/lisp/obsolete/mouse-sel.el b/lisp/obsolete/mouse-sel.el
index a9d6bfee60..fc91cc9fc1 100644
--- a/lisp/obsolete/mouse-sel.el
+++ b/lisp/obsolete/mouse-sel.el
@@ -304,7 +304,7 @@ mouse-sel-get-selection-function
     (if (eq selection 'PRIMARY)
 	(or (gui-selection-value)
 	    (bound-and-true-p x-last-selected-text-primary)
-            gui--last-selected-text-primary)
+            gui--last-selected-text-primary) ;; this variable no longer exists. Does code in lisp/obsolete/ need to be mantained?
       (gui-get-selection selection)))
   "Function to call to get the selection.
 Called with one argument:
diff --git a/lisp/select.el b/lisp/select.el
index 42b50c44e6..55c409d347 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,
@@ -108,16 +109,24 @@ 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.
+;;
+;; TODO: add selection owner to fingerprint, since timestamp is not
+;; always relieable? Probably not worth it, since right now we can't
+;; get the owner with the low-level functions out of the box, and text
+;; plus timestamp is probably a strong enough fingerprint already.
+(defvar gui--last-clipboard-selection-fingerprint nil
+  "The fingerprint of the CLIPBOARD selection last seen, which is a
+list of value and timestamp.")
+(defvar gui--last-primary-selection-fingerprint nil
+  "The fingerprint of the PRIMARY selection last seen, which is a
+list of value and timestamp.")
 
-(defvar gui--last-selected-text-clipboard nil
-  "The value of the CLIPBOARD selection last seen.")
-(defvar gui--last-selected-text-primary nil
-  "The value of the PRIMARY selection last seen.")
 
 (defun gui-select-text (text)
   "Select TEXT, a string, according to the window system.
@@ -127,14 +136,16 @@ 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))
+    (setq gui--last-primary-selection-fingerprint
+          (list text (gui-get-selection 'PRIMARY 'TIMESTAMP))))
   (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)))
+    (setq gui--last-clipboard-selection-fingerprint
+          (list text (gui-get-selection 'CLIPBOARD 'TIMESTAMP)))))
 (define-obsolete-function-alias 'x-select-text 'gui-select-text "25.1")
 
 (defcustom x-select-request-type nil
@@ -175,6 +186,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
@@ -194,33 +206,34 @@ gui--selection-value-internal
 (defun gui-selection-value ()
   (let ((clip-text
          (when select-enable-clipboard
-           (let ((text (gui--selection-value-internal 'CLIPBOARD)))
+           (let ((text (gui--selection-value-internal 'CLIPBOARD))
+                 (timestamp (gui-get-selection 'CLIPBOARD 'TIMESTAMP)))
              (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.
              (prog1
-                 (unless (equal text gui--last-selected-text-clipboard)
+                 (unless (equal (list text timestamp)  gui--last-clipboard-selection-fingerprint)
                    text)
-               (setq gui--last-selected-text-clipboard text)))))
+               (setq gui--last-clipboard-selection-fingerprint (list text timestamp))))))
         (primary-text
          (when select-enable-primary
-           (let ((text (gui--selection-value-internal 'PRIMARY)))
+           (let ((text (gui--selection-value-internal 'PRIMARY))
+                 (timestamp (gui-get-selection 'PRIMARY 'TIMESTAMP)))
              (if (string= text "") (setq text nil))
              ;; Check the PRIMARY selection for 'newness', is it different
              ;; 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 (equal (list text timestamp)  gui--last-primary-selection-fingerprint)
                    text)
-               (setq gui--last-selected-text-primary text))))))
+               (setq gui--last-primary-selection-fingerprint (list text timestamp)))))))
 
     ;; As we have done one selection, clear this now.
     (setq next-selection-coding-system nil)
@@ -239,7 +252,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, 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..2ae3cbd8b2 100644
--- a/lisp/term/pc-win.el
+++ b/lisp/term/pc-win.el
@@ -248,7 +248,7 @@ w16-selection-owner-p
         ;; Windows clipboard.
         (cond
          ((not text) t)
-         ((equal text gui--last-selected-text-clipboard) text)
+         ((equal text (car gui--last-clipbaord-selection-fingerprint)) t)
          (t nil)))))
 
 ;; gui-set-selection is used in gui-set-selection.
-- 
2.25.1

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Eli Zaretskii <eliz@gnu.org>,
>  53894@debbugs.gnu.org
> Date: Mon, 28 Feb 2022 13:12:44 +0100
> 
> I have installed the development version of Emacs and made a first patch
> attempt, preserving the structure of the program as much as possible. I
> have not tested thoroughly, but it works for all the cases I have
> tried. I have attached it to this mail, and comment each change
> below. Let me know what you think.

Thanks.

My worry after skimming this is that you are making X-specific changes
in places that are used by non-X GUI environments and window-systems.
Can't we do all that entirely in X-specific code, so that the
probability of breaking other window-systems would be strictly zero?

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> Here I have not replaced the variable, since I have assumed that code
> under the obsolete/ directory does not need to be maintained, and C-h
> f tells me that function is not loaded by default. However there is a
> warning when compiling. Should I fix it?

I can do that instead, but see below.

> - Using new variables gui--last-clipboard-selection-fingerprint and
>   gui--last-primary-selection-fingerprint instead of
>   gui--last-selected-text-clipboard and
>   gui--last-selected-text-primary. They have the same purpose but hold
>   the text and the timestamp instead of just the text, and I have
>   updated the code everywhere they were referenced to use the
>   text-timestamp pair instead of just the text . Better ideas for the
>   names are welcome.

Can't you put a `timestamp' text property on strings stored inside the
old gui--last-selected-text-clipboard variable instead?

> - Just threw the idea that a better fingerprint would include the owner
>   of the selection, if timestamps are sometimes not updated precisely
>   when the owner changes, as Po Lu said.

What I said is that badly behaved clients do not change the timestamp to
take into account new data becoming available for clients they already
own.  The X server ensures that timestamps always increase with
selection ownership changes.

>> diff --git a/lisp/term/pc-win.el b/lisp/term/pc-win.el
>> index 327d51f275..2ae3cbd8b2 100644
>> --- a/lisp/term/pc-win.el
>> +++ b/lisp/term/pc-win.el
>> @@ -248,7 +248,7 @@ w16-selection-owner-p
>>          ;; Windows clipboard.
>>          (cond
>>           ((not text) t)
>> -         ((equal text gui--last-selected-text-clipboard) text)
>> +         ((equal text (car gui--last-clipbaord-selection-fingerprint)) t)
>>           (t nil)))))
>>  

> - Have not tested since I don't use Windows, but it's semantically
>   equivalent as before.

pc-win is for the MS-DOS port (which does work, and should be kept
working.)  However, I think this modification can be avoided by storing
the timestamp as a text property, which I proposed above.

Thanks.

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

[Ignacio Casso]

> My worry after skimming this is that you are making X-specific changes
> in places that are used by non-X GUI environments and window-systems.
> Can't we do all that entirely in X-specific code, so that the
> probability of breaking other window-systems would be strictly zero?

I've assumed that for GUI environments where (gui-get-selection
'CLIPBOARD 'TIMESTAMP') would not return an actual timestamp as
intended, it would return nil. Under that assumption (which I don't
really know if is correct), the new semantics for those systems is
exactly the same as before. But I can try to move the logic to x-win.el
or check the window-system variable when necessary. I will implement Po
Lu's suggestion of storing the timestamp as a text property first, and
then we can revisit this.

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

[Ignacio Casso]

> Can't you put a `timestamp' text property on strings stored inside the
> old gui--last-selected-text-clipboard variable instead?


Good idea. I'll implement it and send the new patch.

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

[Ignacio Casso]

>> Can't you put a `timestamp' text property on strings stored inside the
>> old gui--last-selected-text-clipboard variable instead?
>
>
> Good idea. I'll implement it and send the new patch.

I have implemented this, but I must admit I have never worked with text
properties before so I might have failed to consider unexpected effects
that these changes could have in other parts of the code. I also think
that using symbol properties for
gui--last-selected-text-clipboard/primary would be better, since a)
using text properties implies some checks that the variable values are
actually strings, and b) those text properties could be modified
elsewhere, since we don't know with which other elisp code those string
are shared. In particular, it could be the case that
gui--last-selected-text-clipboard and gui--last-selected-text-primary
shared the string (although if that was a problem it could easily be
solved by using different timestamp property names for clipboard and
primary). I've never worked with either text or symbol properties
before, so I'm not sure. What do you think?

I attach the new patch anyway and comment each section below:

>  lisp/select.el | 57 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 37 insertions(+), 20 deletions(-)
> 
> diff --git a/lisp/select.el b/lisp/select.el
> index 42b50c44e6..e4d3fc8bd0 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,

- Same as previous patch

> @@ -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, same as previous patch

> @@ -127,6 +129,8 @@ gui-select-text
>  MS-Windows does not have a \"primary\" selection."
>    (when select-enable-primary
>      (gui-set-selection 'PRIMARY text)
> +    (when (stringp text)
> +      (propertize text 'timestamp (gui-get-selection 'PRIMARY 'TIMESTAMP)))
>      (setq gui--last-selected-text-primary text))
>    (when select-enable-clipboard
>      ;; When cutting, the selection is cleared and PRIMARY

- Put timestamp property in text. Suggestions for the property name are
  welcome.

- Previously, calling (gui-select-text value) with a value that was not
  a string did not produce an error, so I make the stringp check to
  preserve that behavior although that situation should never happen.

> @@ -134,6 +138,8 @@ gui-select-text
>      ;; should not be reset by cut (Bug#16382).
>      (setq saved-region-selection text)
>      (gui-set-selection 'CLIPBOARD text)
> +    (when (stringp text)
> +      (propertize text 'timestamp (gui-get-selection 'CLIPBOARD 'TIMESTAMP)))
>      (setq gui--last-selected-text-clipboard text)))
>  (define-obsolete-function-alias 'x-select-text 'gui-select-text "25.1")

Same as above

> @@ -175,6 +181,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

Same as previous patch

> @@ -194,32 +201,41 @@ gui--selection-value-internal
>  (defun gui-selection-value ()
>    (let ((clip-text
>           (when select-enable-clipboard
> -           (let ((text (gui--selection-value-internal 'CLIPBOARD)))
> +           (let ((text (gui--selection-value-internal 'CLIPBOARD))
> +                 (timestamp (gui-get-selection 'CLIPBOARD 'TIMESTAMP))
> +                 (last-ts (when gui--last-selected-text-clipboard
> +                            (get-text-property 0 'timestamp gui--last-selected-text-clipboard))))

- getting new and old timestamp

- Is that the right way to get the property?

>               (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, same as previous patch

>               (prog1
> -                 (unless (equal text gui--last-selected-text-clipboard)
> +                 (unless (and (equal text gui--last-selected-text-clipboard)
> +                              (eq timestamp last-timestamp))
>                     text)
> +               (when text (propertize text 'timestamp timestamp))
>                 (setq gui--last-selected-text-clipboard text)))))

check text and timestamp equality, and update timestamp property

>          (primary-text
>           (when select-enable-primary
> -           (let ((text (gui--selection-value-internal 'PRIMARY)))
> +           (let ((text (gui--selection-value-internal 'PRIMARY))
> +                 (timestamp (gui-get-selection 'PRIMARY 'TIMESTAMP))
> +                 (last-timestamp (when gui--last-selected-text-primary
> +                            (get-text-property 0 'timestamp gui--last-selected-text-primary))))
>               (if (string= text "") (setq text nil))
>               ;; Check the PRIMARY selection for 'newness', is it different
>               ;; 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 (and (equal text gui--last-selected-text-primary)
> +                              (eq timestamp last-timestamp))
>                     text)
> +               (when text (put-text-property 0 1 'timestamp timestamp text))
>                 (setq gui--last-selected-text-primary text))))))
>  
>      ;; As we have done one selection, clear this now.

Same changes but for PRIMARY instead of CLIPBOARD

> @@ -239,7 +255,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, so we can
> +    ;; return the newer.
>      (or clip-text primary-text)
>      ))

Same as previous patch

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

[Ignacio Casso]

> I also think that using symbol properties for
> gui--last-selected-text-clipboard/primary would be better, since a)
> using text properties implies some checks that the variable values are
> actually strings, and b) those text properties could be modified
> elsewhere, since we don't know with which other elisp code those string
> are shared. In particular, it could be the case that
> gui--last-selected-text-clipboard and gui--last-selected-text-primary
> shared the string (although if that was a problem it could easily be
> solved by using different timestamp property names for clipboard and
> primary). I've never worked with either text or symbol properties
> before, so I'm not sure. What do you think?

Another argument in favor of using symbol properties: the comparison to
not duplicate elements in the kill ring when the variable
kill-do-not-save-duplicates is non-nil is made with
equal-including-properties, according to the docstring of the variable.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> I attach the new patch anyway and comment each section below:

LGTM, if Eli's concerns of affecting platforms other than X have also
been resolved.

I think that this change will require you to complete some copyright
paperwork.  Could someone tell me if that is indeed the case?

Thanks.

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

[Ignacio Casso]

Po Lu <luangruo@yahoo.com> writes:

> LGTM, if Eli's concerns of affecting platforms other than X have also
> been resolved.

It should if you can guarantee that (gui-get-selection 'CLIPBOARD
'TIMESTAMP) returns either nil or a valid timestamp. I will advice it in
my machine to return nil and check that everything still works as
expected, but otherwise I can not test it. I should write some comment
about that too.

Also, I you want to go ahead with the ideas of using text properties for
storing the timestamp, I think I'm going to store a copy of the string
in gui--last-selected-text-clipboard just in case, and not the string
that is actually returned. I will also define functions to set and check
the gui--last-selected-text-clipboard variable in one line, to pollute
less the original code. This way is also easier to implement a check of
the window-system variable if we finally want to make sure to only do
this in X systems.

> I think that this change will require you to complete some copyright
> paperwork.  Could someone tell me if that is indeed the case?

It says that for accumulated changes of more than 15 lines I need to do
so. I don't know if it is actually needed for this but I'll start the
paperwork anyway since I'll probably needed in the future.

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Eli Zaretskii <eliz@gnu.org>,
>  53894@debbugs.gnu.org
> Date: Tue, 01 Mar 2022 08:51:23 +0100
> 
> 
> Po Lu <luangruo@yahoo.com> writes:
> 
> > LGTM, if Eli's concerns of affecting platforms other than X have also
> > been resolved.
> 
> It should if you can guarantee that (gui-get-selection 'CLIPBOARD
> 'TIMESTAMP) returns either nil or a valid timestamp. I will advice it in
> my machine to return nil and check that everything still works as
> expected, but otherwise I can not test it. I should write some comment
> about that too.

Not just comment: we should document that every implementation of
gui-get-selection should return either a valid timestamp (and say what
is a valid timestamp) or nil, meaning 'CLIPBOARD 'TIMESTAMP is
unsupported.

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

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> I think that this change will require you to complete some copyright
> paperwork.  Could someone tell me if that is indeed the case?

The patch is large, but it's mostly changes to the comments.  The actual
code change looks like it's less than 15 lines of significance, so it
shouldn't require a copyright assignment.

But starting that process anyway would be nice, of course, and Ignacio
is doing that, apparently?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Ignacio Casso]

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]


> Not just comment: we should document that every implementation of
> gui-get-selection should return either a valid timestamp (and say what
> is a valid timestamp) or nil, meaning 'CLIPBOARD 'TIMESTAMP is
> unsupported.

You are right that it is a dangerous assumption, and it's also hard to
test and probably unnecessary. So I have changed the code to check the
window-system variable instead and only use timestamps for X and haiku
(I don't actually know the list of systems that support it, I have
included haiku because it is also in the list of systems that support
reliably gui-backend-selection-owner-p, according to the code).

I also have defined the functions
gui--set-last-clipboard/primary-selection and
gui--clipboard/primary-selection-unchanged-p to just change the lines
(setq gui--last-selected-text-clipboard/primary text) and (equal text
gui--last-selected-text-clipboard/primary) with them. They behave
exactly the same for systems other than X and haiku, and are backwards
compatible with setting or reading directly the original
gui--last-selected-text-clipboard/primary variable in other places.

I have assumed this last thing (being backwards compatible) was the
reason why Po Lu suggested to use text properties instead of changing
the gui--last-selected-text-clipboard/primary variable, so I have taken
the liberty of moving away from that idea and using the new variables
gui--last-selection-timestamp-clipboard and
gui--last-selection-timestamp-primary instead, which simplify the code.

So here is the final, hopefully definitive patch, attached to the email
and commented below:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for bug#53894 --]
[-- Type: text/x-diff, Size: 9628 bytes --]

From fb91fa4ac94fd57136febec4427e28803afd8c58 Mon Sep 17 00:00:00 2001
From: Ignacio <ignacio.decasso@imdea.org>
Date: Tue, 1 Mar 2022 11:09:15 +0100
Subject: [PATCH] same fix, but with functions

---
 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)))
 
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,
@@ -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.
 
@@ -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.
@@ -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
@@ -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
@@ -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.
              (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)))
@@ -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)
@@ -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
@@ -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)
-- 
2.25.1


[-- Attachment #3: Type: text/plain, Size: 11504 bytes --]



> ---
>  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.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> You are right that it is a dangerous assumption, and it's also hard to
> test and probably unnecessary. So I have changed the code to check the
> window-system variable instead and only use timestamps for X and haiku

Haiku doesn't support timestamps, only `gui-backend-selection-owner-p'
is reliable.  But I can expose the selection counter if you think that's
a good idea.

Thanks.

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

[Ignacio Casso]

>> You are right that it is a dangerous assumption, and it's also hard to
>> test and probably unnecessary. So I have changed the code to check the
>> window-system variable instead and only use timestamps for X and haiku
>
> Haiku doesn't support timestamps, only `gui-backend-selection-owner-p'
> is reliable.  But I can expose the selection counter if you think that's
> a good idea.
>
> Thanks.

I would keep it simple for now and restrict this patch to things that
work out of the box right now. If you like the rest of the patch I can
remove haiku from the list and add any other window system that you tell
me.

Adapting the new code to work also with counters is straightforward
however. So if you think it's worth it to expose counters just to fix this
bug in haiku and other window systems, we can do it.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> I would keep it simple for now and restrict this patch to things that
> work out of the box right now. If you like the rest of the patch I can
> remove haiku from the list and add any other window system that you tell
> me.

Thanks, you can just remove Haiku from the list.  Otherwise, I'm happy
now.

> Adapting the new code to work also with counters is straightforward
> however. So if you think it's worth it to expose counters just to fix this
> bug in haiku and other window systems, we can do it.

Sure, I can work on that later.

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

[Ignacio Casso]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]


> Thanks, you can just remove Haiku from the list.  Otherwise, I'm happy
> now.

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.

Commit message:

Better check for when clipboard or primary selection have changed

Previously it was done by just comparing new and old selection text, now
we use also selection timestamps for systems that support it (only
enabled in X for now). This fixes bug#53894.

lisp/select.el: new variables gui--last-selection-timestamp-clipboard
and gui--last-selection-timestamp-primary. New functions
gui--set-last-clipboard-selection, gui--set-last-primary-selection,
gui--clipboard-selection-unchanged-p and
gui--primary-selection-unchanged-p.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for bug#53894 --]
[-- Type: text/x-diff, Size: 9724 bytes --]

From d64f7b9e734f48666347ae8325b37333ae577a39 Mon Sep 17 00:00:00 2001
From: Ignacio <ignacio.decasso@imdea.org>
Date: Tue, 1 Mar 2022 11:09:15 +0100
Subject: [PATCH] same fix, but with functions

---
 lisp/menu-bar.el    |  3 +-
 lisp/select.el      | 92 ++++++++++++++++++++++++++++++++++-----------
 lisp/term/pc-win.el |  8 ++++
 3 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index ab64928fe7..d8c8c760f7 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -606,7 +606,8 @@ 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'
+        ;; (i.e., that gui--clipboard-selection-unchanged-p returns nil).
         (gui--last-selected-text-clipboard nil))
     (yank)))
 
diff --git a/lisp/select.el b/lisp/select.el
index 42b50c44e6..631f4748bb 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,
@@ -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.
 
@@ -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))
+    (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)))))
+
+
 (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.
@@ -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
@@ -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 gui--clipboard-selection-unchanged-p?
            (gui-backend-selection-owner-p type))
     (let ((request-type (if (memq window-system '(x pgtk haiku))
                             (or x-select-request-type
@@ -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.
              (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)))
@@ -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)
@@ -239,7 +285,9 @@ 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 or selection
+    ;; counter now (for some 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..0aa99a081a 100644
--- a/lisp/term/pc-win.el
+++ b/lisp/term/pc-win.el
@@ -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 should not result in a bug for 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)
-- 
2.25.1

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> 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, I look forward to it.

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

[Ignacio Casso]

No news yet. I have not even got an answer. Do you know how long it
usually takes? I'll send another email just in case the previous was
lost.

Po Lu <luangruo@yahoo.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> 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, I look forward to it.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> No news yet. I have not even got an answer. Do you know how long it
> usually takes? I'll send another email just in case the previous was
> lost.

I'm not sure.  You might want to ask Eli or Lars, they likely know more
about what to do in this situation.

As always, thanks for working on Emacs.

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

[Ignacio Casso]

>>> 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, I look forward to it.
>
> No news yet. I have not even got an answer. Do you know how long it
> usually takes? I'll send another email just in case the previous was
> lost.

They sent me the form no long after I sent this email, after Eli or Lars
intervened and asked himself. Not sure who of the two was since I seem
to have deleted those emails, but thank you for that.

However I've had no news again since I sent them the signed form. They
were supposed to sign it after me and send me a copy, but I haven't
heard from them since. This was two weeks ago. After one week I sent
another email asking how long should I expect the process to take, and
now one week after they haven't replied yet that email either.

I'd keep insisting but I read the following in
https://orgmode.org/worg/org-maintenance.html:

> The assignment process does not always go smoothly, and it has happened
> several times that it gets stuck or forgotten at the FSF. The contact at
> the FSF for this is: copyright-clerk AT fsf DOT org
>
> Emails from the paper submitter have been ignored in the past, but an
> email from the maintainers of Org mode has usually fixed such cases
> within a few days.

So could one of you send them another email, please?

Thanks,

--Ignacio

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, 53894@debbugs.gnu.org
> Date: Thu, 24 Mar 2022 20:59:50 +0100
> 
> 
> >>> 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, I look forward to it.
> >
> > No news yet. I have not even got an answer. Do you know how long it
> > usually takes? I'll send another email just in case the previous was
> > lost.
> 
> They sent me the form no long after I sent this email, after Eli or Lars
> intervened and asked himself. Not sure who of the two was since I seem
> to have deleted those emails, but thank you for that.
> 
> However I've had no news again since I sent them the signed form. They
> were supposed to sign it after me and send me a copy, but I haven't
> heard from them since. This was two weeks ago. After one week I sent
> another email asking how long should I expect the process to take, and
> now one week after they haven't replied yet that email either.
> 
> I'd keep insisting but I read the following in
> https://orgmode.org/worg/org-maintenance.html:
> 
> > The assignment process does not always go smoothly, and it has happened
> > several times that it gets stuck or forgotten at the FSF. The contact at
> > the FSF for this is: copyright-clerk AT fsf DOT org
> >
> > Emails from the paper submitter have been ignored in the past, but an
> > email from the maintainers of Org mode has usually fixed such cases
> > within a few days.
> 
> So could one of you send them another email, please?

Done.

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

[Ignacio Casso]

>> 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, I look forward to it.

Ok, it seems the paperwork is finally complete, so we can
proceed.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> 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.

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

[Ignacio Casso]

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]


Po Lu <luangruo@yahoo.com> writes:

> Ignacio Casso <ignaciocasso@hotmail.com> 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:
>
>
> These checks should use `gui-backend-get-selection' to find the
> selection timestamp, not the public function.

Done.

> Also, the doc strings of the new functions should explain the meaning
> of the argument `text'.

Done.

> and the first line of each doc string should be a complete sentence
> not more than 70 characters wide, terminated by a period.

Done.

> 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...".

Done.


I have also introduced two very minor changes with respect to the last
patch:

- In my comment in pc-win.el: EDIT -> NOTE

- In select.el, inside gui-selection-value: simplified `prog1' forms

Here is the new patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for bug#53894 --]
[-- Type: text/x-diff, Size: 10961 bytes --]

From 9630ca85cf54c5b4d1867948792c65d3f242101c Mon Sep 17 00:00:00 2001
From: Ignacio <ignacio.decasso@imdea.org>
Date: Sun, 13 Mar 2022 21:05:18 +0100
Subject: [PATCH] Better check for when clipboard or primary selection have
 changed

Previously it was done by just comparing new and old selection text, now
we use also selection timestamps for systems that support it (only
enabled in X for now). This fixes bug#53894.

lisp/select.el: new variables gui--last-selection-timestamp-clipboard
and gui--last-selection-timestamp-primary. New functions
gui--set-last-clipboard-selection, gui--set-last-primary-selection,
gui--clipboard-selection-unchanged-p and
gui--primary-selection-unchanged-p.
---
 lisp/menu-bar.el    |   3 +-
 lisp/select.el      | 108 +++++++++++++++++++++++++++++++-------------
 lisp/term/pc-win.el |   8 ++++
 3 files changed, 87 insertions(+), 32 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index ab64928fe7..d8c8c760f7 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -606,7 +606,8 @@ 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'
+        ;; (i.e., that gui--clipboard-selection-unchanged-p returns nil).
         (gui--last-selected-text-clipboard nil))
     (yank)))
 
diff --git a/lisp/select.el b/lisp/select.el
index c352a48261..9db0a7cf84 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,
@@ -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.
 
@@ -119,22 +121,68 @@ 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.
+Save the selected text, passed as argument, and for window
+systems that support it, save the selection timestamp too."
+  (setq gui--last-selected-text-clipboard text)
+  (when (memq window-system '(x))
+    (setq gui--last-selection-timestamp-clipboard
+          (gui-backend-get-selection 'CLIPBOARD 'TIMESTAMP))))
+
+(defun gui--set-last-primary-selection (text)
+  "Save last primary selection.
+Save the selected text, passed as argument, and for window
+systems that support it, save the selection timestamp too."
+  (setq gui--last-selected-text-primary text)
+  (when (memq window-system '(x))
+    (setq gui--last-selection-timestamp-primary
+          (gui-backend-get-selection 'PRIMARY 'TIMESTAMP))))
+
+(defun gui--clipboard-selection-unchanged-p (text)
+  "Check whether the clipboard selection has changed.
+Compare the selection text, passed as argument, with the text
+from the last saved selection. For window systems that support
+it, compare the selection timestamp too."
+  (and
+   (equal text gui--last-selected-text-clipboard)
+   (or (not (memq window-system '(x)))
+       (eq gui--last-selection-timestamp-clipboard
+           (gui-backend-get-selection 'CLIPBOARD 'TIMESTAMP)))))
+
+(defun gui--primary-selection-unchanged-p (text)
+  "Check whether the primary selection has changed.
+Compare the selection text, passed as argument, with the text
+from the last saved selection. For window systems that support
+it, compare the selection timestamp too."
+  (and
+   (equal text gui--last-selected-text-primary)
+   (or (not (memq window-system '(x)))
+       (eq gui--last-selection-timestamp-primary
+           (gui-backend-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.
+If `select-enable-clipboard' is non-nil, copy TEXT to the system's clipboard.
 If `select-enable-primary' is non-nil, put TEXT in the primary selection.
 
 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
@@ -175,6 +223,7 @@ gui--selection-value-internal
            ;; some other window systems.
            (memq window-system '(x haiku))
            (eq type 'CLIPBOARD)
+           ;; Should we unify this with gui--clipboard-selection-unchanged-p?
            (gui-backend-selection-owner-p type))
     (let ((request-type (if (memq window-system '(x pgtk haiku))
                             (or x-select-request-type
@@ -197,19 +246,17 @@ 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.
-             (prog1
-                 (unless (equal text gui--last-selected-text-clipboard)
-                   text)
-               (setq gui--last-selected-text-clipboard text)))))
+             ;; 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.
+             (unless (gui--clipboard-selection-unchanged-p text)
+               (gui--set-last-clipboard-selection text)
+               text))))
         (primary-text
          (when select-enable-primary
            (let ((text (gui--selection-value-internal 'PRIMARY)))
@@ -217,10 +264,9 @@ gui-selection-value
              ;; Check the PRIMARY selection for 'newness', is it different
              ;; from what we remembered them to be last time we did a
              ;; cut/paste operation.
-             (prog1
-                 (unless (equal text gui--last-selected-text-primary)
-                   text)
-               (setq gui--last-selected-text-primary text))))))
+             (unless (gui--primary-selection-unchanged-p text)
+               (gui--set-last-primary-selection text)
+               text)))))
 
     ;; As we have done one selection, clear this now.
     (setq next-selection-coding-system nil)
@@ -235,11 +281,11 @@ gui-selection-value
     ;; something like the following has happened since the last time
     ;; we looked at the selections: Application X set all the
     ;; selections, then Application Y set only one of them.
-    ;; In this case since we don't have
-    ;; 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.
+    ;; In this case, for systems that support selection timestamps, we
+    ;; could return the newer.  For systems that don't, 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.
     (or clip-text primary-text)
     ))
 
diff --git a/lisp/term/pc-win.el b/lisp/term/pc-win.el
index 327d51f275..514267a52d 100644
--- a/lisp/term/pc-win.el
+++ b/lisp/term/pc-win.el
@@ -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.
+        ;; NOTE: 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 should not result in a bug for 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)
-- 
2.25.1

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 53894@debbugs.gnu.org, larsi@gnus.org
> Date: Fri, 01 Apr 2022 12:15:58 +0200
> 
> +(defun gui--set-last-clipboard-selection (text)
> +  "Save last clipboard selection.
> +Save the selected text, passed as argument, and for window
> +systems that support it, save the selection timestamp too."
> +  (setq gui--last-selected-text-clipboard text)
> +  (when (memq window-system '(x))

This should use 'eq', nor 'memq', since you are testing against a
single value.  (There are more of such code elsewhere in the patch.)

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

[Ignacio Casso]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ignacio Casso <ignaciocasso@hotmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, 53894@debbugs.gnu.org, larsi@gnus.org
>> Date: Fri, 01 Apr 2022 12:15:58 +0200
>> 
>> +(defun gui--set-last-clipboard-selection (text)
>> +  "Save last clipboard selection.
>> +Save the selected text, passed as argument, and for window
>> +systems that support it, save the selection timestamp too."
>> +  (setq gui--last-selected-text-clipboard text)
>> +  (when (memq window-system '(x))
>
> This should use 'eq', nor 'memq', since you are testing against a
> single value.  (There are more of such code elsewhere in the patch.)

Well the idea is that in the future there could be more window systems
in that list, that's why I used `memq'. I attach the patch using `eq'
instead.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for bug#53894 --]
[-- Type: text/x-diff, Size: 10945 bytes --]

From b069ee6a9c6454bf30a26730635702cb24dbd950 Mon Sep 17 00:00:00 2001
From: Ignacio <ignacio.decasso@imdea.org>
Date: Sun, 13 Mar 2022 21:05:18 +0100
Subject: [PATCH] Better check for when clipboard or primary selection have
 changed

Previously it was done by just comparing new and old selection text, now
we use also selection timestamps for systems that support it (only
enabled in X for now). This fixes bug#53894.

lisp/select.el: new variables gui--last-selection-timestamp-clipboard
and gui--last-selection-timestamp-primary. New functions
gui--set-last-clipboard-selection, gui--set-last-primary-selection,
gui--clipboard-selection-unchanged-p and
gui--primary-selection-unchanged-p.
---
 lisp/menu-bar.el    |   3 +-
 lisp/select.el      | 108 +++++++++++++++++++++++++++++++-------------
 lisp/term/pc-win.el |   8 ++++
 3 files changed, 87 insertions(+), 32 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index ab64928fe7..d8c8c760f7 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -606,7 +606,8 @@ 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'
+        ;; (i.e., that gui--clipboard-selection-unchanged-p returns nil).
         (gui--last-selected-text-clipboard nil))
     (yank)))
 
diff --git a/lisp/select.el b/lisp/select.el
index c352a48261..0b51f01cc5 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,
@@ -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.
 
@@ -119,22 +121,68 @@ 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.
+Save the selected text, passed as argument, and for window
+systems that support it, save the selection timestamp too."
+  (setq gui--last-selected-text-clipboard text)
+  (when (eq window-system 'x)
+    (setq gui--last-selection-timestamp-clipboard
+          (gui-backend-get-selection 'CLIPBOARD 'TIMESTAMP))))
+
+(defun gui--set-last-primary-selection (text)
+  "Save last primary selection.
+Save the selected text, passed as argument, and for window
+systems that support it, save the selection timestamp too."
+  (setq gui--last-selected-text-primary text)
+  (when (eq window-system 'x)
+    (setq gui--last-selection-timestamp-primary
+          (gui-backend-get-selection 'PRIMARY 'TIMESTAMP))))
+
+(defun gui--clipboard-selection-unchanged-p (text)
+  "Check whether the clipboard selection has changed.
+Compare the selection text, passed as argument, with the text
+from the last saved selection. For window systems that support
+it, compare the selection timestamp too."
+  (and
+   (equal text gui--last-selected-text-clipboard)
+   (or (not (eq window-system 'x))
+       (eq gui--last-selection-timestamp-clipboard
+           (gui-backend-get-selection 'CLIPBOARD 'TIMESTAMP)))))
+
+(defun gui--primary-selection-unchanged-p (text)
+  "Check whether the primary selection has changed.
+Compare the selection text, passed as argument, with the text
+from the last saved selection. For window systems that support
+it, compare the selection timestamp too."
+  (and
+   (equal text gui--last-selected-text-primary)
+   (or (not (eq window-system 'x))
+       (eq gui--last-selection-timestamp-primary
+           (gui-backend-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.
+If `select-enable-clipboard' is non-nil, copy TEXT to the system's clipboard.
 If `select-enable-primary' is non-nil, put TEXT in the primary selection.
 
 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
@@ -175,6 +223,7 @@ gui--selection-value-internal
            ;; some other window systems.
            (memq window-system '(x haiku))
            (eq type 'CLIPBOARD)
+           ;; Should we unify this with gui--clipboard-selection-unchanged-p?
            (gui-backend-selection-owner-p type))
     (let ((request-type (if (memq window-system '(x pgtk haiku))
                             (or x-select-request-type
@@ -197,19 +246,17 @@ 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.
-             (prog1
-                 (unless (equal text gui--last-selected-text-clipboard)
-                   text)
-               (setq gui--last-selected-text-clipboard text)))))
+             ;; 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.
+             (unless (gui--clipboard-selection-unchanged-p text)
+               (gui--set-last-clipboard-selection text)
+               text))))
         (primary-text
          (when select-enable-primary
            (let ((text (gui--selection-value-internal 'PRIMARY)))
@@ -217,10 +264,9 @@ gui-selection-value
              ;; Check the PRIMARY selection for 'newness', is it different
              ;; from what we remembered them to be last time we did a
              ;; cut/paste operation.
-             (prog1
-                 (unless (equal text gui--last-selected-text-primary)
-                   text)
-               (setq gui--last-selected-text-primary text))))))
+             (unless (gui--primary-selection-unchanged-p text)
+               (gui--set-last-primary-selection text)
+               text)))))
 
     ;; As we have done one selection, clear this now.
     (setq next-selection-coding-system nil)
@@ -235,11 +281,11 @@ gui-selection-value
     ;; something like the following has happened since the last time
     ;; we looked at the selections: Application X set all the
     ;; selections, then Application Y set only one of them.
-    ;; In this case since we don't have
-    ;; 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.
+    ;; In this case, for systems that support selection timestamps, we
+    ;; could return the newer.  For systems that don't, 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.
     (or clip-text primary-text)
     ))
 
diff --git a/lisp/term/pc-win.el b/lisp/term/pc-win.el
index 327d51f275..514267a52d 100644
--- a/lisp/term/pc-win.el
+++ b/lisp/term/pc-win.el
@@ -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.
+        ;; NOTE: 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 should not result in a bug for 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)
-- 
2.25.1

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ignacio Casso <ignaciocasso@hotmail.com> writes:

> Well the idea is that in the future there could be more window systems
> in that list, that's why I used `memq'. I attach the patch using `eq'
> instead.

Thanks.  If Eli has no further comments I'll install this now.

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

[Eli Zaretskii]

> From: Ignacio Casso <ignaciocasso@hotmail.com>
> Cc: luangruo@yahoo.com, 53894@debbugs.gnu.org, larsi@gnus.org
> Date: Fri, 01 Apr 2022 13:23:27 +0200
> 
> >> +(defun gui--set-last-clipboard-selection (text)
> >> +  "Save last clipboard selection.
> >> +Save the selected text, passed as argument, and for window
> >> +systems that support it, save the selection timestamp too."
> >> +  (setq gui--last-selected-text-clipboard text)
> >> +  (when (memq window-system '(x))
> >
> > This should use 'eq', nor 'memq', since you are testing against a
> > single value.  (There are more of such code elsewhere in the patch.)
> 
> Well the idea is that in the future there could be more window systems
> in that list, that's why I used `memq'.

Sure, but converting 'eq' comparing to a single value to 'memq'
comparing to a list of values is trivial, and thus doesn't need any
"provisions for".

> I attach the patch using `eq' instead.

Thanks.

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

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  53894@debbugs.gnu.org,  larsi@gnus.org
> Date: Fri, 01 Apr 2022 20:04:03 +0800
> 
> Ignacio Casso <ignaciocasso@hotmail.com> writes:
> 
> > Well the idea is that in the future there could be more window systems
> > in that list, that's why I used `memq'. I attach the patch using `eq'
> > instead.
> 
> Thanks.  If Eli has no further comments I'll install this now.

No further comments.

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> No further comments.

Great, I installed the patch with minor formatting adjustments to the
commit message, and am closing this bug.