emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Eric Schulte <schulte.eric@gmail.com>
To: Aaron Ecay <aaronecay@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [RFC] [PATCH] ob-core.el: allow the auto-generation of output file names for src blocks.
Date: Sun, 04 May 2014 07:55:57 -0600	[thread overview]
Message-ID: <87ha55a5tb.fsf@gmail.com> (raw)
In-Reply-To: 8761lugqyi.fsf@gmail.com

Hi Aaron,

Thanks for this patch, and especially for including documentation.  It
looks good to me, and it (largely [1]) passes all tests.

Aaron Ecay <aaronecay@gmail.com> writes:

> Hi Eric, Bastien, Achim,
>
> Thanks so much for the feedback.  I’ve adopted the :file-ext approach
> suggested by Bastien, leaving the previous default behavior in place for
> blocks with a :file argument.
>
> 2014ko apirilak 22an, Eric Schulte-ek idatzi zuen:
>
> [...]
>
>
>> One option might be to borrow naming behavior from the comment
>> functionality in ob-tangle which looks like the following (from line 426
>> in ob-tangle.el).
>>
>> (let (...
>>       (source-name
>>        (intern (or (nth 4 info)       ; explicit #+name:
>>                    (format "%s:%d"    ; constructed from header and position
>>                            (or (ignore-errors (nth 4 (org-heading-components)))
>>                                "No heading")
>>                            block-counter))))
>>       ...))
>
> I’m not sure I like this approach.  It relies on counting source
> blocks, so an addition/deletion of a block could change the index.
> I’m worried that this can lead to the accumulation of many output
> files: heading:1.ext, heading:2.ext, ... all with no clear indication
> of what block they were spawned by.  It would also be possible for
> the result links in the buffer to become inconsistent with the actual
> block:auto-generated name mapping.
>
> I think I would prefer the code in this patch to do nothing in this case
> (not create a :file value), but for language-specific code that needs a
> :file to raise an error to prompt the user to add a name.
>

Fair enough, especially given that this default will be applied to *all*
code blocks, this seems like a reasonable approach.

>
>>
>>>
>>> 2. should :output-dir apply to the :file case as well?
>>>
>>
>> If you mean "should :output-dir be used as the base when :file is a
>> relative pathname" then I'd say "yes", and I think if this isn't the
>> current behavior then the current behavior should be changed.
>
> Achim raises a backwards compatibility concern.  I am not sure how
> serious it is: the default settings (no :output-dir) are backwards
> compatible, and if users set that arg we ought to just give them what
> they ask for.
>
> Nonetheless, the new version of the patch conservatively obeys Achim’s
> suggestion.  I can change this to your suggestion, if that is the
> consensus.
>

Please do make this change, I'd then be happy to apply the resulting
patch.

Thanks again!
Eric

>
> To address a comment from Bastien: :output-dir accepts absolute as well
> as relative directory names.  Referring to a “subdirectory” was a
> mistake on my part; the docs in the new patch should be clearer.
>
> The updated patch (now with docs and tests) is attached to this email.
>
> Thanks again,
>
> --
> Aaron Ecay
>
> From 4b428820432752117c60b79da0a79fd4e50e4ba1 Mon Sep 17 00:00:00 2001
> From: Aaron Ecay <aaronecay@gmail.com>
> Date: Tue, 22 Apr 2014 15:13:48 -0400
> Subject: [PATCH] ob-core.el: allow the auto-generation of output file names
>  for src blocks.
>
> * lisp/ob-core.el (org-babel-generate-file-param): New function.
> (org-babel-get-src-block-info): Use it.
> * testing/lisp/test-ob.el (test-org-babel/file-ext-and-output-dir):
> New test.
> * doc/org.texi (Specific header arguments): Add doc for :file-ext and
> :output-dir header args.
> ---
>  doc/org.texi               | 27 +++++++++++++++++++++++++++
>  lisp/ob-core.el            | 34 ++++++++++++++++++++++++++++++++++
>  testing/examples/babel.org | 34 ++++++++++++++++++++++++++++++++++
>  testing/lisp/test-ob.el    | 14 ++++++++++++++
>  4 files changed, 109 insertions(+)
>
> diff --git a/doc/org.texi b/doc/org.texi
> index 2546be1..79cc044 100644
> --- a/doc/org.texi
> +++ b/doc/org.texi
> @@ -14406,6 +14406,8 @@ argument in lowercase letters.  The following header arguments are defined:
>                                  be collected and handled
>  * file::                        Specify a path for file output
>  * file-desc::                   Specify a description for file results
> +* file-ext::                    Specify an extension for file output
> +* output-dir::                  Specify a directory to write file output to
>  * dir::                         Specify the default (possibly remote)
>                                  directory for code block execution
>  * exports::                     Export code and/or results
> @@ -14840,6 +14842,31 @@ description for file code block results which are inserted as Org mode links
>  with no value the link path will be placed in both the ``link'' and the
>  ``description'' portion of the Org mode link.
>  
> +@node file-ext
> +@subsubsection @code{:file-ext}
> +@cindex @code{:file-ext}, src header argument
> +
> +The value of the @code{:file-ext} header argument is used to provide an
> +extension to write the file output to.  It is combined with the
> +@code{#+NAME:} of the source block and the value of the @ref{output-dir}
> +header argument to generate a complete file name.
> +
> +This header arg will be overridden by @code{:file}, and thus has no effect
> +when the latter is specified.
> +
> +@node output-dir
> +@subsubsection @code{:output-dir}
> +@cindex @code{:output-dir}, src header argument
> +
> +The value of the @code{:output-dir} header argument is used to provide a
> +directory to write the file output to.  It may specify an absolute directory
> +(beginning with @code{/}) or a relative directory (without @code{/}).  It is
> +combined with the @code{#+NAME:} of the source block and the value of the
> +@ref{output-dir} header argument to generate a complete file name.
> +
> +This header arg will be overridden by @code{:file}, and thus has no effect
> +when the latter is specified.
> +
>  @node dir
>  @subsubsection @code{:dir} and remote execution
>  @cindex @code{:dir}, src header argument
> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
> index 1348f04..4a3683d 100644
> --- a/lisp/ob-core.el
> +++ b/lisp/ob-core.el
> @@ -283,6 +283,8 @@ Returns a list
>      ;; resolve variable references and add summary parameters
>      (when (and info (not light))
>        (setf (nth 2 info) (org-babel-process-params (nth 2 info))))
> +    (when info
> +      (setf (nth 2 info) (org-babel-generate-file-param name (nth 2 info))))
>      (when info (append info (list name indent head)))))
>  
>  (defvar org-babel-exp-reference-buffer nil
> @@ -2890,6 +2892,38 @@ For the format of SAFE-LIST, see `org-babel-safe-header-args'."
>  		      (member (cdr pair) (cdr entry)))
>  		     (t nil)))))))
>  
> +(defun org-babel-generate-file-param (src-name params)
> +  "Calculate the filename for source block results.
> +
> +The directory is calculated from the :output-dir property of the
> +source block; if not specified, use the current directory.
> +
> +If the source block has a #+NAME and the :file parameter does not
> +contain any period characters, then the :file parameter is
> +treated as an extension, and the output file name is the
> +concatenation of the directory (as calculated above), the block
> +name, a period, and the parameter value as a file extension.
> +Otherwise, the :file parameter is treated as a full file name,
> +and the output file name is the directory (as calculated above)
> +plus the parameter value."
> +  (let* ((file-cons (assq :file params))
> +	 (file-ext-cons (assq :file-ext params))
> +	 (file-ext (cdr-safe file-ext-cons))
> +	 (dir (or (cdr-safe (assq :output-dir params)) "")))
> +    ;; Only operate if:
> +    ;; 1. :file is not given
> +    ;; 2. :file-ext is given
> +    ;; 3. the source block has a #+name
> +    (if (and (not file-cons) file-ext src-name)
> +	(progn
> +	  ;; Create the :output-dir if it does not exist
> +	  (when (and file-ext (not (string= dir "")))
> +	    (make-directory dir t)
> +	    (unless (string-match "/\\'" dir)
> +	      (setq dir (concat dir "/"))))
> +	  (cons (cons :file (concat dir src-name "." file-ext)) params))
> +      ;; Otherwise return params unmodified
> +      params)))
>  
>  ;;; Used by backends: R, Maxima, Octave.
>  (defun org-babel-graphical-output-file (params)
> diff --git a/testing/examples/babel.org b/testing/examples/babel.org
> index 449824f..68ba65a 100644
> --- a/testing/examples/babel.org
> +++ b/testing/examples/babel.org
> @@ -458,3 +458,37 @@ function definition
>                  "0"))))
>    (format "elisp a:%d, b:%d, c:%d, d:%d, e:%d" a b c d e)
>  #+END_SRC
> +
> +* =:file-ext= and =:output-dir= header args
> +  :PROPERTIES:
> +  :ID:       93573e1d-6486-442e-b6d0-3fedbdc37c9b
> +  :END:
> +#+name: file-ext-basic
> +#+BEGIN_SRC emacs-lisp :file-ext txt
> +nil
> +#+END_SRC
> +
> +#+name: file-ext-dir-relative
> +#+BEGIN_SRC emacs-lisp :file-ext txt :output-dir foo
> +nil
> +#+END_SRC
> +
> +#+name: file-ext-dir-relative-slash
> +#+BEGIN_SRC emacs-lisp :file-ext txt :output-dir foo/
> +nil
> +#+END_SRC
> +
> +#+name: file-ext-dir-absolute
> +#+BEGIN_SRC emacs-lisp :file-ext txt :output-dir /tmp
> +nil
> +#+END_SRC
> +
> +#+name: file-ext-file-wins
> +#+BEGIN_SRC emacs-lisp :file-ext txt :file foo.bar
> +nil
> +#+END_SRC
> +
> +#+name: file-ext-file-wins2
> +#+BEGIN_SRC emacs-lisp :output-dir xxx :file foo.bar
> +nil
> +#+END_SRC
> diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el
> index c0ca493..fb791d6 100644
> --- a/testing/lisp/test-ob.el
> +++ b/testing/lisp/test-ob.el
> @@ -1237,6 +1237,20 @@ echo \"$data\"
>  		       (org-babel-execute-src-block)))
>        (should (= noweb-expansions-in-cache-var 2)))))
>  
> +(ert-deftest test-org-babel/file-ext-and-output-dir ()
> +  (org-test-at-id "93573e1d-6486-442e-b6d0-3fedbdc37c9b"
> +    (macrolet ((at-next (&rest body)
> +			`(progn
> +			   (org-babel-next-src-block)
> +			   (save-match-data ,@body))))
> +      (at-next (should (equal "file-ext-basic.txt" (cdr (assq :file (nth 2 (org-babel-get-src-block-info t)))))))
> +      (at-next (should (equal "foo/file-ext-dir-relative.txt" (cdr (assq :file (nth 2 (org-babel-get-src-block-info t)))))))
> +      (at-next (should (equal "foo/file-ext-dir-relative-slash.txt" (cdr (assq :file (nth 2 (org-babel-get-src-block-info t)))))))
> +      (at-next (should (equal "/tmp/file-ext-dir-absolute.txt" (cdr (assq :file (nth 2 (org-babel-get-src-block-info t)))))))
> +      (at-next (should (equal "foo.bar" (cdr (assq :file (nth 2 (org-babel-get-src-block-info t)))))))
> +      (at-next (should (equal "foo.bar" (cdr (assq :file (nth 2 (org-babel-get-src-block-info t)))))))
> +      )))
> +
>  (provide 'test-ob)
>  
>  ;;; test-ob ends here


Footnotes: 
[1]  It took some extra work and manual refreshing of org-id's to get
     test-org-babel/file-ext-and-output-dir to pass.  For some reason
     this tests still fails when I run "make test-dirty", but passes
     when run interactively.  This could easily be particular to my
     system.

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

  parent reply	other threads:[~2014-05-04 16:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-22 19:54 [RFC] [PATCH] ob-core.el: allow the auto-generation of output file names for src blocks Aaron Ecay
2014-04-22 21:22 ` Bastien
2014-04-23  0:04   ` Aaron Ecay
2014-04-23  1:35     ` Eric Schulte
2014-04-23 14:58       ` Bastien
2014-04-28  2:18       ` Aaron Ecay
2014-04-28  6:20         ` Achim Gratz
2014-04-29 13:25         ` Bastien
2014-05-04 13:55         ` Eric Schulte [this message]
2014-05-11 20:38           ` Aaron Ecay
2014-05-14 17:46             ` Achim Gratz
2014-05-15 10:05               ` Bastien
2014-05-16  3:28               ` Aaron Ecay
2014-05-17  6:20                 ` Achim Gratz
2014-04-23  6:27     ` Bastien
2014-04-23 11:07       ` Eric Schulte
2014-04-23 19:44     ` Achim Gratz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ha55a5tb.fsf@gmail.com \
    --to=schulte.eric@gmail.com \
    --cc=aaronecay@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).