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
>
>