Hi Eric,
Please find attached a new patch that contains the same changes of the
ob-shell.el exporter as before, but also contains unit tests for checking
that it works.
Let me know if you need some changes in there, and how the google-wide
copyright issues comes along. My information came from Chris DiBona (our
master of open-source licenses), is this helps to find the relevant
documents.
Thanks,
Pascal
On Sat, Mar 29, 2014 at 8:37 PM, Eric Schulte <schulte.e...@gmail.com>wrote:
> Thanks for making these changes. Once they're in and Bastien figures
> out how the google-wide copyright attribution works we should be good to
> go.
>
> Thanks!
>
> Pascal Fleury <fle...@google.com> writes:
>
> > Hi Eric, see comments inline.
> >
> >
> > On Thu, Mar 27, 2014 at 6:43 PM, Eric Schulte <schulte.e...@gmail.com
> >wrote:
> >
> >> Hi Pascal,
> >>
> >> This looks like a good patch. If I could make three changes/requests.
> >>
> >>
> > Thanks, I am happy to see it makes sense.
> >
> >
> >> 1. This patch should also work for "begin_src bash" code blocks. After
> >> a very quick glance it doesn't appear to.
> >>
> >> I have actually tried this with the latest org-mode release (8.2.5h) and
> > it did plainly not accept 'begin_src bash'. Maybe that's new with the
> > ob-shell.el (I noticed it has been renamed :-)
> >
> >
> >> 2. Could you include one or two tests ensuring that this patch does what
> >> it intends with bash code blocks, and does not affect code blocks
> >> executed with bin/sh?
> >>
> >
> > Will do, seeing it has some traction.
> >
> >
> >>
> >> 3. Large contributions like this require some FSF paperwork. Please see
> >> the following page for information on requirements to contribute.
> >>
> >>
> > I checked internally, and apparently, Google has already such a signed
> > document. It would run on the same document, as I am a google employee.
> >
> >
> >> http://orgmode.org/worg/org-contribute.html
> >>
> >> Thanks, and I look forward to seeing this merged into Org-mode!
> >> Eric
> >>
> >> Pascal Fleury <fle...@google.com> writes:
> >>
> >> > Hello,
> >> >
> >> > I'dl like to propose a patch for inclusion into org-mode (ob-shell.el
> in
> >> > particular).
> >> >
> >> > *TL;DR:* use arrays and associative arrays when exporting variables to
> >> bash
> >> > in 'sh' code blocks.
> >> >
> >> > *Details:*
> >> > When variables are defined in a 'sh' code block, they are exported as
> >> > strings. when the variable itself is an array or a table, then we
> simply
> >> > get a shell variable that contains the list of all values in a
> >> > non-structured form. E.g.
> >> >
> >> > #+NAME: my_list
> >> > | one |
> >> > | two |
> >> > | three |
> >> >
> >> > #+NAME: experiment
> >> > | name | first_attempt |
> >> > | date | [2014-03-27 Thu] |
> >> > | user | fleury |
> >> >
> >> > #+BEGIN_SRC sh :var scalar="some value" :var array=my_list :var
> >> table=config
> >> > echo ${scalar} # -> prints 'some value'
> >> > echo ${array} # -> prints 'one two three'
> >> > echo ${table} # -> prints 'first attempt [2014-03-27 Thu] fleury'
> >> > #+END_SRC
> >> >
> >> > This will print simple strings. Also, there is no easy way to access
> the
> >> > date of the experiment, for example. Now bash has things like arrays
> and
> >> > associative arrays, but the current ob-shell.el does not use these.
> >> > Probably because their syntax is bash-specific, and ob-shell.el is
> >> > shell-agnostic.
> >> >
> >> > My patch (attached) changes this in the case you have
> >> > (setq org-babel-sh-command "bash")
> >> > in your emacs config somewhere. If any other value is used, it
> continues
> >> to
> >> > export them as we do today (I don't know enough about other shells).
> >> >
> >> > In that case, it will export the list as an array, the table as an
> >> > associative array, and the scalar as it does already. So the 'sh' code
> >> > block could then use
> >> >
> >> > #+BEGIN_SRC sh :var scalar="some value" :var array=my_list :var
> >> table=config
> >> > echo ${scalar}
> >> > echo ${array[1]} # -> prints "two"
> >> > echo ${table[user]} # -> prints "fleury"
> >> > #+END_SRC
> >> >
> >> > In the case we have a bigger table, then the first row is used as key,
> >> the
> >> > others are represented as a string with all the values (as it is for
> >> array
> >> > currently). bash does not have multi-dimensional arrays, so it needs
> to
> >> be
> >> > a string.
> >> >
> >> > This makes writing shell snippets much easier in my experience, and
> there
> >> > I'd like to propose this fix for the org-mode community at large.
> >> >
> >> > --paf
> >> >
> >> > diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el
> >> > index 3ede701..0a691e2 100644
> >> > --- a/lisp/ob-shell.el
> >> > +++ b/lisp/ob-shell.el
> >> > @@ -106,6 +106,30 @@ This function is called by
> >> `org-babel-execute-src-block'."
> >> >
> >> > ;; helper functions
> >> >
> >> > +(defun org-babel-variable-assignments:bash_array (varname values
> >> &optional sep hline)
> >> > + "Returns a list of statements declaring the values as a bash
> array."
> >> > + (format "declare -a %s=( \"%s\" )"
> >> > + varname
> >> > + (mapconcat 'identity
> >> > + (mapcar
> >> > + (lambda (value) (org-babel-sh-var-to-sh value sep hline))
> >> > + values)
> >> > + "\" \"")))
> >> > +
> >> > +(defun org-babel-variable-assignments:bash_associative (varname
> values
> >> &optional sep hline)
> >> > + "Returns a list of statements declaring the values as bash
> >> associative array."
> >> > + (format "declare -A %s\n%s"
> >> > + varname
> >> > + (mapconcat 'identity
> >> > + (mapcar
> >> > + (lambda (items)
> >> > + (format "%s[\"%s\"]=%s"
> >> > + varname
> >> > + (org-babel-sh-var-to-sh (car items) sep hline)
> >> > + (org-babel-sh-var-to-sh (cdr items) sep hline)))
> >> > + values)
> >> > + "\n")))
> >> > +
> >> > (defun org-babel-variable-assignments:sh (params)
> >> > "Return list of shell statements assigning the block's variables."
> >> > (let ((sep (cdr (assoc :separator params)))
> >> > @@ -114,9 +138,17 @@ This function is called by
> >> `org-babel-execute-src-block'."
> >> > "hline"))))
> >> > (mapcar
> >> > (lambda (pair)
> >> > - (format "%s=%s"
> >> > - (car pair)
> >> > - (org-babel-sh-var-to-sh (cdr pair) sep hline)))
> >> > + (if (and (string= org-babel-sh-command "bash") (listp (cdr
> >> pair)))
> >> > + (if (listp (car (cdr pair)))
> >> > + (org-babel-variable-assignments:bash_associative
> >> > + (car pair) (cdr pair) sep hline)
> >> > + (org-babel-variable-assignments:bash_array
> >> > + (car pair) (cdr pair) sep) hline)
> >> > + (format "%s=%s"
> >> > + (car pair)
> >> > + (org-babel-sh-var-to-sh (cdr pair) sep hline))
> >> > + )
> >> > + )
> >> > (mapcar #'cdr (org-babel-get-header params :var)))))
> >> >
> >> > (defun org-babel-sh-var-to-sh (var &optional sep hline)
> >>
> >> --
> >> Eric Schulte
> >> https://cs.unm.edu/~eschulte
> >> PGP: 0x614CA05D
> >>
>
> --
> Eric Schulte
> https://cs.unm.edu/~eschulte
> PGP: 0x614CA05D
>
>
diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el
index 3ede701..d7f1802 100644
--- a/lisp/ob-shell.el
+++ b/lisp/ob-shell.el
@@ -105,6 +105,44 @@ This function is called by `org-babel-execute-src-block'."
buffer)))
;; helper functions
+(defun org-babel-variable-assignments:generic (varname values &optional sep hline)
+ "Returns a list of statements declaring the values as a generic variable."
+ (format "%s=%s" varname (org-babel-sh-var-to-sh values sep hline)))
+
+(defun org-babel-variable-assignments:bash_array (varname values &optional sep hline)
+ "Returns a list of statements declaring the values as a bash array."
+ (format "unset %s\ndeclare -a %s=( \"%s\" )"
+ varname varname
+ (mapconcat 'identity
+ (mapcar
+ (lambda (value) (org-babel-sh-var-to-sh value sep hline))
+ values)
+ "\" \"")))
+
+(defun org-babel-variable-assignments:bash_assoc (varname values &optional sep hline)
+ "Returns a list of statements declaring the values as bash associative array."
+ (format "unset %s\ndeclare -A %s\n%s"
+ varname varname
+ (mapconcat 'identity
+ (mapcar
+ (lambda (items)
+ (format "%s[\"%s\"]=%s"
+ varname
+ (org-babel-sh-var-to-sh (car items) sep hline)
+ (org-babel-sh-var-to-sh (cdr items) sep hline)))
+ values)
+ "\n")))
+
+(defun org-babel-variable-assignments:bash (varname values &optional sep hline)
+ "Represents the parameters as useful Bash shell variables."
+ (if (listp values)
+ (if (and (listp (car values)) (= 1 (length (car values))))
+ (org-babel-variable-assignments:bash_array varname values sep hline)
+ (org-babel-variable-assignments:bash_assoc varname values sep hline)
+ )
+ (org-babel-variable-assignments:generic varname values sep hline)
+ )
+)
(defun org-babel-variable-assignments:sh (params)
"Return list of shell statements assigning the block's variables."
@@ -114,10 +152,15 @@ This function is called by `org-babel-execute-src-block'."
"hline"))))
(mapcar
(lambda (pair)
- (format "%s=%s"
- (car pair)
- (org-babel-sh-var-to-sh (cdr pair) sep hline)))
- (mapcar #'cdr (org-babel-get-header params :var)))))
+ (if (string= org-babel-sh-command "bash")
+ (org-babel-variable-assignments:bash
+ (car pair) (cdr pair) sep hline)
+ (org-babel-variable-assignments:generic
+ (car pair) (cdr pair) sep hline)
+ )
+ )
+ (mapcar #'cdr (org-babel-get-header params :var))))
+)
(defun org-babel-sh-var-to-sh (var &optional sep hline)
"Convert an elisp value to a shell variable.
diff --git a/testing/examples/ob-shell-test.org b/testing/examples/ob-shell-test.org
new file mode 100644
index 0000000..a54e5c0
--- /dev/null
+++ b/testing/examples/ob-shell-test.org
@@ -0,0 +1,88 @@
+#+Title: a collection of examples for ob-shell tests
+#+OPTIONS: ^:nil
+
+* Sample data structures
+#+NAME: sample_array
+| one |
+| two |
+| three |
+
+#+NAME: sample_mapping_table
+| first | one |
+| second | two |
+| third | three |
+
+#+NAME: sample_big_table
+| bread | 2 | kg |
+| spaghetti | 20 | cm |
+| milk | 50 | dl |
+
+* Array tests
+ :PROPERTIES:
+ :ID: 0ba56632-8dc1-405c-a083-c204bae477cf
+ :END:
+** Generic shell: no arrays
+#+begin_src sh :exports results :var array=sample_array
+echo ${array}
+#+end_src
+
+#+RESULTS:
+: one two three
+
+** Bash shell: support for arrays
+Bash will see a simple indexed array. In this test, we check that the
+returned value is indeed only the first item of the array, as opposed to
+the generic serialiation that will return all elements of the array as
+a single string.
+#+begin_src bash :exports results :var array=sample_array
+echo ${array}
+#+end_src
+
+#+RESULTS:
+: one
+
+* Associative array tests (simple map)
+ :PROPERTIES:
+ :ID: bec1a5b0-4619-4450-a8c0-2a746b44bf8d
+ :END:
+** Generic shell: no special handing
+The shell will see all values as a single string.
+#+begin_src sh :exports results :var table=sample_mapping_table
+echo ${table}
+#+end_src
+
+#+RESULTS:
+: first one second two third three
+
+** Bash shell: support for associative arrays
+Bash will see a table that contains the first column as the 'index'
+of the associative array, and the second column as the value.
+#+begin_src bash :exports results :var table=sample_mapping_table
+echo ${table[second]}
+#+end_src
+
+#+RESULTS:
+: two
+
+* Associative array tests (more than 2 columns)
+ :PROPERTIES:
+ :ID: 82320a48-3409-49d7-85c9-5de1c6d3ff87
+ :END:
+** Generic shell: no special handing
+#+begin_src sh :exports results :var table=sample_big_table
+echo ${table}
+#+end_src
+
+#+RESULTS:
+: bread 2 kg spaghetti 20 cm milk 50 dl
+
+** Bash shell: support for associative arrays with lists
+Bash will see an associative array that contains each row as a single
+string. Bash cannot handle lists in associative arrays.
+#+begin_src bash :exports results :var table=sample_big_table
+echo ${table[spaghetti]}
+#+end_src
+
+#+RESULTS:
+: 20 cm
+
diff --git a/testing/lisp/test-ob-shell.el b/testing/lisp/test-ob-shell.el
index 2b3e48f..58a7859 100644
--- a/testing/lisp/test-ob-shell.el
+++ b/testing/lisp/test-ob-shell.el
@@ -47,6 +47,45 @@ ob-comint.el, which was not previously tested."
(should res)
(should (listp res))))
+; A list of tests using the samples in ob-shell-test.org
+(ert-deftest ob-shell/generic-uses-no-arrays ()
+ "No arrays for generic"
+ (org-test-at-id "0ba56632-8dc1-405c-a083-c204bae477cf"
+ (org-babel-next-src-block)
+ (should (equal "one two three" (org-babel-execute-src-block)))))
+
+(ert-deftest ob-shell/bash-uses-arrays ()
+ "Bash arrays"
+ (org-test-at-id "0ba56632-8dc1-405c-a083-c204bae477cf"
+ (org-babel-next-src-block 2)
+ (should (equal "one" (org-babel-execute-src-block)))))
+
+(ert-deftest ob-shell/generic-uses-no-assoc-arrays ()
+ "No associative arrays for generic"
+ (org-test-at-id "bec1a5b0-4619-4450-a8c0-2a746b44bf8d"
+ (org-babel-next-src-block)
+ (should (equal "first one second two third three"
+ (org-babel-execute-src-block)))))
+
+(ert-deftest ob-shell/bash-uses-assoc-arrays ()
+ "Bash associative arrays"
+ (org-test-at-id "bec1a5b0-4619-4450-a8c0-2a746b44bf8d"
+ (org-babel-next-src-block 2)
+ (should (equal "two" (org-babel-execute-src-block)))))
+
+(ert-deftest ob-shell/generic-uses-no-assoc-arrays ()
+ "No associative arrays for generic"
+ (org-test-at-id "82320a48-3409-49d7-85c9-5de1c6d3ff87"
+ (org-babel-next-src-block)
+ (should (equal "bread 2 kg spaghetti 20 cm milk 50 dl"
+ (org-babel-execute-src-block)))))
+
+(ert-deftest ob-shell/bash-uses-assoc-arrays ()
+ "Bash associative arrays as strings for the row"
+ (org-test-at-id "82320a48-3409-49d7-85c9-5de1c6d3ff87"
+ (org-babel-next-src-block 2)
+ (should (equal "20 cm" (org-babel-execute-src-block)))))
+
(provide 'test-ob-shell)
