From: Visuwesh <visuweshm@gmail.com>
To: Max Nikulin <manikulin@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [BUG] [PATCH] Add yank-media and DND handler [9.6.7 (9.6.7-g6eb773 @ /home/viz/lib/emacs/straight/build/org/)]
Date: Mon, 25 Sep 2023 19:45:33 +0530 [thread overview]
Message-ID: <87y1gul4oq.fsf@gmail.com> (raw)
In-Reply-To: <uepiqs$pgl$2@ciao.gmane.io> (Max Nikulin's message of "Sun, 24 Sep 2023 21:58:37 +0700")
[ Please keep me in the CC as I don't follow the list. ]
[ஞாயிறு செப்டம்பர் 24, 2023] Max Nikulin wrote:
> On 23/09/2023 17:28, Ihor Radchenko wrote:
>> Visuwesh writes:
>>
>>> + (let* ((ext (symbol-name (mailcap-mime-type-to-extension mimetype)))
>>> + (iname (read-string "Insert filename for image: "))
>> It would be nice if we auto-generate the file name here by
>> default. It
>> is what I would expect from yanking an image at least.
>
> Certainly it would be great to provide some default value allowing
> user to override it. However it may be not so trivial. Clipboard
> content may be just copy region from some graphics editor or a
> screenshot tool, so not associated with a file yet. In X11 xprop may
> be used to get clipboard owner application and its title. Wayland may
> be less permissive.
>
> I have tried to copy an image in Firefox. There is a chance to find
> file name (it may be image.php?id=1324 though) in text/html clipboard
> content, but it requires parsing of HTML
>
> xclip -selection clipboard -o -t text/html
I would rather not go down this rabbit hole since there is no way to
cover all the possible cases.
[ஞாயிறு செப்டம்பர் 24, 2023] Max Nikulin wrote:
> On 22/09/2023 21:52, Visuwesh wrote:
>> Attached patch adds yank-media and DND handler to attach files in the
>> clipboard and dropped onto an Emacs frame respectively.
>
> Please, use `make-temp-file' to create files in the temporary
> directory that may be world writable. Predictable file names there are
> undesired from security point of view.
What harm does it cause?
> Other notes are no more than opinion. I may easily miss something and
> verbose reply may be wasting of time. Do not hesitate to ask more
> details if you do not agree.
>
> At first, I expected more common with the following project
> https://github.com/abo-abo/org-download/
> however even the approach to fetch images from clipboard is different.
I have never used that package, this is what I wrote in a discussion
with Ihor in the Matrix room which we iteratively improved. It was
mostly written with my workflow in mind. Without knowledge of how
others work, I cannot improve the implementation. I also don't
understand what the linked package from the Commentary section (which is
why I never used it).
>> +++ b/lisp/org.el
>
> I am in doubts if the following code is more suitable for org.el or
> for org-attach.el
I have the same doubts.
>> @@ -20254,6 +20257,146 @@ it has a `diary' type."
>> (org-format-timestamp timestamp fmt t))
>> (org-format-timestamp timestamp fmt (eq boundary 'end)))))))
>> +;;; Yank media handler and DND
>> +(defun org-setup-yank-dnd-handlers ()
>> + "Setup the `yank-media' and DND handlers for buffer."
>> + (setq-local dnd-protocol-alist
>> + (cons '("^file:///" . org--dnd-local-file-handler)
>> + dnd-protocol-alist))
>> + (yank-media-handler "image/.*" #'org--image-yank-media-handler)
>> + ;; Looks like different DEs go for different handler names,
>> + ;; https://larsee.com/blog/2019/05/clipboard-files/.
>> + (yank-media-handler "x/special-\\(?:gnome\|KDE\|mate\\)-files"
>> + #'org--copied-files-yank-media-handler))
>> +
>> +(defcustom org-media-image-save-type 'attach
>
> Could it be extended to any mime type? If so I would avoid "image" in
> its name. `org-yank-media-save-dir'?
It should be easy enough to do it but do we want to go there?
>> + "Method to save images yanked from clipboard and dropped to Emacs.
>> +It can be the symbol `attach' to add it as an attachment, or a
>> +directory name to copy/cut the image to that directory."
>> + :group 'org
>> + :package-version '(Org . "9.7")
>> + :type '(choice (const :tag "Add it as attachment" attach)
>> + (directory :tag "Save it in directory"))
>> + :safe (lambda (x) (or (stringp x) (eq x 'attach))))
>
> Unsure if every directory may be considered as safe (e.g. ~/.ssh/)
Is there a reliable way to know which directory is "safe"? If not, then
let the users shoot themselves in the foot.
>> +
>> +(declare-function org-attach-attach "org-attach" (file &optional visit-dir method))
>> +
>> +(defun org--image-yank-media-handler (mimetype data)
>> + "Save image DATA of mime-type MIMETYPE and insert link at point.
>> +It is saved as per `org-media-image-save-type'. The name for the
>> +image is prompted and the extension is automatically added to the
>> +end."
>> + (let* ((ext (symbol-name (mailcap-mime-type-to-extension mimetype)))
>> + (iname (read-string "Insert filename for image: "))
>
> Unless 'attach is used, `read-file-name' would allow saving to
> arbitrary directory with path completion.
Sorry, I don't understand what you mean here. I am not really familiar
with attachment facilities of org-mode.
>> + (filename (file-name-with-extension iname ext))
>> + (absname (expand-file-name
>> + filename
>> + (if (eq org-media-image-save-type 'attach)
>> + temporary-file-directory
>
> `make-temp-file' should be used instead of `temporary-file-directory',
> however it requires changes in code around.
>
>> + org-media-image-save-type)))
>> + link)
>> + (when (and (not (eq org-media-image-save-type 'attach))
>> + (not (file-directory-p org-media-image-save-type)))
>> + (make-directory org-media-image-save-type t))
>> + (with-temp-file absname
>> + (insert data))
>> + (if (null (eq org-media-image-save-type 'attach))
>> + (setq link (org-link-make-string
>> + (concat "file:" (file-relative-name absname))
>> + filename))
>
> Is there a chance to request for link description? Unsure if
> `org-insert-link' may be easily reused.
This would include too many prompts and make the whole process annoying
again.
>> [...]
>> +(defcustom org-dnd-default-attach-method nil
>> + "Default attach method to use when DND action is unspecified.
>> +This attach method is used when the DND action is `private'.
>> +This is also used when `org-media-image-save-type' is nil.
>> +When nil, use `org-attach-method'."
>> + :group 'org
>> + :package-version '(Org . "9.7")
>> + :type '(choice (const :tag "Default attach method" nil)
>> + (const :tag "Copy" cp)
>> + (const :tag "Move" mv)
>> + (const :tag "Hard link" ln)
>> + (const :tag "Symbol link" lns)))
>
> Labels in the menu below are a bit different. "Symbolic"?
Okay, will make them both follow what org-attach uses.
>> +
>> +(declare-function mailcap-file-name-to-mime-type "mailcap" (file-name))
>> +(defvar org-attach-method)
>> +
>> +(defun org--dnd-local-file-handler (url action)
>> + "Attach filename given by URL using method pertaining to ACTION.
>> +If ACTION is `move', use `mv' attach method.
>> +If `copy', use `cp' attach method.
>> +If `ask', ask the user.
>> +If `private', use the method denoted in `vz/org-dnd-default-attach-action'.
>
> "vz/" likely should be dropped
Yes, thanks for the catch.
>> +The action `private' is always returned."
>> + (require 'mailcap)
>> + (let* ((filename (dnd-get-local-file-name url))
>> + (mimetype (mailcap-file-name-to-mime-type filename))
>> + (separatep (and (string-prefix-p "image/" mimetype)
>> + (not (eq 'attach org-media-image-save-type))))
>> + (method (pcase action
>> + ('copy 'cp)
>> + ('move 'mv)
>> + ('ask (caddr (read-multiple-choice
>
> `org-mks' has some issues, but it is the current way to present a kind
> of menu in Org.
Okay.
>> + "Attach using method"
>> + '((?c "cp" cp)
>> + (?m "mv" mv)
>> + (?l "ln hard link" ln)
>
> "ln" in the label perhaps should be dropped.
I added ln to make the first letter bolded in rmc prompt, but with
org-mks this shouldn't be necessary anymore.
> Should it be hidden if stat reports different file systems for source
> and target or fallback to copy is implicitly used?
It should be handled appropriately by Emacs already. See (info "(emacs)
Copying and Naming"). I also don't find org-attach doing anything
special.
>> + (?s "symbolic link" lns)))))
>> + ('private (or org-dnd-default-attach-method
>> + org-attach-method)))))
>> + (if separatep
>> + (funcall
>> + (pcase method
>> + ('cp #'copy-file)
>> + ('mv #'rename-file)
>> + ('ln #'add-name-to-file)
>> + ('lns #'make-symbolic-link))
>> + filename
>> + (expand-file-name (file-name-nondirectory filename)
>> + org-media-image-save-type))
>> + (org-attach-attach filename nil method))
>> + (insert
>> + (org-link-make-string
>> + (concat (if separatep
>
> I would consider swapping of `concat' and `if' for the sake of readability.
That can be done.
>> + "file:"
>> + "attachment:")
>> + (if separatep
>> + (expand-file-name (file-name-nondirectory filename)
>> + org-media-image-save-type)
>> + (file-name-nondirectory filename)))
>> + (file-name-nondirectory filename))
>> + "\n")
>> + 'private))
>> +
>> ;;; Other stuff
>> (defvar reftex-docstruct-symbol)
>> --
>
> I do not know if it can be easily implemented, but it may be useful to
> control attachment/generic directory at the moment when a particular
> image is yanked/dropped instead of purely relying on predefined user
> option.
Sorry, I don't understand what you mean here. Note that my usage of
attachments is sparse. "Particular image" is hard to determine since
the only reliable data at hand is its mimetype.
next prev parent reply other threads:[~2023-09-25 14:16 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <87bkdccihf.fsf.ref@yahoo.com>
2023-09-22 14:52 ` [BUG] [PATCH] Add yank-media and DND handler [9.6.7 (9.6.7-g6eb773 @ /home/viz/lib/emacs/straight/build/org/)] Visuwesh
2023-09-22 16:51 ` Max Nikulin
2023-09-22 17:29 ` Visuwesh
2023-09-24 8:06 ` Max Nikulin
2023-09-23 10:28 ` Ihor Radchenko
2023-09-23 16:55 ` Visuwesh
2023-09-25 13:14 ` Visuwesh
2023-09-26 16:25 ` Max Nikulin
2023-09-27 8:33 ` Visuwesh
2023-10-07 11:56 ` Ihor Radchenko
2023-10-07 12:07 ` Ihor Radchenko
2023-10-07 12:27 ` Visuwesh
2023-10-07 12:36 ` Ihor Radchenko
2023-10-07 14:03 ` Visuwesh
2023-10-08 9:30 ` Ihor Radchenko
2023-10-08 11:21 ` Visuwesh
2023-10-09 11:12 ` Ihor Radchenko
2023-10-09 12:17 ` Visuwesh
2023-10-19 7:34 ` Visuwesh
2023-10-19 9:44 ` Ihor Radchenko
2023-10-20 1:52 ` Po Lu
2023-10-20 7:29 ` Ihor Radchenko
2023-10-20 7:46 ` Po Lu
2023-10-20 7:57 ` Ihor Radchenko
2023-10-20 8:29 ` Po Lu
2023-10-20 10:17 ` Visuwesh
2023-10-22 6:19 ` Visuwesh
2023-10-23 8:58 ` Ihor Radchenko
2023-10-23 10:12 ` Visuwesh
2023-10-26 11:39 ` Po Lu
2023-11-05 12:02 ` Ihor Radchenko
2023-11-05 17:45 ` Visuwesh
2023-12-05 13:18 ` Visuwesh
2023-12-10 13:53 ` Ihor Radchenko
2023-12-10 14:47 ` Bastien Guerry
2023-12-10 15:07 ` Ihor Radchenko
2023-09-24 14:58 ` Max Nikulin
2023-09-25 14:15 ` Visuwesh [this message]
2023-09-26 10:24 ` Ihor Radchenko
2023-09-27 8:29 ` Visuwesh
2023-09-28 12:01 ` Max Nikulin
2023-09-24 14:49 ` Max Nikulin
2023-10-06 7:34 ` Po Lu
2023-09-29 8:20 Liu Hui
2023-10-01 14:28 ` Visuwesh
2023-10-02 0:28 ` Liu Hui
-- strict thread matches above, loose matches on Subject: below --
2023-10-11 14:24 Liu Hui
2023-10-11 15:36 ` Visuwesh
2023-10-12 5:12 ` Liu Hui
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.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y1gul4oq.fsf@gmail.com \
--to=visuweshm@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=manikulin@gmail.com \
/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/org-mode.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).