Hello, Eric Abrahamsen <e...@ericabrahamsen.net> writes:
> 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. FWIW, I agree. > Various comments: [...] > 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. What about using `rename-file' so as to move the whole directory to the new location? Maybe a defcustom could let the user choose between moving and copying the attachment directory. > 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: "))) We cannot use `when-let*'. Besides, (let ((attch-dir (org-attach-dir))) (when attach-dir (let ((target (read-directory-name "Move attachments to: "))) ...))) is fine, too, or even (let ((attch-dir (or (org-attach-dir) (error "No attachment directory"))) (target (read-directory-name "Move attachments to: "))) ...) Regards, -- Nicolas Goaziou