Ihor Radchenko <yanta...@posteo.net> writes:
> 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.Fixed all of the corrections. > This progn is redundant. I know, but I put it there intentionally, because seeing an `if' with more than three subforms (condtition, left, right), seems to me extremely ugly. -- Your sincerely, Vladimir Nikishkin (MiEr, lockywolf) (Laptop)
signature.asc
Description: PGP signature
