* bug#75116: [PATCH] Make 'yank-media' autoselect the best media type
@ 2024-12-26 12:27 Visuwesh
2024-12-26 15:49 ` Eli Zaretskii
0 siblings, 1 reply; 3+ messages in thread
From: Visuwesh @ 2024-12-26 12:27 UTC (permalink / raw)
To: 75116; +Cc: Ihor Radchenko, pinmacs, rpluim, Eli Zaretskii
[-- Attachment #1: Type: text/plain, Size: 691 bytes --]
Tags: patch
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.
Before I go about writing NEWS and updating the manual, what do you
think about the attached instead? 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) ...)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: yank-media-autoselect-other.diff --]
[-- Type: text/x-diff, Size: 4456 bytes --]
diff --git a/lisp/yank-media.el b/lisp/yank-media.el
index 17981c37c0e..010e942680d 100644
--- a/lisp/yank-media.el
+++ b/lisp/yank-media.el
@@ -29,19 +29,57 @@
(defvar yank-media--registered-handlers nil)
+(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)
+ "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.")
+
+(defvar yank-media-autoselect-function #'yank-media-autoselect-function
+ "Function to auto select the best mime types when many are available.
+The function is called with a list of mime types that have handler in
+the current buffer, and should return the type or a list of types to
+use, or nil if no preferred type is found.")
+
+(defun yank-media-autoselect-function (mimetypes)
+ (catch 'preferred
+ (dolist (typ yank-media-preferred-types)
+ (let ((ret (if (functionp typ)
+ (funcall typ mimetypes)
+ (and (memq typ mimetypes) typ))))
+ (when ret (throw 'preferred typ))))))
+
;;;###autoload
-(defun yank-media ()
+(defun yank-media (&optional noselect)
"Yank media (images, HTML and the like) from the clipboard.
This command depends on the current major mode having support for
accepting the media type. The mode has to register itself using
the `yank-media-handler' mechanism.
+Optional argument NOSELECT non-nil (interactively, with a prefix
+argument) means to skip auto-selecting the best mimetype and ask
+for the media type to use when multiple are available.
Also see `yank-media-types' for a command that lets you explore
all the different selection types."
- (interactive)
+ (interactive "P")
(unless yank-media--registered-handlers
(user-error "The `%s' mode hasn't registered any handlers" major-mode))
- (let ((all-types nil))
+ (let ((all-types nil)
+ pref-type)
(pcase-dolist (`(,handled-type . ,handler)
yank-media--registered-handlers)
(dolist (type (yank-media--find-matching-media handled-type))
@@ -49,18 +87,28 @@ yank-media
(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)))
- ;; More than one type the user for what type to insert.
+ (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))))
+ ;; More than one type, ask the user for what type to insert.
+ (t
(let ((type
(intern
(completing-read "Several types available, choose one: "
- (mapcar #'car all-types) nil t))))
+ (or pref-type (mapcar #'car all-types))
+ nil t))))
(funcall (alist-get type all-types)
- type (yank-media--get-selection type))))))
+ type (yank-media--get-selection type)))))))
(defun yank-media--find-matching-media (handled-type)
(seq-filter
[-- Attachment #3: Type: text/plain, Size: 629 bytes --]
I know that I have to update the Info node (info "(elisp) Yanking
Media"). Does (info "(emacs) Clipboard") need any update too?
In GNU Emacs 31.0.50 (build 27, x86_64-pc-linux-gnu, X toolkit, cairo
version 1.18.2, Xaw scroll bars) of 2024-12-07 built on astatine
Repository revision: 9ddec89e422d0dd6e9069731b8f2dd2c90aa5607
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101014
System Description: Debian GNU/Linux trixie/sid
Configured using:
'configure --with-sound=alsa --with-x-toolkit=lucid --without-xaw3d
--without-gconf --without-libsystemd --with-cairo CFLAGS=-g3'
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0001-Make-yank-media-autoselect-the-best-media-type.patch --]
[-- Type: text/patch, Size: 4670 bytes --]
From b12ea40822eb0cacf67de514f273cba6f283e580 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Thu, 26 Dec 2024 17:50:13 +0530
Subject: [PATCH] Make 'yank-media' autoselect the best media type
* lisp/yank-media.el (yank-media-autoselect-function)
(yank-media-autoselect-function): Add new variable and function
to make 'yank-media' choose the best/preferred media type out of
the available ones.
(yank-media): Change to account for above.
---
lisp/yank-media.el | 62 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 51 insertions(+), 11 deletions(-)
diff --git a/lisp/yank-media.el b/lisp/yank-media.el
index 17981c37c0e..86dc05cd4f0 100644
--- a/lisp/yank-media.el
+++ b/lisp/yank-media.el
@@ -29,19 +29,49 @@
(defvar yank-media--registered-handlers nil)
+(defvar yank-media-autoselect-function #'yank-media-autoselect-function
+ "Function to auto select the best mime types when many are available.
+The function is called with a list of mime types that have handler in
+the current buffer, and should return the type or a list of types to
+use, or nil if no preferred type is found.")
+
+(defun yank-media-autoselect-function (mimetypes)
+ (cond
+ ;; Check first since LibreOffice also puts a PNG image in the
+ ;; clipboard when a table cell is copied.
+ ((memq 'application/x-libreoffice-tsvc mimetypes)
+ 'application/x-libreoffice-tsvc)
+ ;; Give PNG more priority.
+ ((memq 'image/png mimetypes)
+ 'image/png)
+ ((memq 'image/jpeg mimetypes)
+ 'image/jpeg)
+ ;; These are files copied/cut to the clipboard from a file manager.
+ ((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.
+ ((memq 'text/html mimetypes)
+ 'text/html)))
+
;;;###autoload
-(defun yank-media ()
+(defun yank-media (&optional noselect)
"Yank media (images, HTML and the like) from the clipboard.
This command depends on the current major mode having support for
accepting the media type. The mode has to register itself using
the `yank-media-handler' mechanism.
+Optional argument NOSELECT non-nil (interactively, with a prefix
+argument) means to skip auto-selecting the best mimetype and ask
+for the media type to use when multiple are available.
Also see `yank-media-types' for a command that lets you explore
all the different selection types."
- (interactive)
+ (interactive "P")
(unless yank-media--registered-handlers
(user-error "The `%s' mode hasn't registered any handlers" major-mode))
- (let ((all-types nil))
+ (let ((all-types nil)
+ pref-type)
(pcase-dolist (`(,handled-type . ,handler)
yank-media--registered-handlers)
(dolist (type (yank-media--find-matching-media handled-type))
@@ -49,18 +79,28 @@ yank-media
(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)))
- ;; More than one type the user for what type to insert.
+ (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))))
+ ;; More than one type, ask the user for what type to insert.
+ (t
(let ((type
(intern
(completing-read "Several types available, choose one: "
- (mapcar #'car all-types) nil t))))
+ (or pref-type (mapcar #'car all-types))
+ nil t))))
(funcall (alist-get type all-types)
- type (yank-media--get-selection type))))))
+ type (yank-media--get-selection type)))))))
(defun yank-media--find-matching-media (handled-type)
(seq-filter
--
2.45.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* bug#75116: [PATCH] Make 'yank-media' autoselect the best media type
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
0 siblings, 1 reply; 3+ messages in thread
From: Eli Zaretskii @ 2024-12-26 15:49 UTC (permalink / raw)
To: Visuwesh; +Cc: yantar92, pinmacs, rpluim, 75116
> 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.
^ permalink raw reply [flat|nested] 3+ messages in thread* bug#75116: [PATCH] Make 'yank-media' autoselect the best media type
2024-12-26 15:49 ` Eli Zaretskii
@ 2024-12-27 8:58 ` Visuwesh
0 siblings, 0 replies; 3+ messages in thread
From: Visuwesh @ 2024-12-27 8:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: yantar92, pinmacs, rpluim, 75116
[வியாழன் டிசம்பர் 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-27 8:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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).