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

[Eli Zaretskii <eliz@gnu.org>]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: 75116@debbugs.gnu.org,  yantar92@posteo.net,  pinmacs@cas.cat,
>   rpluim@gmail.com
> Date: Fri, 27 Dec 2024 14:28:53 +0530
> 
> [வியாழன் டிசம்பர் 26, 2024] Eli Zaretskii wrote:
> >> 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?
> 
> I was thinking using a variable like yank-media-preferred-types would be
> easier to ensure that image/svg is tried _before_ image/png but not
> _before_ application/x-libreoffice-tsvc, etc.  Maybe this is
> overengineering things.  I do not hold a strong opinion here so if you
> think `yank-media-autoselect-function' is enough control for major-mode
> authors, that is enough.

If we intend for major modes to bind this variable, we should say so
in the doc string, IMO.

> >> +(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.  
> 
> These are media types for which support for yank-media already exists:

I thought we should have here as many media types as possible,
regardless of their support, because this variable expresses the
relative priorities of the types.  Major modes which add handlers
should not need to also modify the value of this variable
unnecessarily, no?

> > Also, aren't at least some of the types system-dependent?
> 
> Yes, definitely.  x/special-gnome-files and
> application/x-libreoffice-tsvc are system- and software-dependent resp.
> This was one of my comments addressed in the message I posted to
> emacs-devel:
> 
>      The mimetype used for cut/copied files only works in Linux
>      environments.  If other platforms can present such file:// links in
>      the clipboard and Emacs supports it, we would need to add it to the
>      list too.
> 
> If we want platform-agnostic types, I assume we need an abstraction
> layer on top that would present the clipboard data in a uniform manner.
> I do not have the means to work on this since I only use Linux systems.

I just thought about having the value depend on the platform.

> >> +    (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 want to ensure we are on the same page wrt UI here:
> 
>     User asks to autoselect:
>       1. autoselect-function (a-s-f) returns one media type: we yank it.

Yes.

>       2. a-s-f returns multiple media types: we ask the user which one
>          to yank.

No, we use the first one.

>       3. a-s-f returns nil.  We show a message and do what?

We show a message and do nothing.

>     User asks not to autoselect:
>       4. (length all-types) = 1: We show the media type and ask if she
>          wants to yank it.

Yes.

>       5. (length all-types) > 1: We ask for the media type to yank.

Yes.

> Excepting case (3), does the other cases sound good?

See above regarding my opinions.