bug#75116: [PATCH] Make 'yank-media' autoselect the best media type

[Eli Zaretskii <eliz@gnu.org>]

> Cc: Ihor Radchenko <yantar92@posteo.net>, pinmacs@cas.cat, rpluim@gmail.com, Eli Zaretskii <eliz@gnu.org>
> From: Visuwesh <visuweshm@gmail.com>
> Date: Thu, 26 Dec 2024 17:57:50 +0530
> 
> This is a continuation of the long thread in emacs-devel:
> https://yhetil.org/emacs-devel/79fc91f3-c2c3-44db-9817-595808917f26@cas.cat/
> 
> This message provides a summary:
> https://yhetil.org/87r06cj2nd.fsf@gmail.com
> 
> Ihor wrote:
> 
> > The only comment is that leaving an option to return a list of types
> > rather than only a single type will make things more flexible.
> 
> And this is now done in the attached patch.

Thanks.

> Before I go about writing NEWS and updating the manual, what do you
> think about the attached instead?

Some comments below.

> I think the variable
> yank-media-preferred-types gives a more granular control for major-mode
> authors than (add-function (local 'yank-media-autoselect-function) ...)

Maybe.  But one of my comments is exactly about that: I don't quite
understand your intent for how major modes should use this variable
(since neither the doc string nor the comments spell that out).  Would
you please explain your thoughts on that?

> +(defvar yank-media-preferred-types
> +  `(;; Check first since LibreOffice also puts a PNG image in the
> +    ;; clipboard when a table cell is copied.
> +    application/x-libreoffice-tsvc
> +    ;; Give PNG more priority.
> +    image/png
> +    image/jpeg
> +    ;; These are files copied/cut to the clipboard from a file manager.
> +    ,(lambda (mimetypes)
> +       (seq-find (lambda (type)
> +                (string-match-p "x-special/\\(gnome\\|KDE\\|mate\\)-files"
> +                                (symbol-name type)))
> +              mimetypes))
> +    ;; FIXME: We should have a way to handle text/rtf.
> +    text/html)

Not sure I understand the value you suggest.  It seems to lack many
important types.  Also, aren't at least some of the types
system-dependent?

> +  "List of mime types in the order of preference.
> +Each element in the list should be a symbol to choose the mime type
> +denoted by it, or a function of one argument, the mime types available,
> +and should return the mime types to use.")

If this is intended for major modes to override, we should say so in
the doc string.

> +(defvar yank-media-autoselect-function #'yank-media-autoselect-function
> +  "Function to auto select the best mime types when many are available.
                                                       ^^^^
I suggest "more than one" there.  "Many" could be misinterpreted to
exclude the case of just two possible types.

> +    (setq pref-type (and (null noselect)
> +                         (funcall yank-media-autoselect-function
> +                                  (mapcar #'car all-types))))
> +    (cond
> +     ;; We have one preferred mime type so use it unconditionally.
> +     ((and pref-type (symbolp pref-type))
> +      (funcall (cdr (assq pref-type all-types)) pref-type
> +               (yank-media--get-selection pref-type)))
> +     ;; The user chose to not autoselect and there's just a single type,
> +     ;; just call the handler.
> +     ((and (null pref-type) (length= all-types 1))
> +      (funcall (cdar all-types) (caar all-types)
> +               (yank-media--get-selection (caar all-types))))

This goes against what the doc string says.  And I think the doc
string describes a better behavior: if the user asked not to
auto-select, we shouldn't, even if there's just one type available.
We should instead ask the user whether to yank that type, because the
user could decide she doesn't want that type, even it it's the only
one.

Also, I think we should show some message if
yank-media-autoselect-function returns nil.  AFAIU, the code you
posted silently does nothing, which IMO is not the best UI.

> I know that I have to update the Info node (info "(elisp) Yanking
> Media").  Does (info "(emacs) Clipboard") need any update too?

IMO, yes.  In fact, I think the user-facing part of the description of
yank-media should be moved to the Emacs user manual, the "Clipboard"
node.