From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pascal Fleury Subject: Re: [PATCH] ob-shell Date: Mon, 23 Jun 2014 08:26:26 +0200 Message-ID: References: <86vbsycepz.fsf@somewhere.org> <87tx8iks22.fsf@bzg.ath.cx> <86ppi6qnb5.fsf@somewhere.org> <87vbrxx4zr.fsf@Rainer.invalid> <86ppi5uwgl.fsf@somewhere.org> <877g4cacn9.fsf@Rainer.invalid> <87vbrt8hpo.fsf_-_@Rainer.invalid> <87ha3ck9as.fsf@gmail.com> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=047d7b41844104abaa04fc7aee6c Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:45689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WyxiV-0000Xp-NQ for emacs-orgmode@gnu.org; Mon, 23 Jun 2014 02:27:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WyxiT-0008Tz-IL for emacs-orgmode@gnu.org; Mon, 23 Jun 2014 02:26:59 -0400 Received: from mail-ve0-x22a.google.com ([2607:f8b0:400c:c01::22a]:35840) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WyxiT-0008SU-BW for emacs-orgmode@gnu.org; Mon, 23 Jun 2014 02:26:57 -0400 Received: by mail-ve0-f170.google.com with SMTP id i13so5630685veh.15 for ; Sun, 22 Jun 2014 23:26:56 -0700 (PDT) In-Reply-To: <87ha3ck9as.fsf@gmail.com> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Eric Schulte Cc: Achim Gratz , emacs-org list --047d7b41844104abaa04fc7aee6c Content-Type: text/plain; charset=UTF-8 Hi Achim, I was wondering how it would behave if the string that is put into a variable contains newlines, backslashes and other things that bash (and other shells) treats specially. The trick with cat and BABEL_TABLE is resistant to this. As in many cases (when I use it at least) the variable come from other parts of the org file and are computed through another language (python in my case) I have such cases that would break. Also, I think the test coverage for such things is limited for ob-shell.el in the org test suite. We should probably add some test cases for this, and then make sure it works with your implementation as well. --paf On Sun, Jun 22, 2014 at 2:50 PM, Eric Schulte wrote: > If this maintains existing functionality, please go ahead and apply it. > > Thanks, > > Achim Gratz writes: > > > Achim Gratz writes: > >> Sebastien Vauban writes: > >>> I just ran `make test' and got the same error for > >>> `ob-shell/bash-uses-assoc-arrays'. > >> > >> Yes, that's because not all versions of bash that have associative > >> arrays can parse the bizarre quoting style that goes through a > >> sub-process and here-document that is used to fill in the parameters. > > > > Here's a patch that implements the suggestion and tested to work > > correctly with Cygwin and Linux. > > > > From a79aff65d562e59ed4e01e550224eb96a665c1ae Mon Sep 17 00:00:00 2001 > > From: Achim Gratz > > Date: Thu, 19 Jun 2014 21:23:28 +0200 > > Subject: [PATCH] ob-shell: stratify shell variable quoting > > > > * lisp/ob-shell.el: Remove unused defcustom > > `org-babel-sh-var-quote-fmt'. > > (org-babel-variable-assignments:bash_array): > > (org-babel-variable-assignments:bash_assoc): Remove superfluous > > `mapcar' and double quotes around parameters. > > (org-babel-sh-var-to-sh): Single-quote the whole string and escape > > all single quotes in the original string. > > --- > > lisp/ob-shell.el | 42 ++++++++++++++++++------------------------ > > 1 file changed, 18 insertions(+), 24 deletions(-) > > > > diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el > > index 474a8f2..7d87026 100644 > > --- a/lisp/ob-shell.el > > +++ b/lisp/ob-shell.el > > @@ -45,12 +45,6 @@ (defcustom org-babel-sh-command shell-file-name > > :group 'org-babel > > :type 'string) > > > > -(defcustom org-babel-sh-var-quote-fmt > > - "$(cat <<'BABEL_TABLE'\n%s\nBABEL_TABLE\n)" > > - "Format string used to escape variables when passed to shell scripts." > > - :group 'org-babel > > - :type 'string) > > - > > (defcustom org-babel-shell-names > > '("sh" "bash" "csh" "ash" "dash" "ksh" "mksh" "posh") > > "List of names of shell supported by babel shell code blocks." > > @@ -113,28 +107,26 @@ (defun org-babel-variable-assignments:sh-generic > > (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) > > - "\" \""))) > > + (format "unset %s\ndeclare -a %s=( %s )" > > + varname varname > > + (mapconcat > > + (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"))) > > + (mapconcat > > + (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." > > @@ -163,8 +155,10 @@ (defun org-babel-sh-var-to-sh (var &optional sep > hline) > > "Convert an elisp value to a shell variable. > > Convert an elisp var into a string of shell commands specifying a > > var of the same value." > > - (format org-babel-sh-var-quote-fmt > > - (org-babel-sh-var-to-string var sep hline))) > > + (concat "'" (replace-regexp-in-string > > + "'" "'\"'\"'" > > + (org-babel-sh-var-to-string var sep hline)) > > + "'")) > > > > (defun org-babel-sh-var-to-string (var &optional sep hline) > > "Convert an elisp value to a string." > > -- > > 2.0.0 > > > > > > > > Regards, > > Achim. > > -- > Eric Schulte > https://cs.unm.edu/~eschulte > PGP: 0x614CA05D (see https://u.fsf.org/yw) > > -- --paf --047d7b41844104abaa04fc7aee6c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Achim,

I was wondering how it would = behave if the string that is put into a variable contains newlines, backsla= shes and other things that bash (and other shells) treats specially. The tr= ick with cat and BABEL_TABLE is resistant to this. As in many cases (when I= use it at least) the variable come from other parts of the org file and ar= e computed through another language (python in my case) I have such cases t= hat would break.

Also, I think the test coverage for such things is limi= ted for ob-shell.el in the org test suite.
We should probably add= some test cases for this, and then make sure it works with your implementa= tion as well.

--paf


On Sun, Jun 22, 2014 at 2:50 PM, Eric Schulte <schulte.eric@gmail.com> wrote:
If this maintains existing functionality, pl= ease go ahead and apply it.

Thanks,

Achim Gratz <Stromeko@nexgo.de&= gt; writes:

> Achim Gratz writes:
>> Sebastien Vauban writes:
>>> I just ran `make test' and got the same error for
>>> `ob-shell/bash-uses-assoc-arrays'.
>>
>> Yes, that's because not all versions of bash that have associa= tive
>> arrays can parse the bizarre quoting style that goes through a
>> sub-process and here-document that is used to fill in the paramete= rs.
>
> Here's a patch that implements the suggestion and tested to work > correctly with Cygwin and Linux.
>
> From a79aff65d562e59ed4e01e550224eb96a665c1ae Mon Sep 17 00:00:00 2001=
> From: Achim Gratz <Stromeko@Stromeko.DE>
> Date: Thu, 19 Jun 2014 21:23:28 +0200
> Subject: [PATCH] ob-shell: stratify shell variable quoting
>
> * lisp/ob-shell.el: Remove unused defcustom
> =C2=A0 `org-babel-sh-var-quote-fmt'.
> =C2=A0 (org-babel-variable-assignments:bash_array):
> =C2=A0 (org-babel-variable-assignments:bash_assoc): Remove superfluous=
> =C2=A0 `mapcar' and double quotes around parameters.
> =C2=A0 (org-babel-sh-var-to-sh): Single-quote the whole string and esc= ape
> =C2=A0 all single quotes in the original string.
> ---
> =C2=A0lisp/ob-shell.el | 42 ++++++++++++++++++------------------------=
> =C2=A01 file changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el
> index 474a8f2..7d87026 100644
> --- a/lisp/ob-shell.el
> +++ b/lisp/ob-shell.el
> @@ -45,12 +45,6 @@ (defcustom org-babel-sh-command shell-file-name
> =C2=A0 =C2=A0:group 'org-babel
> =C2=A0 =C2=A0:type 'string)
>
> -(defcustom org-babel-sh-var-quote-fmt
> - =C2=A0"$(cat <<'BABEL_TABLE'\n%s\nBABEL_TABLE\n)&= quot;
> - =C2=A0"Format string used to escape variables when passed to sh= ell scripts."
> - =C2=A0:group 'org-babel
> - =C2=A0:type 'string)
> -
> =C2=A0(defcustom org-babel-shell-names
> =C2=A0 =C2=A0'("sh" "bash" "csh" &qu= ot;ash" "dash" "ksh" "mksh" "posh&q= uot;)
> =C2=A0 =C2=A0"List of names of shell supported by babel shell cod= e blocks."
> @@ -113,28 +107,26 @@ (defun org-babel-variable-assignments:sh-generic=
> =C2=A0(defun org-babel-variable-assignments:bash_array
> =C2=A0 =C2=A0 =C2=A0(varname values &optional sep hline)
> =C2=A0 =C2=A0"Returns a list of statements declaring the values a= s a bash array."
> - =C2=A0(format "unset %s\ndeclare -a %s=3D( \"%s\" )&q= uot;
> - =C2=A0 =C2=A0 varname varname
> - =C2=A0 =C2=A0 (mapconcat 'identity
> - =C2=A0 =C2=A0 =C2=A0 (mapcar
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 (lambda (value) (org-babel-sh-var-to-sh = value sep hline))
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 values)
> - =C2=A0 =C2=A0 =C2=A0 "\" \"")))
> + =C2=A0(format "unset %s\ndeclare -a %s=3D( %s )"
> + =C2=A0 =C2=A0 =C2=A0 varname varname
> + =C2=A0 =C2=A0 =C2=A0 (mapconcat
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0(lambda (value) (org-babel-sh-var-to-sh v= alue sep hline))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0values
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0" ")))
>
> =C2=A0(defun org-babel-variable-assignments:bash_assoc
> =C2=A0 =C2=A0 =C2=A0(varname values &optional sep hline)
> =C2=A0 =C2=A0"Returns a list of statements declaring the values a= s bash associative array."
> =C2=A0 =C2=A0(format "unset %s\ndeclare -A %s\n%s"
> =C2=A0 =C2=A0 =C2=A0varname varname
> - =C2=A0 =C2=A0(mapconcat 'identity
> - =C2=A0 =C2=A0 =C2=A0(mapcar
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0(lambda (items)
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(format "%s[\"%s\"]= =3D%s"
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0varname
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(org-babel-sh-var-to-sh (ca= r items) sep hline)
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(org-babel-sh-var-to-sh (cd= r items) sep hline)))
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0values)
> - =C2=A0 =C2=A0 =C2=A0"\n")))
> + =C2=A0 =C2=A0(mapconcat
> + =C2=A0 =C2=A0 (lambda (items)
> + =C2=A0 =C2=A0 =C2=A0 (format "%s[%s]=3D%s"
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0varname
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(org-babel-sh-var-to-sh (ca= r items) sep hline)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(org-babel-sh-var-to-sh (cd= r items) sep hline)))
> + =C2=A0 =C2=A0 values
> + =C2=A0 =C2=A0 "\n")))
>
> =C2=A0(defun org-babel-variable-assignments:bash (varname values &= optional sep hline)
> =C2=A0 =C2=A0"Represents the parameters as useful Bash shell vari= ables."
> @@ -163,8 +155,10 @@ (defun org-babel-sh-var-to-sh (var &optional = sep hline)
> =C2=A0 =C2=A0"Convert an elisp value to a shell variable.
> =C2=A0Convert an elisp var into a string of shell commands specifying = a
> =C2=A0var of the same value."
> - =C2=A0(format org-babel-sh-var-quote-fmt
> - =C2=A0 =C2=A0 =C2=A0 (org-babel-sh-var-to-string var sep hline))) > + =C2=A0(concat "'" (replace-regexp-in-string
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"'" "= 9;\"'\"'"
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(org-babel-sh-var-to-string= var sep hline))
> + =C2=A0 =C2=A0 =C2=A0 "'"))
>
> =C2=A0(defun org-babel-sh-var-to-string (var &optional sep hline)<= br> > =C2=A0 =C2=A0"Convert an elisp value to a string."
> --
> 2.0.0
>
>
>
> Regards,
> Achim.

--
Eric Schulte
https://cs.unm.e= du/~eschulte
PGP: 0x614CA05D (see htt= ps://u.fsf.org/yw)




-- --paf
--047d7b41844104abaa04fc7aee6c--