Lockywolf <for_emacs-orgmode-gnu....@lockywolf.net> writes:

> At the moment, `org-yank-image-save-method' can only save an image
> into a single location, or query "org-attach".
>
> This change allows customising its behaviour, allowing
> `org-yank-image-save-method' to be a function returning a place to
> save the file.
>
> Patch attached.

Thanks!
This addition makes sense.

>    #+attr_texinfo: :columns 0.3 0.7
> -  | {{{kbd(TAB)}}}            | Cycle visibility.               |
> +  | {{{kbd(TAB)}}}                  | Cycle visibility.               |
>    | {{{kbd(DOWN)}}} / {{{kbd(UP)}}} | Next/previous visible headline. |
> -  | {{{kbd(RET)}}}            | Select this location.           |
> -  | {{{kbd(/)}}}              | Do a Sparse-tree search         |
> +  | {{{kbd(RET)}}}                  | Select this location.           |
> +  | {{{kbd(/)}}}                    | Do a Sparse-tree search         |

Please amend the patch to remove irrelevant whitespace changes.
  
> +*** org-yank-image-save-method: allow to be a procedure producing file name

There are no procedures in elisp
*function

> +In previous versions ~org-yank-image-save-method~ could be either
> +a symbol ~attach~ or a string, directory name.  Now it can also be
> +a procedure, which will be called and its return value will be used

*function

> @@ -20485,7 +20485,8 @@ directory name to copy/cut the image to that 
> directory."
>    :group 'org

The docstring still only lists 'attach and a string as allowed values.

> +         (dirname (cond ((eq org-yank-image-save-method 'attach) 
> temporary-file-directory)
> +                        ((stringp org-yank-image-save-method) 
> org-yank-image-save-method)
> +                        ((functionp org-yank-image-save-method) (funcall 
> org-yank-image-save-method))
> +                        (t (error "%s must be a string or a callable" 
> org-yank-image-save-method))))

1. Use `user-error', not `error'.
2. "must be a string or a callable" is not accurate.  Easier just use
   generic "unrecognized value"
3. (optional) What if the user function returns non-string?

> +    (insert (if (null (eq org-yank-image-save-method 'attach))

null is slightly awkward for me. I'd personally use not. But either way
is fine.

> +        (org-link-make-string (concat "file:" (org-link--normalize-filename 
> absname)))
> +        (progn

This progn is redundant.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Reply via email to