From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Lindner Subject: Re: Moving and resetting attachments Date: Wed, 7 Jun 2017 09:52:34 +0200 Message-ID: References: <874lw05njr.fsf@ericabrahamsen.net> <8737bi21dc.fsf@ericabrahamsen.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:58809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dIVlp-0000fT-VG for emacs-orgmode@gnu.org; Wed, 07 Jun 2017 03:52:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dIVlm-0008Bb-05 for emacs-orgmode@gnu.org; Wed, 07 Jun 2017 03:52:50 -0400 Received: from [195.159.176.226] (port=43042 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dIVll-0008BD-P3 for emacs-orgmode@gnu.org; Wed, 07 Jun 2017 03:52:45 -0400 Received: from list by blaine.gmane.org with local (Exim 4.84_2) (envelope-from ) id 1dIVle-0001yD-8e for emacs-orgmode@gnu.org; Wed, 07 Jun 2017 09:52:38 +0200 In-Reply-To: <8737bi21dc.fsf@ericabrahamsen.net> Content-Language: en-US List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: emacs-orgmode@gnu.org Am 02.06.2017 um 11:19 schrieb Eric Abrahamsen:> Florian Lindner writes: > >> Am 01.06.2017 um 06:39 schrieb Eric Abrahamsen: >>> Florian Lindner 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