Re: [O] [FEATURE] Make header argument :mkdirp yes work for other header arguments not just :tangle

2019-03-04 Thread stardiviner


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

2019-03-04 Thread Nicolas Goaziou
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

2019-03-03 Thread stardiviner

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

2019-03-03 Thread Nicolas Goaziou
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