Re: Bug: ob-python mangles multiline :var values [9.3.6 (release_9.3.6-397-ga089600)]

[Jack Kamm <jackkamm@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

Hi,

Here's my second attempt to fix the issue. I'll wait a few days for
comments before merging, to try and avoid causing a bug like last time.

It turns out the problem was deeper than just the :var argument,
multiline strings in the body were also getting mangled. For example,
this block had the same problem as the original report:

#+begin_src python
  text = """a
  b
  c
  """
  return text
#+end_src

#+RESULTS:
: a
: 	b
: 	c

My fix this time was to use functions from python.el to indent, and to
detect whether we are in a string and shouldn't be indented.

I also added a couple more unit tests, one for multiline strings, and
one for the variable scope/assignment issue that Matt reported.

Cheers,
Jack


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-python.el-Fix-multiline-strings-in-non-session-re.patch --]
[-- Type: text/x-patch, Size: 2904 bytes --]

From 179178d39f6216172e1a070f570cf941f99b1a89 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sat, 6 Jun 2020 10:59:23 -0700
Subject: [PATCH] ob-python.el: Fix multiline strings in non-session :results
 value

* lisp/ob-python.el (org-babel-python-evaluate-external-process): Use
functions from python.el to indent lines, avoiding multiline strings.
* testing/lisp/test-ob-python.el (test-ob-python/multiline-var): Set
test as expected to succeed.
(test-ob-python/multiline-str): Add test for multiline string in body.
(test-ob-python/header-var-assignment): Test that :var is in correct
scope and can be assigned to.

cf. https://orgmode.org/list/87tv009l9a.fsf@gmail.com/#t
---
 lisp/ob-python.el              | 14 +++++++++-----
 testing/lisp/test-ob-python.el | 20 +++++++++++++++++++-
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index dbcfac08d..622f69ce3 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -296,11 +296,15 @@ (defun org-babel-python-evaluate-external-process
 			(if (member "pp" result-params)
 			    org-babel-python-pp-wrapper-method
 			  org-babel-python-wrapper-method)
-			(mapconcat
-			 (lambda (line) (format "\t%s" line))
-			 (split-string (org-remove-indentation (org-trim body))
-				       "[\r\n]")
-			 "\n")
+			(with-temp-buffer
+			 (insert body)
+			 (goto-char (point-min))
+			 (while (< (point) (point-max))
+			   (unless (python-syntax-context 'string)
+			     (python-indent-shift-right (line-beginning-position)
+							(line-end-position)))
+			   (forward-line 1))
+			 (buffer-string))
 			(org-babel-process-file-name tmp-file 'noquote))))
 		     (org-babel-eval-read-file tmp-file))))))
     (org-babel-result-cond result-params
diff --git a/testing/lisp/test-ob-python.el b/testing/lisp/test-ob-python.el
index 763942b16..e3f0571a1 100644
--- a/testing/lisp/test-ob-python.el
+++ b/testing/lisp/test-ob-python.el
@@ -174,7 +174,6 @@ (ert-deftest test-ob-python/assign-underscore ()
 	    (org-babel-execute-src-block)))))
 
 (ert-deftest test-ob-python/multiline-var ()
-  :expected-result :failed
   (should
    (equal "a\nb\nc"
 	  (org-test-with-temp-text "#+begin_src python :var text=\"a\\nb\\nc\"
@@ -182,6 +181,25 @@ (ert-deftest test-ob-python/multiline-var ()
 #+end_src"
 	    (org-babel-execute-src-block)))))
 
+(ert-deftest test-ob-python/multiline-str ()
+  (should
+   (equal "a\nb\nc"
+	  (org-test-with-temp-text "#+begin_src python
+text=\"a\\nb\\nc\"
+return text
+#+end_src"
+	    (org-babel-execute-src-block)))))
+
+(ert-deftest test-ob-python/header-var-assignment ()
+  (should
+   (equal "success"
+	  (org-test-with-temp-text "#+begin_src python :var text=\"failure\"
+text
+text=\"success\"
+return text
+#+end_src"
+	    (org-babel-execute-src-block)))))
+
 (provide 'test-ob-python)
 
 ;;; test-ob-python.el ends here
-- 
2.27.0