[-- Attachment #1.1: Type: text/plain, Size: 2084 bytes --] 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 [-- Attachment #1.2: Type: text/html, Size: 3882 bytes --] [-- Attachment #2: orgmode-bash.patch --] [-- Type: text/x-patch, Size: 2036 bytes --] 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)
Hi Pascal, This looks like a good patch. If I could make three changes/requests. 1. This patch should also work for "begin_src bash" code blocks. After a very quick glance it doesn't appear to. 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? 3. Large contributions like this require some FSF paperwork. Please see the following page for information on requirements to contribute. 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
[-- Attachment #1: Type: text/plain, Size: 5964 bytes --] 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 > -- --paf [-- Attachment #2: Type: text/html, Size: 8464 bytes --]
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.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)
Hi Pascal, This patch looks great, thanks for making the additions. Once last request, would you mind formatting it with "git format-patch"? With that done I'll be happy to apply this to the repo. Also, I think the google-wide copyright stuff is sorted out. Best, Pascal Fleury <pascal@telefleuries.com> writes: > 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 >> >> > -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
Hi Eric and Pascal,
Eric Schulte <schulte.eric@gmail.com> writes:
> Also, I think the google-wide copyright stuff is sorted out.
Yes it is: we can accept patch from employees of Google, Inc.
Pascal, I guess it's safe to assume anyone with a @google.com
email address is a Google employee -- let me know if it's not
the case.
Also, if you can sign your patches (git format-patch -s) that'd
be even better, but not mandatory.
Thanks!
--
Bastien
[-- Attachment #1.1: Type: text/plain, Size: 818 bytes --] Hello, Great, thanks for the guidance. I hope I managed it all correctly. On Fri, Apr 11, 2014 at 11:33 AM, Bastien <bzg@gnu.org> wrote: > Hi Eric and Pascal, > > Eric Schulte <schulte.eric@gmail.com> writes: > > > Also, I think the google-wide copyright stuff is sorted out. > > Yes it is: we can accept patch from employees of Google, Inc. > > Good :-) > Pascal, I guess it's safe to assume anyone with a @google.com > email address is a Google employee -- let me know if it's not > the case. > > Yes, I checked internally, and this is a safe assumption. > Also, if you can sign your patches (git format-patch -s) that'd > be even better, but not mandatory. > > I did, also wrote the description of the patch according to the rules I found on orgmode.org > Thanks! > > -- > Bastien > Best regards, --paf [-- Attachment #1.2: Type: text/html, Size: 1922 bytes --] [-- Attachment #2: 0001-ob-shell.el-export-vars-as-arrays-for-sh-code-blocks.patch --] [-- Type: text/x-patch, Size: 8463 bytes --] From c61c28f0b97544a12c3f89180b309cb25ed9f3a9 Mon Sep 17 00:00:00 2001 From: Pascal Fleury <fleury@google.com> Date: Fri, 11 Apr 2014 23:27:02 +0200 Subject: [PATCH] ob-shell.el: export vars as arrays for 'sh' code blocks * lisp/ob-shell.el: added support to serialize vars as arrays or associative arrays as appropriate if it is using bash. * testing/examples/ob-shell-test.org: a file containing a few code blocks both illustrating the use of arrays as well as serving as test for the new export functionality. * testing/lisp/test-ob-shell.el: added a few unit tests that verify that this new logic only triggers for bash and no other shell at this time. 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. When calling the code block with bash, however, it will now export the list as an array, the table as an associative array. A scalar is exported the same way as before. Signed-off-by: Pascal Fleury <fleury@google.com> --- lisp/ob-shell.el | 51 ++++++++++++++++++++-- testing/examples/ob-shell-test.org | 88 ++++++++++++++++++++++++++++++++++++++ testing/lisp/test-ob-shell.el | 39 +++++++++++++++++ 3 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 testing/examples/ob-shell-test.org 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) -- 1.8.3.2
Pascal Fleury <fleury@google.com> writes: > Hello, > > Great, thanks for the guidance. I hope I managed it all correctly. > I've applied this patch. I made a couple of minor changes in a subsequent commit (a7189aa). These were indentation and whitespace changes to enforce two coding conventions, 1. limit line lengths to <80 characters 2. remove dangling parens on lines w/o any other text and to rename one function to be specific to ob-shell.el. Thanks for contributing! > > On Fri, Apr 11, 2014 at 11:33 AM, Bastien <bzg@gnu.org> wrote: > >> Hi Eric and Pascal, >> >> Eric Schulte <schulte.eric@gmail.com> writes: >> >> > Also, I think the google-wide copyright stuff is sorted out. >> >> Yes it is: we can accept patch from employees of Google, Inc. >> >> > Good :-) > > >> Pascal, I guess it's safe to assume anyone with a @google.com >> email address is a Google employee -- let me know if it's not >> the case. >> >> > Yes, I checked internally, and this is a safe assumption. > > >> Also, if you can sign your patches (git format-patch -s) that'd >> be even better, but not mandatory. >> >> > I did, also wrote the description of the patch according to the rules I > found on orgmode.org > > >> Thanks! >> >> -- >> Bastien >> > > > Best regards, > --paf > -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
> > Also, if you can sign your patches (git format-patch -s) that'd > be even better, but not mandatory. > Should I start signing my patches as well? I'm very happy to, I've just never thought about it. If so is there an easy way to make -s a default option for the Org-mode repo? Thanks, -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
[-- Attachment #1: Type: text/plain, Size: 1577 bytes --] Thanks Eric! --pascal On Tue, Apr 15, 2014 at 5:35 AM, Eric Schulte <schulte.eric@gmail.com>wrote: > Pascal Fleury <fleury@google.com> writes: > > > Hello, > > > > Great, thanks for the guidance. I hope I managed it all correctly. > > > > I've applied this patch. > > I made a couple of minor changes in a subsequent commit (a7189aa). > These were indentation and whitespace changes to enforce two coding > conventions, > > 1. limit line lengths to <80 characters > 2. remove dangling parens on lines w/o any other text > > and to rename one function to be specific to ob-shell.el. > > Thanks for contributing! > > > > > On Fri, Apr 11, 2014 at 11:33 AM, Bastien <bzg@gnu.org> wrote: > > > >> Hi Eric and Pascal, > >> > >> Eric Schulte <schulte.eric@gmail.com> writes: > >> > >> > Also, I think the google-wide copyright stuff is sorted out. > >> > >> Yes it is: we can accept patch from employees of Google, Inc. > >> > >> > > Good :-) > > > > > >> Pascal, I guess it's safe to assume anyone with a @google.com > >> email address is a Google employee -- let me know if it's not > >> the case. > >> > >> > > Yes, I checked internally, and this is a safe assumption. > > > > > >> Also, if you can sign your patches (git format-patch -s) that'd > >> be even better, but not mandatory. > >> > >> > > I did, also wrote the description of the patch according to the rules I > > found on orgmode.org > > > > > >> Thanks! > >> > >> -- > >> Bastien > >> > > > > > > Best regards, > > --paf > > > > -- > Eric Schulte > https://cs.unm.edu/~eschulte > PGP: 0x614CA05D > -- --paf [-- Attachment #2: Type: text/html, Size: 2850 bytes --]
Eric Schulte <schulte.eric@gmail.com> writes:
>>
>> Also, if you can sign your patches (git format-patch -s) that'd
>> be even better, but not mandatory.
>>
>
> Should I start signing my patches as well?
>
> I'm very happy to, I've just never thought about it. If so is there an
> easy way to make -s a default option for the Org-mode repo?
>
$ git config --global alias.fm 'format-patch -s'
Then
$ git fm <since-commit>
Nick
Hi Eric,
Eric Schulte <schulte.eric@gmail.com> writes:
>> Also, if you can sign your patches (git format-patch -s) that'd
>> be even better, but not mandatory.
>
> Should I start signing my patches as well?
Up to you, I don't want to enforce a policy here, I just think it's
good to have signed patch for people who don't contribute on behalf
of themselves but do so on behalf of a company they work for.
I'm myself only signing tags for new releases (have been since a
while at least.)
--
Bastien
Test 80/480 fails when I use make up2 with Mac OS X GNU Emacs 24.3.1 (x86_64-apple-darwin12.5.0, Carbon Version 1.6.0 AppKit 1187.4) of 2014-03-05: executing Bash code block... Wrote /var/folders/dt/9hkw2mj50dd566y5qs2s4b8sq962bh/T/tmp-orgtest/ob-input-32986Ogm Code block evaluation complete. Test ob-shell/bash-uses-assoc-arrays backtrace: signal(ert-test-failed (((should (equal "20 cm" (org-babel-execute-s ert-fail(((should (equal "20 cm" (org-babel-execute-src-block))) :fo (if (unwind-protect (setq value-984 (apply fn-982 args-983)) (setq f (let (form-description-986) (if (unwind-protect (setq value-984 (app (let ((value-984 (quote ert-form-evaluation-aborted-985))) (let (for (let ((fn-982 (function equal)) (args-983 (list "20 cm" (org-babel-e (save-restriction (org-babel-next-src-block 2) (let ((fn-982 (functi (progn (org-id-goto "82320a48-3409-49d7-85c9-5de1c6d3ff87") (setq to (unwind-protect (progn (org-id-goto "82320a48-3409-49d7-85c9-5de1c6d (let ((save-match-data-internal (match-data))) (unwind-protect (prog (progn (let ((save-match-data-internal (match-data))) (unwind-protec (unwind-protect (progn (let ((save-match-data-internal (match-data)) (let ((wconfig (current-window-configuration))) (unwind-protect (pro (unwind-protect (let ((wconfig (current-window-configuration))) (unw (let* ((id-location (org-id-find "82320a48-3409-49d7-85c9-5de1c6d3ff (lambda nil (let* ((id-location (org-id-find "82320a48-3409-49d7-85c byte-code("\306\307!q\210\310\216\311 \312\216\313\314\315\316\3 ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc byte-code("\306\307!\211\211r\310\311!q\210\312 d\313\223)L\210)\3 ert-run-test([cl-struct-ert-test ob-shell/bash-uses-assoc-arrays "Ba ert-run-or-rerun-test([cl-struct-ert--stats "\\(org\\|ob\\)" [[cl-st ert-run-tests("\\(org\\|ob\\)" #[(event-type &rest event-args) "\306 ert-run-tests-batch("\\(org\\|ob\\)") ert-run-tests-batch-and-exit("\\(org\\|ob\\)") (let ((org-id-track-globally t) (org-test-selector (if org-test-sele org-test-run-batch-tests("\\(org\\|ob\\)") eval((org-test-run-batch-tests org-test-select-re)) command-line-1(("--eval" "(setq vc-handled-backends nil org-startup- command-line() normal-top-level() Test ob-shell/bash-uses-assoc-arrays condition: (ert-test-failed ((should (equal "20 cm" (org-babel-execute-src-block))) :form (equal "20 cm" "50 dl") :value nil :explanation (array-elt 0 (different-atoms (50 "#x32" "?2") (53 "#x35" "?5"))))) FAILED 80/480 ob-shell/bash-uses-assoc-arrays
Skip Collins <skip.collins@gmail.com> writes:
> Test 80/480 fails when I use make up2 with Mac OS X GNU Emacs 24.3.1
> (x86_64-apple-darwin12.5.0, Carbon Version 1.6.0 AppKit 1187.4) of
> 2014-03-05:
> executing Bash code block...
> Wrote /var/folders/dt/9hkw2mj50dd566y5qs2s4b8sq962bh/T/tmp-orgtest/ob-input-32986Ogm
> Code block evaluation complete.
> Test ob-shell/bash-uses-assoc-arrays backtrace:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I'm not aware of such a test -- where did you get it from?
--
Bastien
As I wrote at the top of the report, the test fails when running make
up2. I am using the unmodified master branch.
--
skip collins
240 687 7802
On Tue, Apr 22, 2014 at 11:37 AM, Bastien <bzg@gnu.org> wrote:
> Skip Collins <skip.collins@gmail.com> writes:
>
>> Test 80/480 fails when I use make up2 with Mac OS X GNU Emacs 24.3.1
>> (x86_64-apple-darwin12.5.0, Carbon Version 1.6.0 AppKit 1187.4) of
>> 2014-03-05:
>> executing Bash code block...
>> Wrote /var/folders/dt/9hkw2mj50dd566y5qs2s4b8sq962bh/T/tmp-orgtest/ob-input-32986Ogm
>> Code block evaluation complete.
>> Test ob-shell/bash-uses-assoc-arrays backtrace:
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> I'm not aware of such a test -- where did you get it from?
>
> --
> Bastien
Skip Collins <skip.collins@gmail.com> writes:
> As I wrote at the top of the report, the test fails when running make
> up2. I am using the unmodified master branch.
You should probably check for "sh" in this local.mk line:
BTEST_OB_LANGUAGES: ...
and replace is by "shell".
--
Bastien
On Tue, Apr 22, 2014 at 4:59 PM, Bastien <bzg@gnu.org> wrote:
> You should probably check for "sh" in this local.mk line:
Nope. I suspect this is a Mac-related thing. Here are the contents of
my local.mk:
ORG_ADD_CONTRIB = org-download.el*
EMACS = /Applications/Emacs.app/Contents/MacOS/Emacs
prefix = /usr/local/share
On Tue, Apr 22, 2014 at 5:04 PM, Skip Collins <skip.collins@gmail.com> wrote: > I suspect this is a Mac-related thing. OS X 10.8.5 ships with bash version 3 which seems to have some issues with declaring arrays: http://stackoverflow.com/questions/6047648/bash-4-associative-arrays-error-declare-a-invalid-option
Skip Collins <skip.collins@gmail.com> writes: > On Tue, Apr 22, 2014 at 4:59 PM, Bastien <bzg@gnu.org> wrote: >> You should probably check for "sh" in this local.mk line: > > Nope. I suspect this is a Mac-related thing. > Here are the contents of > my local.mk: > ORG_ADD_CONTRIB = org-download.el* > EMACS = /Applications/Emacs.app/Contents/MacOS/Emacs > prefix = /usr/local/share What happens if you remove local.mk, then git checkout master, then make config to recreate local.mk? -- Bastien
Skip Collins <skip.collins@gmail.com> writes:
> On Tue, Apr 22, 2014 at 5:04 PM, Skip Collins <skip.collins@gmail.com> wrote:
>> I suspect this is a Mac-related thing.
>
> OS X 10.8.5 ships with bash version 3 which seems to have some issues
> with declaring arrays:
> http://stackoverflow.com/questions/6047648/bash-4-associative-arrays-error-declare-a-invalid-option
Mhh... okay then, thanks for mentioning it.
--
Bastien
On Tue, Apr 22, 2014 at 5:22 PM, Bastien <bzg@gnu.org> wrote:
> Mhh... okay then, thanks for mentioning it.
The stackoverflow link contains what appears to be a good workaround
that functions in old and new versions of bash. Perhaps Pascal Fleury
could modify the org code to avoid using 'declare -A' when bash
version < 4.
[-- Attachment #1: Type: text/plain, Size: 691 bytes --] Hi, Yes that is a test I added, and did not think of older versions of bash (have not used bash3 in quite a long time :-) Actually, if we have bash3, then we should not use the associative arrays at all, but fallback to the lists. Will give it a go. --paf On Wed, Apr 23, 2014 at 3:51 PM, Skip Collins <skip.collins@gmail.com>wrote: > On Tue, Apr 22, 2014 at 5:22 PM, Bastien <bzg@gnu.org> wrote: > > Mhh... okay then, thanks for mentioning it. > > The stackoverflow link contains what appears to be a good workaround > that functions in old and new versions of bash. Perhaps Pascal Fleury > could modify the org code to avoid using 'declare -A' when bash > version < 4. > -- --paf [-- Attachment #2: Type: text/html, Size: 1199 bytes --]
On Wed, Apr 23, 2014 at 10:13 AM, Pascal Fleury <fleury@google.com> wrote:
> (have not used bash3 in quite a long time :-)
Even OS X Mavericks uses bash 3. So it will be around for quite a long time.
Skip Collins <skip.collins@gmail.com> writes: > On Wed, Apr 23, 2014 at 10:13 AM, Pascal Fleury <fleury@google.com> wrote: >> (have not used bash3 in quite a long time :-) > > Even OS X Mavericks uses bash 3. So it will be around for quite a long time. > I believe Bash 4 is GPLv3 and Bash 3 is GPLv2, so it is very possible that OSX will never upgrade to the current bash. If the fix is obvious and simple then it sounds like a win. I suppose the argument could be made that an Emacs project should not go out of it's way to support old versions of software which are potentially being used only to avoid GPLv3 licensing... but I won't make that argument here. -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
[-- Attachment #1: Type: text/plain, Size: 1287 bytes --] On Thu, Apr 24, 2014 at 3:23 AM, Eric Schulte <schulte.eric@gmail.com>wrote: > Skip Collins <skip.collins@gmail.com> writes: > > > On Wed, Apr 23, 2014 at 10:13 AM, Pascal Fleury <fleury@google.com> > wrote: > >> (have not used bash3 in quite a long time :-) > > > > Even OS X Mavericks uses bash 3. So it will be around for quite a long > time. > > > > I believe Bash 4 is GPLv3 and Bash 3 is GPLv2, so it is very possible > that OSX will never upgrade to the current bash. > > If the fix is obvious and simple then it sounds like a win. > > I think it should be. It could prod the version the very first time it runs, and use that. > I suppose the argument could be made that an Emacs project should not go > out of it's way to support old versions of software which are > potentially being used only to avoid GPLv3 licensing... but I won't make > that argument here. > Even array support in ob-shell.el is a win for bash3, and as a user of Mac OSX myself, I would not like a project like org-mode to push such pains onto the users especially as here it is a regression for bash3 users. I'm a bit overbooked these days, I hope you won't mind if it will take a few more days for a fix. --paf > > -- > Eric Schulte > https://cs.unm.edu/~eschulte > PGP: 0x614CA05D > -- --paf [-- Attachment #2: Type: text/html, Size: 2354 bytes --]