From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aaron Ecay Subject: Re: Bug: org-babel-script-escape misses a lonely double quote [8.2.10 (8.2.10-23-g1ec416-elpaplus @ c:/Users/jowik/.emacs.d/elpa/org-plus-contrib-20141208/)] Date: Fri, 16 Jan 2015 18:03:13 -0500 Message-ID: <87lhl2jpla.fsf@gmail.com> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:56153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCFvD-0002P3-Bq for emacs-orgmode@gnu.org; Fri, 16 Jan 2015 18:03:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YCFvA-00029C-3K for emacs-orgmode@gnu.org; Fri, 16 Jan 2015 18:03:19 -0500 Received: from mail-qa0-x233.google.com ([2607:f8b0:400d:c00::233]:43734) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCFv9-00028L-Iq for emacs-orgmode@gnu.org; Fri, 16 Jan 2015 18:03:15 -0500 Received: by mail-qa0-f51.google.com with SMTP id f12so16695492qad.10 for ; Fri, 16 Jan 2015 15:03:15 -0800 (PST) In-Reply-To: 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: =?utf-8?Q?Johan_W=2E_Kl=C3=BCwer?= , emacs-orgmode@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Johan, Thanks for raising this issue. 2014ko abenudak 10an, "Johan W. Kl=C3=BCwer"-ek idatzi zuen: >=20 > I encountered a problem with unmatched double quotes in an org-babel > table of results. Seems like the issue lies with escaped double quotes: >=20 > (org-babel-script-escape "[[\"a\", \"b\\\"\"]]") >=20 > returns error > Invalid read syntax: "] in a list". >=20 > Adding the following after line 41 of org-babel-script-escape appears to > fix the problem. >=20 > (92 (if in-single ; \ > (append (list 92 34 out)) > (setq in-double (not in-double)) (cons 92 out))) >=20 > We now get >=20 > (org-babel-script-escape "[[\"a\", \"b\\\"\"]]") > (("a" "b\"")) Unfortunately, while your suggested modification does correct the issue in your case, it is not a very general fix. As you noted, the org-babel-script-escape function did not take account of backslash escapes; it had other issues as well. Attached is a proposed patch which (tries to) improve the situation, including adding a test for this function. (There are also two clean-up patches attached.) Unfortunately these functions try to interpret the list and string formats of many different programming languages. So there will always be corner cases. (One obvious one that exists now is: should \n escape sequences be turned into newline characters?) In the long run, I=E2=80=99d be tempted to say that babel should only provi= de functions to convert JSON, CSV (and perhaps a few other widely-used formats) to lisp (piggybacking off of existing libraries, when available). Org-babel-script-escape could be kept but marked deprecated, in order to encourage babel languages to either use one of the common formats or do any needed processing themselves in a guaranteed-proper way. Comments are welcome. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-babel-Remove-functions-which-are-never-called.patch >From e8d72b293fa67662c0f7fd58c4837c161ed529db Mon Sep 17 00:00:00 2001 From: Aaron Ecay Date: Fri, 16 Jan 2015 17:40:24 -0500 Subject: [PATCH 1/3] babel: Remove functions which are never called. * lisp/ob-awk.el (org-babel-awk-table-or-string): * lisp/ob-shell.el (org-babel-sh-table-or-results): Remove. --- lisp/ob-awk.el | 5 ----- lisp/ob-shell.el | 6 ------ 2 files changed, 11 deletions(-) diff --git a/lisp/ob-awk.el b/lisp/ob-awk.el index 6c0fb86..a96ba1a 100644 --- a/lisp/ob-awk.el +++ b/lisp/ob-awk.el @@ -105,11 +105,6 @@ called by `org-babel-execute-src-block'" (mapconcat echo-var var "\n")) (t (funcall echo-var var))))) -(defun org-babel-awk-table-or-string (results) - "If the results look like a table, then convert them into an -Emacs-lisp table, otherwise return the results as a string." - (org-babel-script-escape results)) - (provide 'ob-awk) diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el index aa14a69..4d6d7c4 100644 --- a/lisp/ob-shell.el +++ b/lisp/ob-shell.el @@ -164,12 +164,6 @@ var of the same value." (mapconcat echo-var var "\n")) (t (funcall echo-var var))))) -(defun org-babel-sh-table-or-results (results) - "Convert RESULTS to an appropriate elisp value. -If the results look like a table, then convert them into an -Emacs-lisp table, otherwise return the results as a string." - (org-babel-script-escape results)) - (defun org-babel-sh-initiate-session (&optional session params) "Initiate a session named SESSION according to PARAMS." (when (and session (not (string= session "none"))) -- 2.2.2 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0002-babel-Remove-functions-which-are-just-indirection-ar.patch >From 397154f332b9390c3e9527993a99c22d72212322 Mon Sep 17 00:00:00 2001 From: Aaron Ecay Date: Fri, 16 Jan 2015 17:42:04 -0500 Subject: [PATCH 2/3] babel: Remove functions which are just indirection around org-babel-script-escape * lisp/ob-groovy.el (org-babel-groovy-table-or-string): Remove. (org-babel-groovy-evaluate): Call `org-babel-script-escape' instead. * lisp/ob-haskell.el (org-babel-haskell-table-or-string): Remove. (org-babel-execute:haskell): Call `org-babel-script-escape' instead. * lisp/ob-io.el (org-babel-io-table-or-string): Remove. (org-babel-io-evaluate):Call `org-babel-script-escape' instead. * lisp/ob-scala.el (org-babel-scala-table-or-string): Remove. (org-babel-scala-evaluate): Call `org-babel-script-escape' instead. --- lisp/ob-groovy.el | 10 +--------- lisp/ob-haskell.el | 8 +------- lisp/ob-io.el | 10 +--------- lisp/ob-scala.el | 10 +--------- 4 files changed, 4 insertions(+), 34 deletions(-) diff --git a/lisp/ob-groovy.el b/lisp/ob-groovy.el index 0068df9..8797ec9 100644 --- a/lisp/ob-groovy.el +++ b/lisp/ob-groovy.el @@ -66,14 +66,6 @@ called by `org-babel-execute-src-block'" (org-babel-pick-name (cdr (assoc :rowname-names params)) (cdr (assoc :rownames params)))))) - -(defun org-babel-groovy-table-or-string (results) - "Convert RESULTS into an appropriate elisp value. -If RESULTS look like a table, then convert them into an -Emacs-lisp table, otherwise return the results as a string." - (org-babel-script-escape results)) - - (defvar org-babel-groovy-wrapper-method "class Runner extends Script { @@ -106,7 +98,7 @@ in BODY as elisp." (concat org-babel-groovy-command " " src-file) ""))) (org-babel-result-cond result-params raw - (org-babel-groovy-table-or-string raw))))))) + (org-babel-script-escape raw))))))) (defun org-babel-prep-session:groovy (session params) diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el index 0006670..2e1d390 100644 --- a/lisp/ob-haskell.el +++ b/lisp/ob-haskell.el @@ -84,7 +84,7 @@ (output (mapconcat #'identity (reverse (cdr results)) "\n")) (value (car results))))) (org-babel-result-cond (cdr (assoc :result-params params)) - result (org-babel-haskell-table-or-string result))) + result (org-babel-script-escape result))) (org-babel-pick-name (cdr (assoc :colname-names params)) (cdr (assoc :colname-names params))) (org-babel-pick-name (cdr (assoc :rowname-names params)) @@ -133,12 +133,6 @@ then create one. Return the initialized session." (org-babel-haskell-var-to-haskell (cdr pair)))) (mapcar #'cdr (org-babel-get-header params :var)))) -(defun org-babel-haskell-table-or-string (results) - "Convert RESULTS to an Emacs-lisp table or string. -If RESULTS look like a table, then convert them into an -Emacs-lisp table, otherwise return the results as a string." - (org-babel-script-escape results)) - (defun org-babel-haskell-var-to-haskell (var) "Convert an elisp value VAR into a haskell variable. The elisp VAR is converted to a string of haskell source code diff --git a/lisp/ob-io.el b/lisp/ob-io.el index 971b37f..c309b88 100644 --- a/lisp/ob-io.el +++ b/lisp/ob-io.el @@ -62,14 +62,6 @@ called by `org-babel-execute-src-block'" (org-babel-pick-name (cdr (assoc :rowname-names params)) (cdr (assoc :rownames params)))))) - -(defun org-babel-io-table-or-string (results) - "Convert RESULTS into an appropriate elisp value. -If RESULTS look like a table, then convert them into an -Emacs-lisp table, otherwise return the results as a string." - (org-babel-script-escape results)) - - (defvar org-babel-io-wrapper-method "( %s @@ -98,7 +90,7 @@ in BODY as elisp." (concat org-babel-io-command " " src-file) ""))) (org-babel-result-cond result-params raw - (org-babel-io-table-or-string raw))))))) + (org-babel-script-escape raw))))))) (defun org-babel-prep-session:io (session params) diff --git a/lisp/ob-scala.el b/lisp/ob-scala.el index 0584342..838bc8f 100644 --- a/lisp/ob-scala.el +++ b/lisp/ob-scala.el @@ -60,14 +60,6 @@ called by `org-babel-execute-src-block'" (org-babel-pick-name (cdr (assoc :rowname-names params)) (cdr (assoc :rownames params)))))) - -(defun org-babel-scala-table-or-string (results) - "Convert RESULTS into an appropriate elisp value. -If RESULTS look like a table, then convert them into an -Emacs-lisp table, otherwise return the results as a string." - (org-babel-script-escape results)) - - (defvar org-babel-scala-wrapper-method "var str_result :String = null; @@ -104,7 +96,7 @@ in BODY as elisp." (concat org-babel-scala-command " " src-file) ""))) (org-babel-result-cond result-params raw - (org-babel-scala-table-or-string raw))))))) + (org-babel-script-escape raw))))))) (defun org-babel-prep-session:scala (session params) -- 2.2.2 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0003-babel-fix-up-org-babel-script-escape.patch >From f3d7530127a79352c2788632c79bf849f664db79 Mon Sep 17 00:00:00 2001 From: Aaron Ecay Date: Fri, 16 Jan 2015 17:47:32 -0500 Subject: [PATCH 3/3] babel: fix up org-babel-script-escape * lisp/ob-core.el (org-babel--script-escape-inner): New function. (org-babel-script-escape): Use it. * testing/lisp/test-ob.el (test-org-babel/script-escape): New test. --- lisp/ob-core.el | 140 ++++++++++++++++++++++++++++++++---------------- testing/lisp/test-ob.el | 85 +++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 47 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 892c3e3..024c76d 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -2669,60 +2669,106 @@ block but are passed literally to the \"example-block\"." (funcall nb-add (buffer-substring index (point-max)))) new-body)) +(defun org-babel--script-escape-inner (str) + (let (in-single in-double backslash out) + (mapc + (lambda (ch) + (setq + out + (if backslash + (progn + (setq backslash nil) + (cond + ((and in-single (eq ch ?')) + ;; Escaped single quote inside single quoted string: + ;; emit just a single quote, since we've changed the + ;; outer quotes to double. + (cons ch out)) + ((eq ch ?\") + ;; Escaped double quote + (if in-single + ;; This should be interpreted as backslash+quote, + ;; not an escape. Emit a three backslashes + ;; followed by a quote (because one layer of + ;; quoting will be stripped by `org-babel-read'). + (append (list ch ?\\ ?\\ ?\\) out) + ;; Otherwise we are in a double-quoted string. Emit + ;; a single escaped quote + (append (list ch ?\\) out))) + ((eq ch ?\\) + ;; Escaped backslash: emit a single escaped backslash + (append (list ?\\ ?\\) out)) + ;; Other: emit a quoted backslash followed by whatever + ;; the character was (because one layer of quoting will + ;; be stripped by `org-babel-read'). + (t (append (list ch ?\\ ?\\) out)))) + (case ch + (?\[ (if (or in-double in-single) + (cons ?\[ out) + (cons ?\( out))) + (?\] (if (or in-double in-single) + (cons ?\] out) + (cons ?\) out))) + (?\{ (if (or in-double in-single) + (cons ?\{ out) + (cons ?\( out))) + (?\} (if (or in-double in-single) + (cons ?\} out) + (cons ?\) out))) + (?, (if (or in-double in-single) + (cons ?, out) (cons ?\s out))) + (?\' (if in-double + (cons ?\' out) + (setq in-single (not in-single)) (cons ?\" out))) + (?\" (if in-single + (append (list ?\" ?\\) out) + (setq in-double (not in-double)) (cons ?\" out))) + (?\\ (unless (or in-single in-double) + (error "Can't handle backslash outside string in `org-babel-script-escape'")) + (setq backslash t) + out) + (t (cons ch out)))))) + (string-to-list str)) + (when (or in-single in-double) + (error "Unterminated string in `org-babel-script-escape'.")) + (apply #'string (reverse out)))) + (defun org-babel-script-escape (str &optional force) "Safely convert tables into elisp lists." + (unless (stringp str) + (error "`org-babel-script-escape' expects a string.")) (let ((escaped - (if (or force - (and (stringp str) - (> (length str) 2) - (or (and (string-equal "[" (substring str 0 1)) - (string-equal "]" (substring str -1))) - (and (string-equal "{" (substring str 0 1)) - (string-equal "}" (substring str -1))) - (and (string-equal "(" (substring str 0 1)) - (string-equal ")" (substring str -1)))))) - (org-babel-read - (concat - "'" - (let (in-single in-double out) - (mapc - (lambda (ch) - (setq - out - (case ch - (91 (if (or in-double in-single) ; [ - (cons 91 out) - (cons 40 out))) - (93 (if (or in-double in-single) ; ] - (cons 93 out) - (cons 41 out))) - (123 (if (or in-double in-single) ; { - (cons 123 out) - (cons 40 out))) - (125 (if (or in-double in-single) ; } - (cons 125 out) - (cons 41 out))) - (44 (if (or in-double in-single) ; , - (cons 44 out) (cons 32 out))) - (39 (if in-double ; ' - (cons 39 out) - (setq in-single (not in-single)) (cons 34 out))) - (34 (if in-single ; " - (append (list 34 32) out) - (setq in-double (not in-double)) (cons 34 out))) - (t (cons ch out))))) - (string-to-list str)) - (apply #'string (reverse out))))) - str))) + (cond + ((and (> (length str) 2) + (or (and (string-equal "[" (substring str 0 1)) + (string-equal "]" (substring str -1))) + (and (string-equal "{" (substring str 0 1)) + (string-equal "}" (substring str -1))) + (and (string-equal "(" (substring str 0 1)) + (string-equal ")" (substring str -1))))) + + (concat "'" (org-babel--script-escape-inner str))) + ((or force + (and (> (length str) 2) + (or (and (string-equal "'" (substring str 0 1)) + (string-equal "'" (substring str -1))) + ;; We need to pass double-quoted strings + ;; through the backslash-twiddling bits, even + ;; though we don't need to change their + ;; delimiters. + (and (string-equal "\"" (substring str 0 1)) + (string-equal "\"" (substring str -1)))))) + (org-babel--script-escape-inner str)) + (t str)))) (condition-case nil (org-babel-read escaped) (error escaped)))) (defun org-babel-read (cell &optional inhibit-lisp-eval) "Convert the string value of CELL to a number if appropriate. -Otherwise if cell looks like lisp (meaning it starts with a -\"(\", \"'\", \"`\" or a \"[\") then read it as lisp, -otherwise return it unmodified as a string. Optional argument -NO-LISP-EVAL inhibits lisp evaluation for situations in which is -it not appropriate." +Otherwise if CELL looks like lisp (meaning it starts with a +\"(\", \"'\", \"`\" or a \"[\") then read and evaluate it as +lisp, otherwise return it unmodified as a string. Optional +argument INHIBIT-LISP-EVAL inhibits lisp evaluation for +situations in which is it not appropriate." (if (and (stringp cell) (not (equal cell ""))) (or (org-babel-number-p cell) (if (and (not inhibit-lisp-eval) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index ce28435..3ff26de 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -1279,6 +1279,91 @@ echo \"$data\" (cdr (assq :file (nth 2 (org-babel-get-src-block-info t)))))) )) +(ert-deftest test-org-babel/script-escape () + ;; Delimited lists of numbers + (should (equal '(1 2 3) + (org-babel-script-escape "[1 2 3]"))) + (should (equal '(1 2 3) + (org-babel-script-escape "{1 2 3}"))) + (should (equal '(1 2 3) + (org-babel-script-escape "(1 2 3)"))) + ;; Delimited lists of double-quoted strings + (should (equal '("foo" "bar") + (org-babel-script-escape "(\"foo\" \"bar\")"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "[\"foo\" \"bar\"]"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "{\"foo\" \"bar\"}"))) + ;; ... with commas + (should (equal '("foo" "bar") + (org-babel-script-escape "(\"foo\", \"bar\")"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "[\"foo\", \"bar\"]"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "{\"foo\", \"bar\"}"))) + ;; Delimited lists of single-quoted strings + (should (equal '("foo" "bar") + (org-babel-script-escape "('foo' 'bar')"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "['foo' 'bar']"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "{'foo' 'bar'}"))) + ;; ... with commas + (should (equal '("foo" "bar") + (org-babel-script-escape "('foo', 'bar')"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "['foo', 'bar']"))) + (should (equal '("foo" "bar") + (org-babel-script-escape "{'foo', 'bar'}"))) + ;; Single quoted strings + (should (equal "foo" + (org-babel-script-escape "'foo'"))) + ;; ... with internal double quote + (should (equal "foo\"bar" + (org-babel-script-escape "'foo\"bar'"))) + ;; ... with internal backslash + (should (equal "foo\\bar" + (org-babel-script-escape "'foo\\bar'"))) + ;; ... with internal escaped backslash + (should (equal "foo\\bar" + (org-babel-script-escape "'foo\\\\bar'"))) + ;; ... with internal backslash-double quote + (should (equal "foo\\\"bar" + (org-babel-script-escape "'foo\\\"bar'"))) + ;; ... with internal escaped backslash-double quote + (should (equal "foo\\\"bar" + (org-babel-script-escape "'foo\\\\\"bar'"))) + ;; ... with internal escaped single quote + (should (equal "foo'bar" + (org-babel-script-escape "'foo\\'bar'"))) + ;; ... with internal escaped backslash-escaped single quote + (should (equal "foo\\'bar" + (org-babel-script-escape "'foo\\\\\\'bar'"))) + ;; Double quoted strings + (should (equal "foo" + (org-babel-script-escape "\"foo\""))) + ;; ... with internal single quote + (should (equal "foo'bar" + (org-babel-script-escape "\"foo'bar\""))) + ;; ... with internal backslash + (should (equal "foo\\bar" + (org-babel-script-escape "\"foo\\bar\""))) + ;; ... with internal escaped backslash + (should (equal "foo\\bar" + (org-babel-script-escape "\"foo\\\\bar\""))) + ;; ... with internal backslash-single quote + (should (equal "foo\\'bar" + (org-babel-script-escape "\"foo\\'bar\""))) + ;; ... with internal escaped backslash-single quote + (should (equal "foo\\'bar" + (org-babel-script-escape "\"foo\\\\'bar\""))) + ;; ... with internal escaped double quote + (should (equal "foo\"bar" + (org-babel-script-escape "\"foo\\\"bar\""))) + ;; ... with internal escaped backslash-escaped double quote + (should (equal "foo\\\"bar" + (org-babel-script-escape "\"foo\\\\\\\"bar\"")))) + (provide 'test-ob) ;;; test-ob ends here -- 2.2.2 --=-=-= Content-Type: text/plain Thanks, -- Aaron Ecay --=-=-=--