Re: [O] [FEATURE] Make header argument :mkdirp yes work for other header arguments not just :tangle
Nicolas Goaziou writes: > Hello, > > stardiviner writes: > >> New patch in attachment. > > Thank you. > >> From aafdd41f7ae5f6218a2be890f58d45be443de4a9 Mon Sep 17 00:00:00 2001 >> From: stardiviner >> Date: Sat, 2 Mar 2019 12:11:47 +0800 >> Subject: [PATCH] ob-core.el: Make :mkdirp work for :dir too >> >> * lisp/ob-core.el (org-babel-execute-src-block): make directory if :dir >> path does not exist when :mkdirp yes exist. >> >> * doc/org-manualo.rg (mkdirp): declare new change in manual. >> >> * etc/ORG-NEWS: declare changes in ORG-NEWS. > > No need to declare changes in changes file. This could end up in an > infloop. > >> + (or (and dir >> +;; Possibly create the parent directories for file. >> +(let ((fnd (file-name-as-directory >> (expand-file-name dir >> + (cond >> + ((member mkdirp '("yes" "t")) (make-directory >> fnd 'parents)) >> + ((member mkdirp '("no" "nil")) nil) >> + (t (make-directory fnd 'parents) >> default-directory)) > > I used: > > (or (and dir > (not (member mkdirp '("no" "nil" nil))) > (progn > (let ((d (file-name-as-directory > (expand-file-name dir > (make-directory d 'parents) > d))) I indeed have this thought of code too. But later changed still. I'm not good at Elisp yet, so hard to figure out which style code is better. I will keep learning. Thanks for your advice and teaching. > Do we need to make a case when dir is a remote? Absolutely, some people use remote dir very often, myself too. I use literate programming in Org Mode, so TRAMP etc support is very necessary. But I don't know how to support for remote dir. > >> +(ert-deftest test-ob-core/dir-mkdirp () >> + (org-test-with-temp-text >> + "#+begin_src sh :mkdirp yes :dir \"data/code\" >> +pwd >> +#+end_src" >> + (org-babel-execute-src-block)) >> + (should (file-directory-p "data/code"))) >> + > > I meant (should (org-test-with-temp-text "..." ... (file-directory-p > "data/code"))) Learned. I will check out to learn a bit of ert testing framework in recently. > > I applied your patch with the changes above. > > Regards, Thanks as always. -- [ stardiviner ] I try to make every word tell the meaning what I want to express. Blog: https://stardiviner.github.io/ IRC(freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
Re: [O] [FEATURE] Make header argument :mkdirp yes work for other header arguments not just :tangle
Hello, stardiviner writes: > New patch in attachment. Thank you. > From aafdd41f7ae5f6218a2be890f58d45be443de4a9 Mon Sep 17 00:00:00 2001 > From: stardiviner > Date: Sat, 2 Mar 2019 12:11:47 +0800 > Subject: [PATCH] ob-core.el: Make :mkdirp work for :dir too > > * lisp/ob-core.el (org-babel-execute-src-block): make directory if :dir > path does not exist when :mkdirp yes exist. > > * doc/org-manualo.rg (mkdirp): declare new change in manual. > > * etc/ORG-NEWS: declare changes in ORG-NEWS. No need to declare changes in changes file. This could end up in an infloop. > +(or (and dir > + ;; Possibly create the parent directories for file. > + (let ((fnd (file-name-as-directory > (expand-file-name dir > + (cond > +((member mkdirp '("yes" "t")) (make-directory > fnd 'parents)) > +((member mkdirp '("no" "nil")) nil) > +(t (make-directory fnd 'parents) > default-directory)) I used: (or (and dir (not (member mkdirp '("no" "nil" nil))) (progn (let ((d (file-name-as-directory (expand-file-name dir (make-directory d 'parents) d))) Do we need to make a case when dir is a remote? > +(ert-deftest test-ob-core/dir-mkdirp () > + (org-test-with-temp-text > + "#+begin_src sh :mkdirp yes :dir \"data/code\" > +pwd > +#+end_src" > + (org-babel-execute-src-block)) > + (should (file-directory-p "data/code"))) > + I meant (should (org-test-with-temp-text "..." ... (file-directory-p "data/code"))) I applied your patch with the changes above. Regards, -- Nicolas Goaziou
Re: [O] [FEATURE] Make header argument :mkdirp yes work for other header arguments not just :tangle
Nicolas Goaziou writes: > Hello, > > stardiviner writes: > >> I hope ~:mkdirp~ header argument can also work for other related header >> arguments >> like ~:dir~, ~:file~ etc not just ~:tangle~. Like following example. >> >> #+begin_src sh :mkdirp yes :dir "data/code/mkdirp/dir" :file "test" :results >> file link >> echo "hello" >> #+end_src >> >> So I added a simple patch to make it work. > > Thank you. Some comments follow. > >> #+cindex: @samp{mkdirp}, header argument >> -The =mkdirp= header argument creates parent directories for tangled >> -files if the directory does not exist. =yes= enables directory >> -creation and =no= inhibits directory creation. >> +The =mkdirp= header argument creates parent directories for =dir= >> +header argument specified path and tangled files if the directory does >> +not exist. =yes= enables directory creation and =no= inhibits >> +directory creation. > > Please also support "t" and "nil", or, more generally, make "no" and > "nil" equivalent, and anything else would be "t". I use ~cond~ to handle this. Don't know whether have better. I can use ~if~ to exclusive on "no" and "nil". But it is not robust. > >> - (or (and dir (file-name-as-directory (expand-file-name dir))) >> + (or (and dir >> +;; Possibly create the parent directories for file. >> +(let (fnd (file-name-as-directory (expand-file-name >> dir))) >> + (if (and (string= mkdirp "yes") fnd) >> + (make-directory fnd 'parents > >> +;;; test-ob-core.el --- tests for ob-core.el > > Tests are in "test-ob.el" file. You should add yours there instead of > creating a new file. I though it is "test-ob-core.el", but have not found it. So I created a new one. :) > >> +(ert-deftest test-ob-core/dir-mkdirp () >> + (org-test-with-temp-text >> + "#+begin_src sh :mkdirp yes :dir \"data/code\" >> +pwd >> +#+end_src" >> + (org-babel-execute-src-block) >> + (should (file-directory-p "data/code" > > Nitpick: `should' is better outside `org-test-with-temp-text'? Updated. > > Could you send an updated patch? New patch in attachment. >From aafdd41f7ae5f6218a2be890f58d45be443de4a9 Mon Sep 17 00:00:00 2001 From: stardiviner Date: Sat, 2 Mar 2019 12:11:47 +0800 Subject: [PATCH] ob-core.el: Make :mkdirp work for :dir too * lisp/ob-core.el (org-babel-execute-src-block): make directory if :dir path does not exist when :mkdirp yes exist. * doc/org-manualo.rg (mkdirp): declare new change in manual. * etc/ORG-NEWS: declare changes in ORG-NEWS. * testing/lisp/test-ob.el: Add a specific testing file for ob-core.el, and add a testing for :mkdir yes work with :dir header argument usage. --- doc/org-manual.org | 7 --- etc/ORG-NEWS| 6 ++ lisp/ob-core.el | 9 - testing/lisp/test-ob.el | 8 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/doc/org-manual.org b/doc/org-manual.org index 00e5e1072..30c797ad7 100644 --- a/doc/org-manual.org +++ b/doc/org-manual.org @@ -17476,9 +17476,10 @@ to source file(s). location. Example: =:tangle FILENAME=. #+cindex: @samp{mkdirp}, header argument -The =mkdirp= header argument creates parent directories for tangled -files if the directory does not exist. =yes= enables directory -creation and =no= inhibits directory creation. +The =mkdirp= header argument creates parent directories for =dir= +header argument specified path and tangled files if the directory does +not exist. =yes= enables directory creation and =no= inhibits +directory creation. #+cindex: @samp{comments}, header argument The =comments= header argument controls inserting comments into diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 057440a71..cd5e4d900 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -157,6 +157,12 @@ A call of ~org-set-tags-command~ with prefix argument C-u C-u avoids the fast tag selection interface and instead offers the plain interface. +*** ~:mkdirp~ now supports create directory for ~:dir~ path + +The ~:mkdirp~ header argument used to only work for ~:tangle~ tangle +files. Now ~:mkdirp~ works for ~:dir~ too. This is more convenient for +specify default directory and with ~:file~ header argument. + * Version 9.2 ** Incompatible changes *** Removal of OrgStruct mode mode and radio lists diff --git a/lisp/ob-core.el b/lisp/ob-core.el index e6f0edba5..c2b629bab 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -677,8 +677,15 @@ block." (replace-regexp-in-string (org-src-coderef-regexp coderef) "" expand nil nil 1 (dir (cdr (assq :dir params))) + (mkdirp (cdr (assq :mkdirp params))) (default-directory - (or (and dir (file-name-as-directory (expand-file-name dir))) + (or (and dir + ;; Possibly create the parent directories for file. + (let ((fnd (file-name-as-directory (expand-file-name dir + (cond +
Re: [O] [FEATURE] Make header argument :mkdirp yes work for other header arguments not just :tangle
Hello, stardiviner writes: > I hope ~:mkdirp~ header argument can also work for other related header > arguments > like ~:dir~, ~:file~ etc not just ~:tangle~. Like following example. > > #+begin_src sh :mkdirp yes :dir "data/code/mkdirp/dir" :file "test" :results > file link > echo "hello" > #+end_src > > So I added a simple patch to make it work. Thank you. Some comments follow. > #+cindex: @samp{mkdirp}, header argument > -The =mkdirp= header argument creates parent directories for tangled > -files if the directory does not exist. =yes= enables directory > -creation and =no= inhibits directory creation. > +The =mkdirp= header argument creates parent directories for =dir= > +header argument specified path and tangled files if the directory does > +not exist. =yes= enables directory creation and =no= inhibits > +directory creation. Please also support "t" and "nil", or, more generally, make "no" and "nil" equivalent, and anything else would be "t". > -(or (and dir (file-name-as-directory (expand-file-name dir))) > +(or (and dir > + ;; Possibly create the parent directories for file. > + (let (fnd (file-name-as-directory (expand-file-name > dir))) > + (if (and (string= mkdirp "yes") fnd) > + (make-directory fnd 'parents > +;;; test-ob-core.el --- tests for ob-core.el Tests are in "test-ob.el" file. You should add yours there instead of creating a new file. > +(ert-deftest test-ob-core/dir-mkdirp () > + (org-test-with-temp-text > + "#+begin_src sh :mkdirp yes :dir \"data/code\" > +pwd > +#+end_src" > + (org-babel-execute-src-block) > + (should (file-directory-p "data/code" Nitpick: `should' is better outside `org-test-with-temp-text'? Could you send an updated patch? Regards, -- Nicolas Goaziou