unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Visuwesh <visuweshm@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: yantar92@posteo.net, pinmacs@cas.cat, rpluim@gmail.com,
	75116@debbugs.gnu.org
Subject: bug#75116: [PATCH] Make 'yank-media' autoselect the best media type
Date: Fri, 27 Dec 2024 14:28:53 +0530	[thread overview]
Message-ID: <871pxtcxiq.fsf@gmail.com> (raw)
In-Reply-To: <86r05uxx4i.fsf@gnu.org> (Eli Zaretskii's message of "Thu, 26 Dec 2024 17:49:33 +0200")

[வியாழன் டிசம்பர் 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.





  reply	other threads:[~2024-12-27  8:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-26 12:27 bug#75116: [PATCH] Make 'yank-media' autoselect the best media type Visuwesh
2024-12-26 15:49 ` Eli Zaretskii
2024-12-27  8:58   ` Visuwesh [this message]
2024-12-28 12:24     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871pxtcxiq.fsf@gmail.com \
    --to=visuweshm@gmail.com \
    --cc=75116@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=pinmacs@cas.cat \
    --cc=rpluim@gmail.com \
    --cc=yantar92@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).