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

[Visuwesh]

[வியாழன் டிசம்பர் 26, 2024] Eli Zaretskii wrote:

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

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.

As for (add-function (local 'yank-media-autoselect-function) ...)
scenario, taking Robert's example of preferring image/svg in some HTML
documents, one could say

    (add-function :around (local 'yank-media-autoselect-function)
      (lambda (oldfun types)
        (if (memq 'image/svg types) 
            'image/svg
          (funcall oldfun types))))

>> +(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:

    ./lisp/gnus/message.el:3180:  (yank-media-handler "image/.*" 
#'message--yank-media-image-handler)
    ./lisp/org/org.el:20757:    (yank-media-handler "image/.*" 
#'org--image-yank-media-handler)
    ./lisp/org/org.el:20760:    (yank-media-handler 
"x/special-\\(?:gnome\\|KDE\\|mate\\)-files"
    ./lisp/textmodes/sgml-mode.el:2419:  (yank-media-handler 'text/html 
#'html-mode--html-yank-handler)
    ./lisp/textmodes/sgml-mode.el:2420:  (yank-media-handler "image/.*" 
#'html-mode--image-yank-handler)

and org-mode's main branch recently gained support for
application/x-libreoffice-tsvc.

Personally, these are the only media types which I use/come across
daily.  Someone™ needs to comment on other media types that are
potentially useful.
[ I have a patch for message.el to add support for x/special-gnome-files
  which I need to bring in sync with master and send soon™.  ]

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

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

Thanks, I was stuck on the phrasing here.

>> +    (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.
      2. a-s-f returns multiple media types: we ask the user which one
         to yank.
      3. a-s-f returns nil.  We show a message and do what?

         If (length all-types) = 1, should we insert it no questions asked?
         If (length all-types) > 1, we ask as usual.
         
         Or, should we proceed as if the user did not ask for
         autoselection?

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

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

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

OK, yank-media is already documented in the Emacs user manual.  We would
need to talk about C-u and provide a description of auto-selection.