Re: [O] Moving and resetting attachments

[Florian Lindner]

Am 02.06.2017 um 11:19 schrieb Eric Abrahamsen:> Florian Lindner 
<address@hidden> writes:
>
>> Am 01.06.2017 um 06:39 schrieb Eric Abrahamsen:
>>> Florian Lindner <address@hidden> writes:
>>>
>>>> Hello,
>>>>
>>>> two questions about moving attachments to org files:
>>>>
>>>> C-c C-a a attaches a file and stores it under ./data/ID/...
>>>>
>>>> Using C-c C-a s I can set another directory a attachment directory.
>>>> Can I make org-mode move the content of the previous
>>>> directory to the new directory?
>>>>
>>>> Can I "reset" the attachment directory, i.e. like C-c C-a s but
>>>> :ATTACH_DIR: is deleted and the contents of the previous
>>>> directory are moved to ./data/ID?
>
>> I hacked together some lines of lisp that should achieve that. It's my first 
>> non-trivial (from my point of trivialness)
>> piece of code. I'm open for any suggestions:
>>
[...]
>
> Looks like a good start! My first comment is, this should definitely be
> written as a patch to `org-attach-set-directory'. It's useful
> functionality, and fits well into the whole system -- so long as you
> give users a chance to say no, I don't see why it shouldn't be part of
> the library.
>
> Various comments:
>
> 1. Use the prefix arg to differentiate between setting and unsetting a
>    directory. Ie, no prefix arg means set (and an empty string for
>    directory name aborts),
>    prefix arg means unset. The `org-attach' dispatch mechanism will pass
>    the prefix arg through to this function.
> 2. Definitely use `read-directory-name'!
> 3. This is a good use of `copy-directory' with the COPY-CONTENTS flag,
>    but I'd still recommend using `directory-files' and then looping over
>    all the files with a `map-y-or-n-p'. That will give users a chance to
>    selectively choose files to move. This is a matter of taste. If you
>    stick with `copy-directory', at least ask the user first.
> 4. I think you're right not to delete the directory afterwards. Best not
>    to assume too much.
> 5. The "else" branch of an `if' statement has an implicit `progn', you
>    don't need to add it.
> 6. Convention is to not put closing parentheses on their own line. Just
>    pile them up at the end of the last form.
> 7. Personally I'd rework things so you only call `org-attach-dir' once.
>    How to handle this depends a bit on when when-let was introduced into
>    Emacs, and whether Org is okay to support it. Probably safest to use
>    when-let*. so:
>
> (when-let* ((attach-dir (org-attach-dir))
>             (target (read-directory-name "Move attachments to: ")))
>
> That way everything will bail if there's no attach dir.

Ok, my new version is here. It should be able to replace 
org-attach-set-directory

(defun flo/org-attach-move (prefix)
  "If PREFIX arg, reset attach directory, else set target directory."
  (interactive "p") ; Correct check for boolean prefix?

  (let ((old-attach-dir (org-attach-dir))
        (new-attach-dir (if (eq prefix 1)
                            (let ((dir (org-entry-get nil "ATTACH_DIR")))
                              (setq dir (read-directory-name "Attachment 
directory: " dir))
                              (org-entry-put nil "ATTACH_DIR" dir)
                              (org-attach-dir t)) ; Changes semantics
                          (org-entry-delete nil "ATTACH_DIR")
                          (org-attach-dir t))))

    (message "old-attach-dir = %s" old-attach-dir)
    (message "new-attach-dir = %s" new-attach-dir)
    (unless (or (string= old-attach-dir new-attach-dir)
                (not old-attach-dir))
      (when (y-or-n-p "Copy over attachments from old directory? ")
        (copy-directory old-attach-dir new-attach-dir t nil t))
      (when (y-or-n-p (concat "Delete " old-attach-dir))
        (shell-command (format "rm -fr %s" old-attach-dir))
        ))))

Some questions about the code

* Is that the correct way to deal with a boolean prefix arg? I'm not interested 
in the value of the prefix arg, only if
it's given or not.

* The code changes the semantics of org-attach-set-directory, because it 
creates the newly set attach dir. IMHO this
makes more sense.

* It deletes only the first part of the dir, e.g. data/83/1234567, it only 
deletes the 1234567 dir, even if 83 is empty
afterwards. But I think that's ok.

> 1. Use the prefix arg to differentiate between setting and unsetting a
>    directory. Ie, no prefix arg means set (and an empty string for
>    directory name aborts),
>    prefix arg means unset. The `org-attach' dispatch mechanism will pass
>    the prefix arg through to this function.

Tried to take that into account.

> 3. This is a good use of `copy-directory' with the COPY-CONTENTS flag,
>    but I'd still recommend using `directory-files' and then looping over
>    all the files with a `map-y-or-n-p'. That will give users a chance to
>    selectively choose files to move. This is a matter of taste. If you
>    stick with `copy-directory', at least ask the user first.
> 4. I think you're right not to delete the directory afterwards. Best not
>    to assume too much.

I think this it is a good compromise now. IMHO the default workflow when 
changing an attachment directory is to move
contents.

> 5. The "else" branch of an `if' statement has an implicit `progn', you
>    don't need to add it.

Yes, I know, but I like it for reasons of consistency with the "then" branch.

Thanks for your comments!

Best,
Florian