[PATCH] Async sessions: Fix prompt removal regression in ob-R

[PATCH] Async sessions: Fix prompt removal regression in ob-R

[Jack Kamm]

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

Consider the following R block, which prints the occurrences of each
element in a list, including NAs:

#+begin_src R :session :results output :async
  table(c("ab","ab","c",NA,NA), useNA='always')
#+end_src

#+RESULTS:
:   ab    c <NA> 
:    2    1    2

Since Org 9.7, it instead prints:

#+RESULTS:
: ab    c <
: 2    1    2

The regression happens in commit:

e9c288dfaccc2960e5b6889e6aabea700ad4e05a

which made the prompt filtering more consistent between
`org-babel-comint-with-output' and `org-babel-comint-async-filter'.
However, it causes ob-R async session blocks to be over-aggressive in
removing the prompt.

Note that non-async ob-R blocks don't suffer from this problem,
because ob-R let-binds `comint-prompt-regexp' around
`org-babel-comint-with-output' (specifically, it adds a
beginning-of-line at the front of the regexp). However, I don't see a
good way to let-bind this around `org-babel-comint-async-filter'.

The best solution I could think of was to define a new buffer-local
variable, `org-babel-comint-prompt-regexp-override', which could be used
to override `comint-prompt-regexp' for the purpose of filtering. I
attach a patch with this solution.

Additionally, the regression causes causes misalignment of the output
due to removal of indentation. The fix for this is simpler, and
involves replacing a call of `org-trim' with `org-babel-chomp'.

I'm not sure if my patch is the best solution. But whatever solution
we arrive at, I would like to request that it be applied to bugfix
branch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-R-Fix-over-aggressive-async-prompt-removal.patch --]
[-- Type: text/x-patch, Size: 6494 bytes --]

From 11177e57f8a0c77b6c6541b852c5d105d70afec0 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sun, 22 Sep 2024 13:48:45 -0700
Subject: [PATCH] ob-R: Fix over-aggressive async prompt removal

* lisp/ob-comint.el (org-babel-comint-prompt-regexp-override): New
variable to override `comint-prompt-regexp' in
`org-babel-comint--prompt-filter'.
(org-babel-comint-async-filter):  Replace `org-trim' with
`org-babel-chomp' to avoid removing leading indentation.
* lisp/ob-R.el (org-babel-R-evaluate): Set
`org-babel-comint-regexp-override' in session evaluation.
(org-babel-R-evaluate-session): Remove let binding of
`comint-prompt-regexp', since `org-babel-comint-regexp-override' is
now set.
* testing/lisp/test-ob-R.el (test-ob-R/async-prompt-filter): Test for
over-aggressive prompt removal.
---
 lisp/ob-R.el              | 25 ++++++++++++++-----------
 lisp/ob-comint.el         | 18 +++++++++++++++---
 testing/lisp/test-ob-R.el | 28 ++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index de2d27a9a..a9a58d0e4 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -375,11 +375,15 @@ (defun org-babel-R-evaluate
     (session body result-type result-params column-names-p row-names-p async)
   "Evaluate R code in BODY."
   (if session
-      (if async
-          (ob-session-async-org-babel-R-evaluate-session
-           session body result-type column-names-p row-names-p)
-        (org-babel-R-evaluate-session
-         session body result-type result-params column-names-p row-names-p))
+      (progn
+        (with-current-buffer session
+          (setq org-babel-comint-prompt-regexp-override
+                (concat "^" comint-prompt-regexp)))
+        (if async
+            (ob-session-async-org-babel-R-evaluate-session
+             session body result-type column-names-p row-names-p)
+          (org-babel-R-evaluate-session
+           session body result-type result-params column-names-p row-names-p)))
     (org-babel-R-evaluate-external-process
      body result-type result-params column-names-p row-names-p)))
 
@@ -456,12 +460,11 @@ (defun org-babel-R-evaluate-session
 		     (substring line (match-end 1))
 		   line))
 	       (with-current-buffer session
-		 (let ((comint-prompt-regexp (concat "^" comint-prompt-regexp)))
-		   (org-babel-comint-with-output (session org-babel-R-eoe-output)
-		     (insert (mapconcat 'org-babel-chomp
-					(list body org-babel-R-eoe-indicator)
-					"\n"))
-		     (inferior-ess-send-input)))))))) "\n"))))
+		 (org-babel-comint-with-output (session org-babel-R-eoe-output)
+		   (insert (mapconcat 'org-babel-chomp
+				      (list body org-babel-R-eoe-indicator)
+				      "\n"))
+		   (inferior-ess-send-input))))))) "\n"))))
 
 (defun org-babel-R-process-value-result (result column-names-p)
   "R-specific processing of return value.
diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index 764927af7..7f1686035 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -75,11 +75,17 @@ (defun org-babel-comint--set-fallback-prompt ()
       (setq comint-prompt-regexp org-babel-comint-prompt-regexp-old
             org-babel-comint-prompt-regexp-old tmp))))
 
+(defvar-local org-babel-comint-prompt-regexp-override nil
+  "Overrides `comint-prompt-regexp' in `org-babel-comint--prompt-filter.'")
+
 (defun org-babel-comint--prompt-filter (string &optional prompt-regexp)
   "Remove PROMPT-REGEXP from STRING.
 
-PROMPT-REGEXP defaults to `comint-prompt-regexp'."
-  (let* ((prompt-regexp (or prompt-regexp comint-prompt-regexp))
+PROMPT-REGEXP defaults to `comint-prompt-regexp', which can be
+overridden with `org-babel-comint-prompt-regexp-override'."
+  (let* ((prompt-regexp (or prompt-regexp
+                            org-babel-comint-prompt-regexp-override
+                            comint-prompt-regexp))
          ;; We need newline in case if we do progressive replacement
          ;; of agglomerated comint prompts with `comint-prompt-regexp'
          ;; containing ^.
@@ -327,7 +333,13 @@ (defun org-babel-comint-async-filter (string)
 				 (equal (match-string 2) uuid))
 		      finally return (+ 1 (match-end 0)))))
                    ;; Remove prompt
-                   (res-promptless (org-trim (string-join (mapcar #'org-trim (org-babel-comint--prompt-filter res-str-raw)) "\n") "\n"))
+                   (res-promptless
+                    (org-trim (string-join
+                               (mapcar #'org-babel-chomp
+                                       (org-babel-comint--prompt-filter
+                                        res-str-raw))
+                               "\n")
+                              t))
 		   ;; Apply user callback
 		   (res-str (funcall org-babel-comint-async-chunk-callback res-promptless)))
 	      ;; Search for uuid in associated org-buffers to insert results
diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index 9ffbf3afd..05b91afd6 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -316,6 +316,34 @@ (org-test-with-temp-text-in-file
             (string= (concat text result)
                      (buffer-string)))))))
 
+(ert-deftest test-ob-R/async-prompt-filter ()
+  "Test that async evaluation doesn't remove spurious prompts and leading indentation."
+  (let* (ess-ask-for-ess-directory
+         ess-history-file
+         org-confirm-babel-evaluate
+         (session-name "*R:test-ob-R/session-async-results*")
+         (kill-buffer-query-functions nil)
+         (start-time (current-time))
+         (wait-time (time-add start-time 3))
+         uuid-placeholder)
+    (org-test-with-temp-text
+     (concat "#+begin_src R :session " session-name " :async t :results output
+table(c('ab','ab','c',NA,NA), useNA='always')
+#+end_src")
+     (setq uuid-placeholder (org-trim (org-babel-execute-src-block)))
+     (catch 'too-long
+       (while (string-match uuid-placeholder (buffer-string))
+         (progn
+           (sleep-for 0.01)
+           (when (time-less-p wait-time (current-time))
+             (throw 'too-long (ert-fail "Took too long to get result from callback"))))))
+     (search-forward "#+results")
+     (beginning-of-line 2)
+     (when (should (re-search-forward "\
+:\\([ ]+ab\\)[ ]+c[ ]+<NA>[ ]*
+:\\([ ]+2\\)[ ]+1[ ]+2"))
+       (should (equal (length (match-string 1)) (length (match-string 2))))
+       (kill-buffer session-name)))))
 
 (provide 'test-ob-R)
 
-- 
2.46.0

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> Consider the following R block, which prints the occurrences of each
> element in a list, including NAs:
>
> #+begin_src R :session :results output :async
>   table(c("ab","ab","c",NA,NA), useNA='always')
> #+end_src
>
> #+RESULTS:
> :   ab    c <NA> 
> :    2    1    2
>
> Since Org 9.7, it instead prints:
>
> #+RESULTS:
> : ab    c <
> : 2    1    2
> ...
> The best solution I could think of was to define a new buffer-local
> variable, `org-babel-comint-prompt-regexp-override', which could be used
> to override `comint-prompt-regexp' for the purpose of filtering. I
> attach a patch with this solution.

Maybe we can simply override `comint-prompt-regexp' as we do in
ob-shell? The default regexp seems to be too permissive.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Jack Kamm]

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

Ihor Radchenko <yantar92@posteo.net> writes:

> Maybe we can simply override `comint-prompt-regexp' as we do in
> ob-shell? The default regexp seems to be too permissive.

I don't think this is a good idea, since this is a deliberate choice by
ESS, which contains explicit commentary that the regexp should not
contain BOL, because in some cases multiple prompts can end up on the
same line [1][2].

After some more thought, rather than overriding `comint-prompt-regexp',
I think it would be better to provide a mechanism to altogether prevent
the prompt removal in `org-babel-comint-async-filter'. There is no need
to remove prompts from Python and R async session evaluation, since I
wrote them in a way that avoids leaking any prompts in the output. And
in both cases, it is easy to come up with examples that work fine in Org
9.6 but get mangled in Org 9.7.

Therefore, I've attached an updated patch that provides such a mechanism
for ob-R and ob-python, reverting them to the Org 9.6 behavior. This is
done through a variable `org-babel-comint-async-remove-prompts-p', which
is set by an optional argument in `org-babel-comint-async-register'.

More generally, I think it is best to avoid doing the prompt removal
when possible, since it is difficult (impossible?) to do it perfectly,
and it can cause many problems. This is why ob-python avoids using
`org-babel-comint-with-output' -- it sources a tmp file rather than
inputting code directly to comint, so that prompts do not leak. I think
non-async R evaluation would benefit from a similar approach, and plan
to propose a patch to make ob-R non-async eval more similar to
ob-python.

[1] https://github.com/emacs-ess/ESS/blob/d60c13a6a347ea7a91ea3408bb464cff0ab4fef6/lisp/ess-r-mode.el#L2538
[2] https://github.com/emacs-ess/ESS/blob/d60c13a6a347ea7a91ea3408bb464cff0ab4fef6/lisp/ess-custom.el#L1829


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Disable-async-prompt-removal-in-ob-R-python.patch --]
[-- Type: text/x-patch, Size: 7059 bytes --]

From 5c9d6f28f14c51fc542c997ed6aa6792e59857c6 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sun, 22 Sep 2024 13:48:45 -0700
Subject: [PATCH] Disable async prompt removal in ob-R,python

* lisp/ob-comint.el (org-babel-comint-async-remove-prompts-p): New
variable to disable prompt removal in async output.
(org-babel-comint-async-filter): Check
`org-babel-comint-async-remove-prompts-p' before calling
`org-babel-comint--prompt-filter'.
(org-babel-comint-async-register): Added argument for whether prompts
should be removed from async output.
* lisp/ob-python.el (org-babel-python-async-evaluate-session): Set
option to inhibit prompt removal when registering async evaluators.
* lisp/ob-R.el (ob-session-async-org-babel-R-evaluate-session): Set
option to inhibit prompt removal when registering async evaluators.
* testing/lisp/test-ob-R.el (test-ob-R/async-prompt-filter): Test for
over-aggressive prompt removal.
---
 lisp/ob-R.el              |  3 ++-
 lisp/ob-comint.el         | 27 ++++++++++++++++++++-------
 lisp/ob-python.el         |  3 ++-
 testing/lisp/test-ob-R.el | 28 ++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index de2d27a9a..08d8227f0 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -486,7 +486,8 @@ (defun ob-session-async-org-babel-R-evaluate-session
    session (current-buffer)
    "^\\(?:[>.+] \\)*\\[1\\] \"ob_comint_async_R_\\(start\\|end\\|file\\)_\\(.+\\)\"$"
    'org-babel-chomp
-   'ob-session-async-R-value-callback)
+   'ob-session-async-R-value-callback
+   t)
   (cl-case result-type
     (value
      (let ((tmp-file (org-babel-temp-file "R-")))
diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index 764927af7..f37aa5264 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -239,6 +239,9 @@ (defvar-local org-babel-comint-async-chunk-callback nil
 comint process.  It should return a string that will be passed
 to `org-babel-insert-result'.")
 
+(defvar-local org-babel-comint-async-remove-prompts-p t
+  "Whether prompts should be detected and removed from async output.")
+
 (defvar-local org-babel-comint-async-dangling nil
   "Dangling piece of the last process output, as a string.
 Used when `org-babel-comint-async-indicator' is spread across multiple
@@ -326,10 +329,16 @@ (defun org-babel-comint-async-filter (string)
 		      until (and (equal (match-string 1) "start")
 				 (equal (match-string 2) uuid))
 		      finally return (+ 1 (match-end 0)))))
-                   ;; Remove prompt
-                   (res-promptless (org-trim (string-join (mapcar #'org-trim (org-babel-comint--prompt-filter res-str-raw)) "\n") "\n"))
 		   ;; Apply user callback
-		   (res-str (funcall org-babel-comint-async-chunk-callback res-promptless)))
+		   (res-str (funcall org-babel-comint-async-chunk-callback
+                                     (if org-babel-comint-async-remove-prompts-p
+                                         (org-trim (string-join
+                                                    (mapcar #'org-trim
+                                                            (org-babel-comint--prompt-filter
+                                                             res-str-raw))
+                                                    "\n")
+                                                   t)
+                                       res-str-raw))))
 	      ;; Search for uuid in associated org-buffers to insert results
 	      (cl-loop for buf in org-buffers
 		       until (with-current-buffer buf
@@ -350,18 +359,22 @@ (defun org-babel-comint-async-filter (string)
 
 (defun org-babel-comint-async-register
     (session-buffer org-buffer indicator-regexp
-		    chunk-callback file-callback)
+		    chunk-callback file-callback
+                    &optional inhibit-prompt-removal)
   "Set local org-babel-comint-async variables in SESSION-BUFFER.
 ORG-BUFFER is added to `org-babel-comint-async-buffers' if not
 present.  `org-babel-comint-async-indicator',
 `org-babel-comint-async-chunk-callback', and
 `org-babel-comint-async-file-callback' are set to
-INDICATOR-REGEXP, CHUNK-CALLBACK, and FILE-CALLBACK
-respectively."
+INDICATOR-REGEXP, CHUNK-CALLBACK, and FILE-CALLBACK respectively.
+If INHIBIT-PROMPT-REMOVAL,
+`org-babel-comint-async-remove-prompts-p' is set to `nil' to
+prevent prompt detection and removal from async output."
   (org-babel-comint-in-buffer session-buffer
     (setq org-babel-comint-async-indicator indicator-regexp
 	  org-babel-comint-async-chunk-callback chunk-callback
-	  org-babel-comint-async-file-callback file-callback)
+	  org-babel-comint-async-file-callback file-callback
+          org-babel-comint-async-remove-prompts-p (not inhibit-prompt-removal))
     (unless (memq org-buffer org-babel-comint-async-buffers)
       (setq org-babel-comint-async-buffers
 	    (cons org-buffer org-babel-comint-async-buffers)))
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 8a3c24f70..38ebe9147 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -538,7 +538,8 @@ (defun org-babel-python-async-evaluate-session
   (org-babel-comint-async-register
    session (current-buffer)
    "ob_comint_async_python_\\(start\\|end\\|file\\)_\\(.+\\)"
-   'org-babel-chomp 'org-babel-python-async-value-callback)
+   'org-babel-chomp 'org-babel-python-async-value-callback
+   t)
   (pcase result-type
     (`output
      (let ((uuid (org-id-uuid)))
diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index 9ffbf3afd..05b91afd6 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -316,6 +316,34 @@ (org-test-with-temp-text-in-file
             (string= (concat text result)
                      (buffer-string)))))))
 
+(ert-deftest test-ob-R/async-prompt-filter ()
+  "Test that async evaluation doesn't remove spurious prompts and leading indentation."
+  (let* (ess-ask-for-ess-directory
+         ess-history-file
+         org-confirm-babel-evaluate
+         (session-name "*R:test-ob-R/session-async-results*")
+         (kill-buffer-query-functions nil)
+         (start-time (current-time))
+         (wait-time (time-add start-time 3))
+         uuid-placeholder)
+    (org-test-with-temp-text
+     (concat "#+begin_src R :session " session-name " :async t :results output
+table(c('ab','ab','c',NA,NA), useNA='always')
+#+end_src")
+     (setq uuid-placeholder (org-trim (org-babel-execute-src-block)))
+     (catch 'too-long
+       (while (string-match uuid-placeholder (buffer-string))
+         (progn
+           (sleep-for 0.01)
+           (when (time-less-p wait-time (current-time))
+             (throw 'too-long (ert-fail "Took too long to get result from callback"))))))
+     (search-forward "#+results")
+     (beginning-of-line 2)
+     (when (should (re-search-forward "\
+:\\([ ]+ab\\)[ ]+c[ ]+<NA>[ ]*
+:\\([ ]+2\\)[ ]+1[ ]+2"))
+       (should (equal (length (match-string 1)) (length (match-string 2))))
+       (kill-buffer session-name)))))
 
 (provide 'test-ob-R)
 
-- 
2.47.0

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> ...
> Therefore, I've attached an updated patch that provides such a mechanism
> for ob-R and ob-python, reverting them to the Org 9.6 behavior. This is
> done through a variable `org-babel-comint-async-remove-prompts-p', which
> is set by an optional argument in `org-babel-comint-async-register'.

Looks reasonable in general.

> More generally, I think it is best to avoid doing the prompt removal
> when possible, since it is difficult (impossible?) to do it perfectly,
> and it can cause many problems. This is why ob-python avoids using
> `org-babel-comint-with-output' -- it sources a tmp file rather than
> inputting code directly to comint, so that prompts do not leak. I think
> non-async R evaluation would benefit from a similar approach, and plan
> to propose a patch to make ob-R non-async eval more similar to
> ob-python.

Agree. The prompt removal code is there simply because comint has no
reliable facilities to distinguish between prompts, input, and output.
If we can avoid it, we should.

I recall the annoying issues with ob-ruby, where the REPL spits prompts
asynchronously, making prompt filtering very hard (impossible?).
https://list.orgmode.org/orgmode/875y4vv8ie.fsf@localhost/

I have one small nipick comment on the patch:

>    (org-babel-comint-async-register
>     session (current-buffer)
>     "ob_comint_async_python_\\(start\\|end\\|file\\)_\\(.+\\)"
> -   'org-babel-chomp 'org-babel-python-async-value-callback)
> +   'org-babel-chomp 'org-babel-python-async-value-callback
> +   t)

Rather than `t', I'd use something more descriptive like 'disable-prompt-filtering.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Jack Kamm]

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

Ihor Radchenko <yantar92@posteo.net> writes:

> I have one small nipick comment on the patch:
>
>>    (org-babel-comint-async-register
>>     session (current-buffer)
>>     "ob_comint_async_python_\\(start\\|end\\|file\\)_\\(.+\\)"
>> -   'org-babel-chomp 'org-babel-python-async-value-callback)
>> +   'org-babel-chomp 'org-babel-python-async-value-callback
>> +   t)
>
> Rather than `t', I'd use something more descriptive like 'disable-prompt-filtering.

Is the attached patch what you had in mind? If so I will squash it with
the previous patch.

You can also see both commits together in my branch here:
https://github.com/jackkamm/org-mode/tree/2024-ob-r-async-prompt-bugfix2

> Looks reasonable in general.

Thanks, look forward to applying it.

But first -- note that the current patch is on top of bugfix. I had
mentioned this in my original email but want to double check if it's
OK. In particular, I'm not sure if it's acceptable for bugfix branch
anymore, now that I'm changing the function signature of
`org-babel-comint-async-register' (albeit in a backward-compatible way).

I had originally proposed bugfix since I use R's table() function a lot
(as in my original example), and it was causing me problems when I
belatedly upgraded my work machine to Org 9.7.  But if you prefer, I can
rebase onto main.

Another possibility would be to add a hardcoded check on bugfix to skip
the prompt filtering if the major-mode is R or Python; then on main,
revert the hard-coded check, and update the signature of
`org-babel-comint-async-register' to set it properly with a
variable. But not sure it's worth the hassle -- I might just switch to
using main branch on my work machine at that point.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-to-be-squashed-with-previous-commit.patch --]
[-- Type: text/x-patch, Size: 3486 bytes --]

From f155c878ffa9f7244bdae417b824ffa6c4d92742 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sat, 19 Oct 2024 23:46:03 -0700
Subject: [PATCH 2/2] to be squashed with previous commit

---
 lisp/ob-R.el      |  2 +-
 lisp/ob-comint.el | 21 +++++++++++++++------
 lisp/ob-python.el |  2 +-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 08d8227f0..5a8dfe22c 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -487,7 +487,7 @@ (defun ob-session-async-org-babel-R-evaluate-session
    "^\\(?:[>.+] \\)*\\[1\\] \"ob_comint_async_R_\\(start\\|end\\|file\\)_\\(.+\\)\"$"
    'org-babel-chomp
    'ob-session-async-R-value-callback
-   t)
+   'disable-prompt-filtering)
   (cl-case result-type
     (value
      (let ((tmp-file (org-babel-temp-file "R-")))
diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index f37aa5264..ea42eaccd 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -360,21 +360,30 @@ (defun org-babel-comint-async-filter (string)
 (defun org-babel-comint-async-register
     (session-buffer org-buffer indicator-regexp
 		    chunk-callback file-callback
-                    &optional inhibit-prompt-removal)
+                    &optional prompt-handling)
   "Set local org-babel-comint-async variables in SESSION-BUFFER.
 ORG-BUFFER is added to `org-babel-comint-async-buffers' if not
 present.  `org-babel-comint-async-indicator',
 `org-babel-comint-async-chunk-callback', and
 `org-babel-comint-async-file-callback' are set to
 INDICATOR-REGEXP, CHUNK-CALLBACK, and FILE-CALLBACK respectively.
-If INHIBIT-PROMPT-REMOVAL,
-`org-babel-comint-async-remove-prompts-p' is set to `nil' to
-prevent prompt detection and removal from async output."
+PROMPT-HANDLING may be either of the symbols `filter-prompts', in
+which case prompts matching `comint-prompt-regexp' are filtered
+from output before it is passed to CHUNK-CALLBACK, or
+`disable-prompt-filtering', in which case this behavior is
+disabled.  For backward-compatibility, the default value of `nil'
+is equivalent to `filter-prompts'."
   (org-babel-comint-in-buffer session-buffer
     (setq org-babel-comint-async-indicator indicator-regexp
 	  org-babel-comint-async-chunk-callback chunk-callback
-	  org-babel-comint-async-file-callback file-callback
-          org-babel-comint-async-remove-prompts-p (not inhibit-prompt-removal))
+	  org-babel-comint-async-file-callback file-callback)
+    (setq org-babel-comint-async-remove-prompts-p
+          (let ((prompt-handling (or prompt-handling 'filter-prompts)))
+            (cond
+             ((eq prompt-handling 'disable-prompt-filtering) nil)
+             ((eq prompt-handling 'filter-prompts) t)
+             (t (error (format "Unrecognized prompt handling behavior %s"
+                               (symbol-name prompt-handling)))))))
     (unless (memq org-buffer org-babel-comint-async-buffers)
       (setq org-babel-comint-async-buffers
 	    (cons org-buffer org-babel-comint-async-buffers)))
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 38ebe9147..f41f44dbd 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -539,7 +539,7 @@ (defun org-babel-python-async-evaluate-session
    session (current-buffer)
    "ob_comint_async_python_\\(start\\|end\\|file\\)_\\(.+\\)"
    'org-babel-chomp 'org-babel-python-async-value-callback
-   t)
+   'disable-prompt-filtering)
   (pcase result-type
     (`output
      (let ((uuid (org-id-uuid)))
-- 
2.46.2

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

>> Rather than `t', I'd use something more descriptive like 'disable-prompt-filtering.
>
> Is the attached patch what you had in mind? If so I will squash it with
> the previous patch.

Your variant is even better than what I had in mind.

> But first -- note that the current patch is on top of bugfix. I had
> mentioned this in my original email but want to double check if it's
> OK. In particular, I'm not sure if it's acceptable for bugfix branch
> anymore, now that I'm changing the function signature of
> `org-babel-comint-async-register' (albeit in a backward-compatible way).
>
> I had originally proposed bugfix since I use R's table() function a lot
> (as in my original example), and it was causing me problems when I
> belatedly upgraded my work machine to Org 9.7.  But if you prefer, I can
> rebase onto main.
>
> Another possibility would be to add a hardcoded check on bugfix to skip
> the prompt filtering if the major-mode is R or Python; then on main,
> revert the hard-coded check, and update the signature of
> `org-babel-comint-async-register' to set it properly with a
> variable. But not sure it's worth the hassle -- I might just switch to
> using main branch on my work machine at that point.

I think that it is ok for bugfix as the patch essentially reverses the
commit that introduced the regression for ob-R and ob-python. (The
original patch was fixing a problem with ob-shell). So, that patch is
fairly trivial.

We might want to document the signature change in ORG-NEWS on main
though, as an additional patch for main.

For reference, here is the official policy on bugfix branch:
https://orgmode.org/worg/org-maintenance.html#release-types

> +    (setq org-babel-comint-async-remove-prompts-p
> +          (let ((prompt-handling (or prompt-handling 'filter-prompts)))
> +            (cond
> +             ((eq prompt-handling 'disable-prompt-filtering) nil)
> +             ((eq prompt-handling 'filter-prompts) t)
> +             (t (error (format "Unrecognized prompt handling behavior %s"
> +                               (symbol-name prompt-handling)))))))

nitpick: I'd rather add (eq prompt-handling nil) as a cond clause, to
make it more explicit.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Jack Kamm]

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

Ihor Radchenko <yantar92@posteo.net> writes:

> I think that it is ok for bugfix as the patch essentially reverses the
> commit that introduced the regression for ob-R and ob-python. (The
> original patch was fixing a problem with ob-shell). So, that patch is
> fairly trivial.
>
> We might want to document the signature change in ORG-NEWS on main
> though, as an additional patch for main.
>
> nitpick: I'd rather add (eq prompt-handling nil) as a cond clause, to
> make it more explicit.

Great, thanks. I'm attaching an updated patch with the more explicit
cond clause, and rebased onto latest bugfix.  I also attach a second
patch with the NEWS entry on main.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Disable-async-prompt-removal-in-ob-R-python.patch --]
[-- Type: text/x-patch, Size: 7539 bytes --]

From 000b7ea1025a7f076c0af0b69d6a2a653c415b40 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sun, 22 Sep 2024 13:48:45 -0700
Subject: [PATCH] Disable async prompt removal in ob-R,python

* lisp/ob-comint.el (org-babel-comint-async-remove-prompts-p): New
variable to disable prompt removal in async output.
(org-babel-comint-async-filter): Check
`org-babel-comint-async-remove-prompts-p' before calling
`org-babel-comint--prompt-filter'.
(org-babel-comint-async-register): Added argument for whether prompts
should be removed from async output.
* lisp/ob-python.el (org-babel-python-async-evaluate-session): Set
option to inhibit prompt removal when registering async evaluators.
* lisp/ob-R.el (ob-session-async-org-babel-R-evaluate-session): Set
option to inhibit prompt removal when registering async evaluators.
* testing/lisp/test-ob-R.el (test-ob-R/async-prompt-filter): Test for
over-aggressive prompt removal.
---
 lisp/ob-R.el              |  3 ++-
 lisp/ob-comint.el         | 34 ++++++++++++++++++++++++++++------
 lisp/ob-python.el         |  3 ++-
 testing/lisp/test-ob-R.el | 28 ++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index de2d27a9a..5a8dfe22c 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -486,7 +486,8 @@ (defun ob-session-async-org-babel-R-evaluate-session
    session (current-buffer)
    "^\\(?:[>.+] \\)*\\[1\\] \"ob_comint_async_R_\\(start\\|end\\|file\\)_\\(.+\\)\"$"
    'org-babel-chomp
-   'ob-session-async-R-value-callback)
+   'ob-session-async-R-value-callback
+   'disable-prompt-filtering)
   (cl-case result-type
     (value
      (let ((tmp-file (org-babel-temp-file "R-")))
diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index 764927af7..efec7badc 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -239,6 +239,9 @@ (defvar-local org-babel-comint-async-chunk-callback nil
 comint process.  It should return a string that will be passed
 to `org-babel-insert-result'.")
 
+(defvar-local org-babel-comint-async-remove-prompts-p t
+  "Whether prompts should be detected and removed from async output.")
+
 (defvar-local org-babel-comint-async-dangling nil
   "Dangling piece of the last process output, as a string.
 Used when `org-babel-comint-async-indicator' is spread across multiple
@@ -326,10 +329,16 @@ (defun org-babel-comint-async-filter (string)
 		      until (and (equal (match-string 1) "start")
 				 (equal (match-string 2) uuid))
 		      finally return (+ 1 (match-end 0)))))
-                   ;; Remove prompt
-                   (res-promptless (org-trim (string-join (mapcar #'org-trim (org-babel-comint--prompt-filter res-str-raw)) "\n") "\n"))
 		   ;; Apply user callback
-		   (res-str (funcall org-babel-comint-async-chunk-callback res-promptless)))
+		   (res-str (funcall org-babel-comint-async-chunk-callback
+                                     (if org-babel-comint-async-remove-prompts-p
+                                         (org-trim (string-join
+                                                    (mapcar #'org-trim
+                                                            (org-babel-comint--prompt-filter
+                                                             res-str-raw))
+                                                    "\n")
+                                                   t)
+                                       res-str-raw))))
 	      ;; Search for uuid in associated org-buffers to insert results
 	      (cl-loop for buf in org-buffers
 		       until (with-current-buffer buf
@@ -350,18 +359,31 @@ (defun org-babel-comint-async-filter (string)
 
 (defun org-babel-comint-async-register
     (session-buffer org-buffer indicator-regexp
-		    chunk-callback file-callback)
+		    chunk-callback file-callback
+                    &optional prompt-handling)
   "Set local org-babel-comint-async variables in SESSION-BUFFER.
 ORG-BUFFER is added to `org-babel-comint-async-buffers' if not
 present.  `org-babel-comint-async-indicator',
 `org-babel-comint-async-chunk-callback', and
 `org-babel-comint-async-file-callback' are set to
-INDICATOR-REGEXP, CHUNK-CALLBACK, and FILE-CALLBACK
-respectively."
+INDICATOR-REGEXP, CHUNK-CALLBACK, and FILE-CALLBACK respectively.
+PROMPT-HANDLING may be either of the symbols `filter-prompts', in
+which case prompts matching `comint-prompt-regexp' are filtered
+from output before it is passed to CHUNK-CALLBACK, or
+`disable-prompt-filtering', in which case this behavior is
+disabled.  For backward-compatibility, the default value of `nil'
+is equivalent to `filter-prompts'."
   (org-babel-comint-in-buffer session-buffer
     (setq org-babel-comint-async-indicator indicator-regexp
 	  org-babel-comint-async-chunk-callback chunk-callback
 	  org-babel-comint-async-file-callback file-callback)
+    (setq org-babel-comint-async-remove-prompts-p
+          (cond
+           ((eq prompt-handling 'disable-prompt-filtering) nil)
+           ((eq prompt-handling 'filter-prompts) t)
+           ((eq prompt-handling nil) t)
+           (t (error (format "Unrecognized prompt handling behavior %s"
+                             (symbol-name prompt-handling))))))
     (unless (memq org-buffer org-babel-comint-async-buffers)
       (setq org-babel-comint-async-buffers
 	    (cons org-buffer org-babel-comint-async-buffers)))
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 8a3c24f70..f41f44dbd 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -538,7 +538,8 @@ (defun org-babel-python-async-evaluate-session
   (org-babel-comint-async-register
    session (current-buffer)
    "ob_comint_async_python_\\(start\\|end\\|file\\)_\\(.+\\)"
-   'org-babel-chomp 'org-babel-python-async-value-callback)
+   'org-babel-chomp 'org-babel-python-async-value-callback
+   'disable-prompt-filtering)
   (pcase result-type
     (`output
      (let ((uuid (org-id-uuid)))
diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index 9ffbf3afd..05b91afd6 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -316,6 +316,34 @@ (org-test-with-temp-text-in-file
             (string= (concat text result)
                      (buffer-string)))))))
 
+(ert-deftest test-ob-R/async-prompt-filter ()
+  "Test that async evaluation doesn't remove spurious prompts and leading indentation."
+  (let* (ess-ask-for-ess-directory
+         ess-history-file
+         org-confirm-babel-evaluate
+         (session-name "*R:test-ob-R/session-async-results*")
+         (kill-buffer-query-functions nil)
+         (start-time (current-time))
+         (wait-time (time-add start-time 3))
+         uuid-placeholder)
+    (org-test-with-temp-text
+     (concat "#+begin_src R :session " session-name " :async t :results output
+table(c('ab','ab','c',NA,NA), useNA='always')
+#+end_src")
+     (setq uuid-placeholder (org-trim (org-babel-execute-src-block)))
+     (catch 'too-long
+       (while (string-match uuid-placeholder (buffer-string))
+         (progn
+           (sleep-for 0.01)
+           (when (time-less-p wait-time (current-time))
+             (throw 'too-long (ert-fail "Took too long to get result from callback"))))))
+     (search-forward "#+results")
+     (beginning-of-line 2)
+     (when (should (re-search-forward "\
+:\\([ ]+ab\\)[ ]+c[ ]+<NA>[ ]*
+:\\([ ]+2\\)[ ]+1[ ]+2"))
+       (should (equal (length (match-string 1)) (length (match-string 2))))
+       (kill-buffer session-name)))))
 
 (provide 'test-ob-R)
 
-- 
2.46.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-NEWS-entry-for-the-new-argument-of-org-babel-comint-.patch --]
[-- Type: text/x-patch, Size: 1870 bytes --]

From ab249b7f518115f8dfd85191a64656f8f40ab58e Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Mon, 21 Oct 2024 20:22:00 -0700
Subject: [PATCH] NEWS entry for the new argument of
 org-babel-comint-async-register

The optional argument was added on bugfix branch, but we are adding
this extra NEWS entry on main.  See also:

https://list.orgmode.org/875xpnrubg.fsf@localhost/T/#m179d313e1db284ff28eb4098129bb49418824a25
---
 etc/ORG-NEWS | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 5d421172f..17ffc4068 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -186,6 +186,24 @@ accept the INFO channel and return a string.  This makes it possible
 to dynamically generate the content of the resulting ~<head>~ tag in
 the resulting HTML document.
 
+*** ~org-babel-comint-async-register~: Added argument to specify prompt handling
+
+The new argument ~prompt-handling~ allows Babel languages to specify
+how prompts should be handled when passing output to
+~org-babel-comint-async-chunk-callback~.  If equal to
+~filter-prompts~, prompts are removed beforehand, similar to the
+default behavior of ~org-babel-comint-with-output~.  If equal to
+~disable-prompt-filtering~, then the prompt filtering is skipped.  If
+unset, then the default behavior is the same as ~filter-prompts~ for
+backwards compatibility.
+
+Prompt filtering is needed for some Babel languages, such as ob-shell,
+which leave extra prompts in the output as a side effect of
+evaluation.  However other Babel languages, like ob-python, don't
+leave extra prompts after evaluation, and skipping the prompt
+filtering can be more robust for such languages (as this avoids
+removing false positive prompts).
+
 ** Miscellaneous
 *** Org mode no longer prevents =flyspell= from spell-checking inside =LOGBOOK= drawers
 
-- 
2.46.2

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> Great, thanks. I'm attaching an updated patch with the more explicit
> cond clause, and rebased onto latest bugfix.  I also attach a second
> patch with the NEWS entry on main.

Thanks!

> +           (t (error (format "Unrecognized prompt handling behavior %s"
> +                             (symbol-name prompt-handling))))))

I think that `symbol-name' is unnecessary here.
It will lead to cryptic error if PROMPT-HANDLING happens to be something
that is not symbol (for example, a number)

> +*** ~org-babel-comint-async-register~: Added argument to specify prompt handling

Maybe "New optional argument controlling prompt handling"

> +The new argument ~prompt-handling~ allows Babel languages to specify
> +how prompts should be handled when passing output to
> +~org-babel-comint-async-chunk-callback~.  If equal to
> +~filter-prompts~, prompts are removed beforehand, similar to the

similar -> same (or did I miss something?)

> +default behavior of ~org-babel-comint-with-output~.  If equal to
> +~disable-prompt-filtering~, then the prompt filtering is skipped.  If
> +unset, then the default behavior is the same as ~filter-prompts~ for
> +backwards compatibility.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Jack Kamm]

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

Ihor Radchenko <yantar92@posteo.net> writes:

>> +           (t (error (format "Unrecognized prompt handling behavior %s"
>> +                             (symbol-name prompt-handling))))))
>
> I think that `symbol-name' is unnecessary here.
> It will lead to cryptic error if PROMPT-HANDLING happens to be something
> that is not symbol (for example, a number)
>
>> +*** ~org-babel-comint-async-register~: Added argument to specify prompt handling
>
> Maybe "New optional argument controlling prompt handling"
>
>> +The new argument ~prompt-handling~ allows Babel languages to specify
>> +how prompts should be handled when passing output to
>> +~org-babel-comint-async-chunk-callback~.  If equal to
>> +~filter-prompts~, prompts are removed beforehand, similar to the
>
> similar -> same (or did I miss something?)

Thanks for the suggestions, all of which I agree with. I've updated the
patches accordingly, and also rebased them to most recent versions of
bugfix/main.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Disable-async-prompt-removal-in-ob-R-python.patch --]
[-- Type: text/x-patch, Size: 7525 bytes --]

From 16dfc0d4500ab941333a9e705b2ddac85c30efdc Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sun, 22 Sep 2024 13:48:45 -0700
Subject: [PATCH] Disable async prompt removal in ob-R,python

* lisp/ob-comint.el (org-babel-comint-async-remove-prompts-p): New
variable to disable prompt removal in async output.
(org-babel-comint-async-filter): Check
`org-babel-comint-async-remove-prompts-p' before calling
`org-babel-comint--prompt-filter'.
(org-babel-comint-async-register): Added argument for whether prompts
should be removed from async output.
* lisp/ob-python.el (org-babel-python-async-evaluate-session): Set
option to inhibit prompt removal when registering async evaluators.
* lisp/ob-R.el (ob-session-async-org-babel-R-evaluate-session): Set
option to inhibit prompt removal when registering async evaluators.
* testing/lisp/test-ob-R.el (test-ob-R/async-prompt-filter): Test for
over-aggressive prompt removal.
---
 lisp/ob-R.el              |  3 ++-
 lisp/ob-comint.el         | 34 ++++++++++++++++++++++++++++------
 lisp/ob-python.el         |  3 ++-
 testing/lisp/test-ob-R.el | 28 ++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 8074496f8..e9a1ce768 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -486,7 +486,8 @@ (defun ob-session-async-org-babel-R-evaluate-session
    session (current-buffer)
    "^\\(?:[>.+] \\)*\\[1\\] \"ob_comint_async_R_\\(start\\|end\\|file\\)_\\(.+\\)\"$"
    'org-babel-chomp
-   'ob-session-async-R-value-callback)
+   'ob-session-async-R-value-callback
+   'disable-prompt-filtering)
   (cl-case result-type
     (value
      (let ((tmp-file (org-babel-temp-file "R-")))
diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index 764927af7..be5b41331 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -239,6 +239,9 @@ (defvar-local org-babel-comint-async-chunk-callback nil
 comint process.  It should return a string that will be passed
 to `org-babel-insert-result'.")
 
+(defvar-local org-babel-comint-async-remove-prompts-p t
+  "Whether prompts should be detected and removed from async output.")
+
 (defvar-local org-babel-comint-async-dangling nil
   "Dangling piece of the last process output, as a string.
 Used when `org-babel-comint-async-indicator' is spread across multiple
@@ -326,10 +329,16 @@ (defun org-babel-comint-async-filter (string)
 		      until (and (equal (match-string 1) "start")
 				 (equal (match-string 2) uuid))
 		      finally return (+ 1 (match-end 0)))))
-                   ;; Remove prompt
-                   (res-promptless (org-trim (string-join (mapcar #'org-trim (org-babel-comint--prompt-filter res-str-raw)) "\n") "\n"))
 		   ;; Apply user callback
-		   (res-str (funcall org-babel-comint-async-chunk-callback res-promptless)))
+		   (res-str (funcall org-babel-comint-async-chunk-callback
+                                     (if org-babel-comint-async-remove-prompts-p
+                                         (org-trim (string-join
+                                                    (mapcar #'org-trim
+                                                            (org-babel-comint--prompt-filter
+                                                             res-str-raw))
+                                                    "\n")
+                                                   t)
+                                       res-str-raw))))
 	      ;; Search for uuid in associated org-buffers to insert results
 	      (cl-loop for buf in org-buffers
 		       until (with-current-buffer buf
@@ -350,18 +359,31 @@ (defun org-babel-comint-async-filter (string)
 
 (defun org-babel-comint-async-register
     (session-buffer org-buffer indicator-regexp
-		    chunk-callback file-callback)
+		    chunk-callback file-callback
+                    &optional prompt-handling)
   "Set local org-babel-comint-async variables in SESSION-BUFFER.
 ORG-BUFFER is added to `org-babel-comint-async-buffers' if not
 present.  `org-babel-comint-async-indicator',
 `org-babel-comint-async-chunk-callback', and
 `org-babel-comint-async-file-callback' are set to
-INDICATOR-REGEXP, CHUNK-CALLBACK, and FILE-CALLBACK
-respectively."
+INDICATOR-REGEXP, CHUNK-CALLBACK, and FILE-CALLBACK respectively.
+PROMPT-HANDLING may be either of the symbols `filter-prompts', in
+which case prompts matching `comint-prompt-regexp' are filtered
+from output before it is passed to CHUNK-CALLBACK, or
+`disable-prompt-filtering', in which case this behavior is
+disabled.  For backward-compatibility, the default value of `nil'
+is equivalent to `filter-prompts'."
   (org-babel-comint-in-buffer session-buffer
     (setq org-babel-comint-async-indicator indicator-regexp
 	  org-babel-comint-async-chunk-callback chunk-callback
 	  org-babel-comint-async-file-callback file-callback)
+    (setq org-babel-comint-async-remove-prompts-p
+          (cond
+           ((eq prompt-handling 'disable-prompt-filtering) nil)
+           ((eq prompt-handling 'filter-prompts) t)
+           ((eq prompt-handling nil) t)
+           (t (error (format "Unrecognized prompt handling behavior %s"
+                             prompt-handling)))))
     (unless (memq org-buffer org-babel-comint-async-buffers)
       (setq org-babel-comint-async-buffers
 	    (cons org-buffer org-babel-comint-async-buffers)))
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index f881918c7..9975e83be 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -538,7 +538,8 @@ (defun org-babel-python-async-evaluate-session
   (org-babel-comint-async-register
    session (current-buffer)
    "ob_comint_async_python_\\(start\\|end\\|file\\)_\\(.+\\)"
-   'org-babel-chomp 'org-babel-python-async-value-callback)
+   'org-babel-chomp 'org-babel-python-async-value-callback
+   'disable-prompt-filtering)
   (pcase result-type
     (`output
      (let ((uuid (org-id-uuid)))
diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index 9ffbf3afd..05b91afd6 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -316,6 +316,34 @@ (org-test-with-temp-text-in-file
             (string= (concat text result)
                      (buffer-string)))))))
 
+(ert-deftest test-ob-R/async-prompt-filter ()
+  "Test that async evaluation doesn't remove spurious prompts and leading indentation."
+  (let* (ess-ask-for-ess-directory
+         ess-history-file
+         org-confirm-babel-evaluate
+         (session-name "*R:test-ob-R/session-async-results*")
+         (kill-buffer-query-functions nil)
+         (start-time (current-time))
+         (wait-time (time-add start-time 3))
+         uuid-placeholder)
+    (org-test-with-temp-text
+     (concat "#+begin_src R :session " session-name " :async t :results output
+table(c('ab','ab','c',NA,NA), useNA='always')
+#+end_src")
+     (setq uuid-placeholder (org-trim (org-babel-execute-src-block)))
+     (catch 'too-long
+       (while (string-match uuid-placeholder (buffer-string))
+         (progn
+           (sleep-for 0.01)
+           (when (time-less-p wait-time (current-time))
+             (throw 'too-long (ert-fail "Took too long to get result from callback"))))))
+     (search-forward "#+results")
+     (beginning-of-line 2)
+     (when (should (re-search-forward "\
+:\\([ ]+ab\\)[ ]+c[ ]+<NA>[ ]*
+:\\([ ]+2\\)[ ]+1[ ]+2"))
+       (should (equal (length (match-string 1)) (length (match-string 2))))
+       (kill-buffer session-name)))))
 
 (provide 'test-ob-R)
 
-- 
2.46.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-NEWS-entry-for-the-new-argument-of-org-babel-comint-.patch --]
[-- Type: text/x-patch, Size: 1875 bytes --]

From a4e66d0f66e04983d75d93ac84acf96d6e41d870 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Mon, 21 Oct 2024 20:22:00 -0700
Subject: [PATCH] NEWS entry for the new argument of
 org-babel-comint-async-register

The optional argument was added on bugfix branch, but we are adding
this extra NEWS entry on main.  See also:

https://list.orgmode.org/875xpnrubg.fsf@localhost/T/#m179d313e1db284ff28eb4098129bb49418824a25
---
 etc/ORG-NEWS | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 5d421172f..de4f11b25 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -186,6 +186,24 @@ accept the INFO channel and return a string.  This makes it possible
 to dynamically generate the content of the resulting ~<head>~ tag in
 the resulting HTML document.
 
+*** ~org-babel-comint-async-register~: New optional argument controlling prompt handling
+
+The new argument ~prompt-handling~ allows Babel languages to specify
+how prompts should be handled when passing output to
+~org-babel-comint-async-chunk-callback~.  If equal to
+~filter-prompts~, prompts are removed beforehand, same as the
+default behavior of ~org-babel-comint-with-output~.  If equal to
+~disable-prompt-filtering~, then the prompt filtering is skipped.  If
+unset, then the default behavior is the same as ~filter-prompts~ for
+backwards compatibility.
+
+Prompt filtering is needed for some Babel languages, such as ob-shell,
+which leave extra prompts in the output as a side effect of
+evaluation.  However other Babel languages, like ob-python, don't
+leave extra prompts after evaluation, and skipping the prompt
+filtering can be more robust for such languages (as this avoids
+removing false positive prompts).
+
 ** Miscellaneous
 *** Org mode no longer prevents =flyspell= from spell-checking inside =LOGBOOK= drawers
 
-- 
2.46.2

Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R

[Ihor Radchenko]

Jack Kamm <jackkamm@gmail.com> writes:

> Thanks for the suggestions, all of which I agree with. I've updated the
> patches accordingly, and also rebased them to most recent versions of
> bugfix/main.

Thanks!
I have no more comments. LGTM.
Feel free to push.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>