From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Lindner Subject: Re: Moving and resetting attachments Date: Tue, 13 Jun 2017 10:49:05 +0200 Message-ID: References: <874lw05njr.fsf@ericabrahamsen.net> <8737bi21dc.fsf@ericabrahamsen.net> <87mv9gz43o.fsf@nicolasgoaziou.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:41510) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dKhY3-0004o9-6I for emacs-orgmode@gnu.org; Tue, 13 Jun 2017 04:51:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dKhY0-0000i6-28 for emacs-orgmode@gnu.org; Tue, 13 Jun 2017 04:51:39 -0400 Received: from [195.159.176.226] (port=43580 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dKhXz-0000h4-Qm for emacs-orgmode@gnu.org; Tue, 13 Jun 2017 04:51:35 -0400 Received: from list by blaine.gmane.org with local (Exim 4.84_2) (envelope-from ) id 1dKhXq-0008SR-Ra for emacs-orgmode@gnu.org; Tue, 13 Jun 2017 10:51:26 +0200 In-Reply-To: <87mv9gz43o.fsf@nicolasgoaziou.fr> 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" Cc: emacs-orgmode@gnu.org Am 10.06.2017 um 09:36 schrieb Nicolas Goaziou: > Hello, > > Florian Lindner writes: > >> Ok, my new version is here. It should be able to replace >> org-attach-set-directory > > Thank you. Comments follow. > >> 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. > > No, it should be (interactive "P") so PREFIX, or more commonly, ARG, is > nil when not provided. Thanks!. >> * The code changes the semantics of org-attach-set-directory, because it creates the newly set attach dir. IMHO this >> makes more sense. > > OK. > >> * 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. > > OK. > > Here is an update of your function, with comments and FIXME. The > docstring could certainly be improved, but you get the idea. Yeah, docstring is usually the last I add, since I should at least know what the function is supposed to do ;-) > (defun flo/org-attach-move (&optional arg) > "Move current attachements to another directory. > When ARG is non-nil, reset attach directory. Create directory if > needed." > (interactive "P") > (let ((old (org-attach-dir)) > (new > (progn > (if arg (org-entry-delete nil "ATTACH_DIR") > (let ((dir (read-directory-name > "Attachment directory: " > (org-entry-get nil > "ATTACH_DIR" > (and org-attach-allow-inheritance t))))) What is the use of (and org-attach-allow-inheritance t)? Doesn't it always returns org-attach-allow-inheritance? Anyways, I'm not really sure if I understand the doc of org-entry-get correctly. Does org-entry-get not automatically take inheritance into account, based on the the per-entry or global setting? > (org-entry-put nil "ATTACH_DIR" dir))) > (org-attach-dir t)))) > (message "old-attach-dir = %S" old) ;FIXME: remove? > (message "new-attach-dir = %S" new) ;FIXME: remove? Yes, of course. > (unless (or (string= old new) > (not old)) > ;; FIXME: Need a special case for directory reset (non-nil ARG). Why that? Aren't old and new holding the appropriate dirs in that case and copy over / delete as they should? > ;; FIXME: Maybe `yes-or-no-p' is safer when moving data around? Ok. I wasn't aware of the difference, since I have (fset 'yes-or-no-p 'y-or-n-p) in my .emacs. > (when (y-or-n-p "Copy over attachments from old directory? ") > (copy-directory old-attach-dir new t nil t)) > (when (y-or-n-p (concat "Delete " old)) > ;; FIXME: Why not `delete-directory'? > (shell-command (format "rm -fr %s" old)))))) I took it from org-attach-delete-all. But you're delete-directory is probably better than a shell-command. Latest version: (defun flo/org-attach-move (&optional arg) "Move current attachements to another directory. When ARG is non-nil, reset attach directory. Create directory if needed." (interactive "P") (let ((old (org-attach-dir)) (new (progn (if arg (org-entry-delete nil "ATTACH_DIR") (let ((dir (read-directory-name "Attachment directory: " (org-entry-get nil "ATTACH_DIR" (and org-attach-allow-inheritance t))))) (org-entry-put nil "ATTACH_DIR" dir))) (org-attach-dir t)))) (unless (or (string= old new) (not old)) ;; FIXME: Need a special case for directory reset (non-nil ARG). (when (yes-or-no-p "Copy over attachments from old directory? ") (copy-directory old new t nil t)) (when (yes-or-no-p (concat "Delete " old)) (delete-directory old t))))) Best, Florian