Re: Export arrays for 'sh' code blocks when using bash

[Pascal Fleury <pascal@telefleuries.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7302 bytes --]

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.eric@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 <fleury@google.com> writes:
>
> > Hi Eric, see comments inline.
> >
> >
> > On Thu, Mar 27, 2014 at 6:43 PM, Eric Schulte <schulte.eric@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 <fleury@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
>
>

[-- Attachment #1.2: Type: text/html, Size: 10558 bytes --]

[-- Attachment #2: bash-arrays_with_test.patch --]
[-- Type: text/x-patch, Size: 7030 bytes --]

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)