Subject: Re: bug#65892: 30.0.50; yank-media fails for clipboard selection that are wiped after being accessed

From: Visuwesh <[email-masked]>
Date: Mon, 24 Jun 2024 19:16:09 +0530

[திங்கள் ஜூன் 24, 2024] Eli Zaretskii wrote:

>> From: Visuwesh <[email-masked]>
>> Date: Mon, 24 Jun 2024 09:23:20 +0530
>> 
>> I apologise for neglecting to speak out loud before the emacs-30 branch
>> cut.  Is there a chance to get this patch in emacs-30?  I am using this
>> patch ever since I sent it to debbugs and I do not face any issue wrt
>> the pcmanfm issue shown here and other applications which do not wipe
>> the clipboard.  It would be nice to have it in emacs-30 especially with
>> org-mode gaining yank-media support for files copied/cut from a file
>> manager.
>
> I can be convinced to make such changes on the release branch only if
> all they do is make a function whose return value was not interesting
> to return a useful value.  But the patch does more than that: it
> introduces pcase-dolist and replaces some FOO with (car FOO) in at
> least 3 occasions.  

Thanks for the review and consideration, Eli.  The patch only touched
the return value of a helper function.

> Such changes make me uncomfortable, especially given that this is a
> case where Emacs needs to jump through hoops to work around
> misfeatures of some 3rd-party utility.

I suspect similar behaviour will be shown by other file managers.

> Can you rewrite the patch so that it will be clear at a glance that it
> cannot possibly cause any harm because it just causes the same code
> return a value?  If you can, we have a chance of installing this on
> the emacs-30 branch.

I hope the attached patch makes the intent clear.  I added a doc-string
to the internal function too, and removed pcase-dolist and extra car to
make the code simpler.

> And, btw, I wonder whether enough people use pcmanfm for us to bother
> about this issue.

AFAIK, pcmanfm is fairly popular among the window manager users crowd
due to its light-weight.

>From 0aa3efd2e05dcc228e042b43fce6f997fd8afc1c Mon Sep 17 00:00:00 2001
From: Visuwesh <[email-masked]>
Date: Tue, 12 Sep 2023 22:21:58 +0530
Subject: [PATCH] Fix yank-media when clipboard selection is wiped after access

Certain applications wipes the clipboard selection after is accessed
once e.g., pcmanfm with cut files which makes yank-media pass nil to
the chosen yank-media handler eventually breaking it.  (bug#65892)

* lisp/yank-media.el (yank-media--find-matching-media): Make it return
the selection.  Also document the return type.
(yank-media): Pass the returned selection from above to the handler
instead of refetching the selection from the clipboard.
---
 lisp/yank-media.el | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/lisp/yank-media.el b/lisp/yank-media.el
index abc137d9c38..830ff40c667 100644
--- a/lisp/yank-media.el
+++ b/lisp/yank-media.el
@@ -44,26 +44,31 @@ yank-media
   (let ((all-types nil))
     (pcase-dolist (`(,handled-type . ,handler)
                    yank-media--registered-handlers)
-      (dolist (type (yank-media--find-matching-media handled-type))
-        (push (cons type handler) all-types)))
+      (dolist (sel (yank-media--find-matching-media handled-type))
+        (push (list (car sel) handler (cdr sel)) all-types)))
     (unless all-types
       (user-error
        "No handler in the current buffer for anything on the clipboard"))
     ;; We have a handler in the current buffer; if there's just
     ;; matching type, just call the handler.
     (if (length= all-types 1)
-        (funcall (cdar all-types) (caar all-types)
-                 (yank-media--get-selection (caar all-types)))
+        (funcall (nth 1 (car all-types)) (nth 0 (car all-types))
+                 (nth 2 (car all-types)))
       ;; More than one type the user for what type to insert.
-      (let ((type
-             (intern
-              (completing-read "Several types available, choose one: "
-                               (mapcar #'car all-types) nil t))))
-        (funcall (alist-get type all-types)
-                 type (yank-media--get-selection type))))))
+      (let* ((type
+              (intern
+               (completing-read "Several types available, choose one: "
+                                (mapcar #'car all-types) nil t)))
+             (chosen-sel (assq type all-types)))
+        (funcall (nth 1 chosen-sel) (nth 0 chosen-sel)
+                 (nth 2 chosen-sel))))))
 
 (defun yank-media--find-matching-media (handled-type)
-  (seq-filter
+  "Return list of clipboard data with mime-type matched by HANDLED-TYPE.
+Each element in the list is a cons pair (TYPE . SELECTION) where TYPE is
+the mime-type of the clipboard selection, and SELECTION is the clipboard
+selection data as a string."
+  (seq-keep
    (lambda (type)
      (pcase-let ((`(,major ,minor) (split-string (symbol-name type) "/")))
        (if (and (equal major "image")
@@ -73,11 +78,13 @@ yank-media--find-matching-media
            ;; `image/x-win-bitmap'.
            nil
          ;; Check that the handler wants this type.
-         (and (if (symbolp handled-type)
-                  (eq handled-type type)
-                (string-match-p handled-type (symbol-name type)))
-              ;; An element may be in TARGETS but be empty.
-              (yank-media--get-selection type)))))
+         (let ((selection (and (if (symbolp handled-type)
+                                   (eq handled-type type)
+                                 (string-match-p handled-type (symbol-name type)))
+                               ;; An element may be in TARGETS but be empty.
+                               (yank-media--get-selection type))))
+           (and selection
+                (cons type selection))))))
    (gui-get-selection 'CLIPBOARD 'TARGETS)))
 
 (defun yank-media--get-selection (data-type)
-- 
2.43.0