* [PATCH] ob-comint,R,python: Options for more robust non-async session output
@ 2024-10-18 21:20 Jack Kamm
2024-10-19 7:20 ` Ihor Radchenko
0 siblings, 1 reply; 5+ messages in thread
From: Jack Kamm @ 2024-10-18 21:20 UTC (permalink / raw)
To: emacs-orgmode; +Cc: yantar92, jeremiejuste
[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]
This is related to my recent thread,
https://list.orgmode.org/87wmi9etku.fsf@gmail.com/T/#m93bd964243638ee358d4375c9ed4f40e066238d4
which addresses problems with prompt filtering of R and python async evaluation in Org 9.7.
Here, I address the non-async case.
First, I introduce options to `org-babel-comint-with-output' to skip the
cleanup of prompts and dangling text. This allows individual Babel
languages to handle these tasks in a more robust, language-specific
manner, e.g. by sourcing temp files or relying on functionality from
external libraries like python.el or ESS.
In particular, as I discussed in the link above, it is very difficult to
do the prompt cleanup in a way that is robust to false positives. ob-shell
accomplishes this by changing the prompt to a unique string, but this is
not an option for other languages, and has its own drawbacks (e.g. with
conda). ob-python takes a different approach, and evaluates code in a
way that avoids leaking prompts, thereby skipping the need to clean them
up.
Previously, ob-python had eschewed using `org-babel-comint-with-output',
and re-implemented its own version without the cleanup steps. With this
patch, ob-python can go back to using `org-babel-comint-with-output',
thereby reducing code duplication.
Finally, I add a new implementation for R non-async output, that follows
the ob-python approach of avoiding prompt leakage altogether, thereby
skipping the need for a cleanup step. I add a unit test to demonstrate
the improved robustness of this approach: previously, this test would
fail due to a false positive prompt, but it passes with the new
implementation.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-options-to-skip-extra-processing-in-org-babel-co.patch --]
[-- Type: text/x-patch, Size: 11563 bytes --]
From 232f4c60323b13e3d46cb377095a8b2751f7ed74 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Thu, 17 Oct 2024 17:33:47 -0700
Subject: [PATCH] Add options to skip extra processing in
org-babel-comint-with-output
This patch adds options to org-babel-comint-with-output to skip prompt
removal and saving of dangling text. Allowing individual languages to
handle this cleanup can be more robust than relying on the generic
ob-comint implementation. This allows ob-python to switch back to
using `org-babel-comint-with-output' rather than its own bespoke
reimplementation, reducing code duplication. Furthermore, this adds a
new implementation of ob-R non-async session output evaluation, that
is similar to the ob-python approach in that it avoids leaking prompts
or interfering with dangling text altogether, rather than relying on
the cleanup from `org-babel-comint-with-output'. A test is added to
test-ob-R.el to demonstrate the improved robustness of the new
approach; previously, this test would fail due to a false positive
prompt, but now passes.
* lisp/ob-comint.el (org-babel-comint-with-output): Add new arguments
to prevent extra processing for prompt cleanup and saving dangling
output. Also, search for the end-of-execution sentinel within the
collected output rather than the comint buffer, so that point and
dangling text don't need to be modified.
* lisp/ob-python.el (org-babel-python-send-string): Switch to using
`org-babel-comint-with-output', rather than bespoke reimplementation.
* lisp/ob-R.el (ess-send-string): Declare external function.
(org-babel-R-evaluate-session): New implementation of output
evaluation that avoids leaking prompts, by writing the code block to a
tmp file and then sourcing it. Also uses `ess-send-string' (rather
than inserting code into the comint buffer) to avoid interfering with
dangling text.
* testing/lisp/test-ob-R.el (test-ob-r/session-output-with->-bol): New
test for robustness against false positive prompts at the beginning of
a line.
---
lisp/ob-R.el | 32 ++++++++-----------
lisp/ob-comint.el | 65 +++++++++++++++++++++++----------------
lisp/ob-python.el | 26 +++++-----------
testing/lisp/test-ob-R.el | 12 ++++++++
4 files changed, 69 insertions(+), 66 deletions(-)
diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 481212202..80c6b0e09 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -42,6 +42,8 @@ (declare-function ess-make-buffer-current "ext:ess-inf" ())
(declare-function ess-eval-buffer "ext:ess-inf" (vis))
(declare-function ess-wait-for-process "ext:ess-inf"
(&optional proc sec-prompt wait force-redisplay))
+(declare-function ess-send-string "ext:ess-inf"
+ (process string &optional visibly message type))
(defvar ess-current-process-name) ; ess-custom.el
(defvar ess-local-process-name) ; ess-custom.el
@@ -448,26 +450,16 @@ (defun org-babel-R-evaluate-session
(org-babel-import-elisp-from-file tmp-file '(16)))
column-names-p)))
(output
- (mapconcat
- 'org-babel-chomp
- (butlast
- (delq nil
- (mapcar
- (lambda (line) (when (> (length line) 0) line))
- (mapcar
- (lambda (line) ;; cleanup extra prompts left in output
- (if (string-match
- "^\\([>+.]\\([ ][>.+]\\)*[ ]\\)"
- (car (split-string line "\n")))
- (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"))))
+ (let ((tmp-src-file (org-babel-temp-file "R-")))
+ (with-temp-file tmp-src-file
+ (insert (concat
+ (org-babel-chomp body) "\n" org-babel-R-eoe-indicator)))
+ (with-current-buffer session
+ (org-babel-comint-with-output (session org-babel-R-eoe-output nil nil t t)
+ (ess-send-string (get-buffer-process (current-buffer))
+ (format "source('%s', echo=F, print.eval=T)"
+ (org-babel-process-file-name
+ tmp-src-file 'noquote)))))))))
(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 b88ac445a..3fbfbe0ec 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -105,11 +105,17 @@ (defmacro org-babel-comint-with-output (meta &rest body)
"Evaluate BODY in BUFFER and return process output.
Will wait until EOE-INDICATOR appears in the output, then return
all process output. If REMOVE-ECHO and FULL-BODY are present and
-non-nil, then strip echo'd body from the returned output. META
-should be a list containing the following where the last two
-elements are optional.
-
- (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY)
+non-nil, then strip echo'd body from the returned output. If
+NO-SAVE-DANGLE is nil, current text at the prompt is removed
+before evaluation, then restored; if non-nil, supress that
+behavior. If NO-CLEANUP-PROMPT is nil, prompts are detected in
+the output, and the returned value is a list of the output split
+on the prompt positions; if non-nil, suppress that behavior, and
+just return a single string of all the output up to
+EOE-INDICATOR. META should be a list containing the following
+where the last four elements are optional.
+
+ (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY NO-SAVE-DANGLE NO-CLEANUP-PROMPT)
This macro ensures that the filter is removed in case of an error
or user `keyboard-quit' during execution of body."
@@ -117,7 +123,9 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(let ((buffer (nth 0 meta))
(eoe-indicator (nth 1 meta))
(remove-echo (nth 2 meta))
- (full-body (nth 3 meta)))
+ (full-body (nth 3 meta))
+ (no-save-dangle (nth 5 meta))
+ (no-cleanup-prompt (nth 4 meta)))
`(org-babel-comint-in-buffer ,buffer
(let* ((string-buffer "")
(comint-output-filter-functions
@@ -125,23 +133,22 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(setq string-buffer (concat string-buffer text)))
comint-output-filter-functions))
dangling-text)
- ;; got located, and save dangling text
- (goto-char (process-mark (get-buffer-process (current-buffer))))
- (let ((start (point))
- (end (point-max)))
- (setq dangling-text (buffer-substring start end))
- (delete-region start end))
+ (unless ,no-save-dangle
+ ;; got located, and save dangling text
+ (goto-char (process-mark (get-buffer-process (current-buffer))))
+ (let ((start (point))
+ (end (point-max)))
+ (setq dangling-text (buffer-substring start end))
+ (delete-region start end)))
;; pass FULL-BODY to process
,@body
;; wait for end-of-evaluation indicator
(let ((start-time (current-time)))
- (while (progn
- (goto-char comint-last-input-end)
- (not (save-excursion
- (and (re-search-forward
- (regexp-quote ,eoe-indicator) nil t)
- (re-search-forward
- comint-prompt-regexp nil t)))))
+ (while (not (save-excursion
+ (and (string-match
+ (regexp-quote ,eoe-indicator) string-buffer)
+ (string-match
+ comint-prompt-regexp string-buffer))))
(accept-process-output
(get-buffer-process (current-buffer))
org-babel-comint-fallback-regexp-threshold)
@@ -152,21 +159,25 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(goto-char comint-last-input-end)
(save-excursion
(and
- (re-search-forward
- (regexp-quote ,eoe-indicator) nil t)
- (re-search-forward
- org-babel-comint-prompt-regexp-fallback nil t)))))
+ (string-match
+ (regexp-quote ,eoe-indicator) string-buffer)
+ (string-match
+ org-babel-comint-prompt-regexp-fallback string-buffer)))))
(org-babel-comint--set-fallback-prompt))))
;; replace cut dangling text
- (goto-char (process-mark (get-buffer-process (current-buffer))))
- (insert dangling-text)
+ (unless ,no-save-dangle
+ (goto-char (process-mark (get-buffer-process (current-buffer))))
+ (insert dangling-text))
;; remove echo'd FULL-BODY from input
(and ,remove-echo ,full-body
(setq string-buffer (org-babel-comint--echo-filter string-buffer ,full-body)))
- ;; Filter out prompts.
- (org-babel-comint--prompt-filter string-buffer)))))
+ (if ,no-cleanup-prompt
+ (save-match-data
+ (string-match (regexp-quote ,eoe-indicator) string-buffer)
+ (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))
+ (org-babel-comint--prompt-filter string-buffer))))))
(defun org-babel-comint-input-command (buffer cmd)
"Pass CMD to BUFFER.
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 8a3c24f70..82f7e4e8f 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -451,31 +451,19 @@ (defun org-babel-python-evaluate-external-process
(defun org-babel-python-send-string (session body)
"Pass BODY to the Python process in SESSION.
Return output."
- (with-current-buffer session
- (let* ((string-buffer "")
- (comint-output-filter-functions
- (cons (lambda (text) (setq string-buffer
- (concat string-buffer text)))
- comint-output-filter-functions))
- (body (format "\
+ (org-babel-comint-with-output
+ ((org-babel-session-buffer:python session)
+ org-babel-python-eoe-indicator
+ nil nil t t)
+ (python-shell-send-string (format "\
try:
%s
except:
raise
finally:
print('%s')"
- (org-babel-python--shift-right body 4)
- org-babel-python-eoe-indicator)))
- (let ((python-shell-buffer-name
- (org-babel-python-without-earmuffs session)))
- (python-shell-send-string body))
- ;; same as `python-shell-comint-end-of-output-p' in emacs-25.1+
- (while (not (and (python-shell-comint-end-of-output-p string-buffer)
- (string-match
- org-babel-python-eoe-indicator
- string-buffer)))
- (accept-process-output (get-buffer-process (current-buffer))))
- (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))))
+ (org-babel-python--shift-right body 4)
+ org-babel-python-eoe-indicator))))
(defun org-babel-python-evaluate-session
(session body &optional result-type result-params graphics-file)
diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index 0d291bf54..b8dcaa973 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -126,6 +126,18 @@ (ert-deftest test-ob-r/output-with-<> ()
))))
+(ert-deftest test-ob-r/session-output-with->-bol ()
+ "make sure prompt-like strings are well formatted, even when at beginning of line."
+ (let (ess-ask-for-ess-directory ess-history-file)
+ (should (string="abc
+def> <ghi"
+ (org-test-with-temp-text "#+begin_src R :results output :session R
+ cat(\"abc
+ def> <ghi\")
+ #+end_src
+"
+ (org-babel-execute-src-block))
+))))
;; (ert-deftest test-ob-r/output-with-error ()
--
2.46.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ob-comint,R,python: Options for more robust non-async session output
2024-10-18 21:20 [PATCH] ob-comint,R,python: Options for more robust non-async session output Jack Kamm
@ 2024-10-19 7:20 ` Ihor Radchenko
2024-10-21 5:45 ` Jack Kamm
0 siblings, 1 reply; 5+ messages in thread
From: Ihor Radchenko @ 2024-10-19 7:20 UTC (permalink / raw)
To: Jack Kamm; +Cc: emacs-orgmode, jeremiejuste
Jack Kamm <jackkamm@gmail.com> writes:
> This is related to my recent thread,
>
> https://list.orgmode.org/87wmi9etku.fsf@gmail.com/T/#m93bd964243638ee358d4375c9ed4f40e066238d4
>
> which addresses problems with prompt filtering of R and python async evaluation in Org 9.7.
>
> Here, I address the non-async case.
>
> First, I introduce options to `org-babel-comint-with-output' to skip the
> cleanup of prompts and dangling text. This allows individual Babel
> languages to handle these tasks in a more robust, language-specific
> manner, e.g. by sourcing temp files or relying on functionality from
> external libraries like python.el or ESS.
I have no objections to the idea.
Just one comment on the patch.
> - ;; got located, and save dangling text
> - (goto-char (process-mark (get-buffer-process (current-buffer))))
> - (let ((start (point))
> - (end (point-max)))
> - (setq dangling-text (buffer-substring start end))
> - (delete-region start end))
> + (unless ,no-save-dangle
> + ;; got located, and save dangling text
> + (goto-char (process-mark (get-buffer-process (current-buffer))))
> + (let ((start (point))
> + (end (point-max)))
> + (setq dangling-text (buffer-substring start end))
> + (delete-region start end)))
> ;; pass FULL-BODY to process
> ,@body
Previously, the point was always at process-mark when BODY is
called. Now, it may not be the case. It may cause problems.
--
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ob-comint,R,python: Options for more robust non-async session output
2024-10-19 7:20 ` Ihor Radchenko
@ 2024-10-21 5:45 ` Jack Kamm
2024-10-23 17:24 ` Ihor Radchenko
0 siblings, 1 reply; 5+ messages in thread
From: Jack Kamm @ 2024-10-21 5:45 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode, jeremiejuste
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]
Ihor Radchenko <yantar92@posteo.net> writes:
> Previously, the point was always at process-mark when BODY is
> called. Now, it may not be the case. It may cause problems.
On the one hand -- I'm not sure I agree that it would cause
problems. IMO setting point to process mark goes hand-in-hand with
cutting the dangling text -- so that text can be inserted into the
comint buffer at process mark. And the default behavior remains to cut
dangling text and move point to process mark, so nothing should change
for those languages that rely on this behavior.
OTOH -- I'm not sure the option to skip cutting dangling is really
necessary anymore. While the ob-python-style approach (which relies on
`process-send-string', or more specifically `python-shell-send-string')
doesn't require moving point or cutting the dangling text -- it doesn't
seem to affect anything either, since BODY is wrapped in
`save-excursion' (via `org-babel-comint-in-buffer').
In an earlier version of this patch I did see some weird behavior that I
thought was due to cutting the dangling text, but I can't reproduce it
anymore. So for now, I think it's better to just remove the
NO-SAVE-DANGLING option. If we find we need this option later for some
reason, we can figure out the details about point/process-mark then.
I've reattached the patch with this option removed, and also rebased
onto latest main.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-options-to-skip-extra-processing-in-org-babel-co.patch --]
[-- Type: text/x-patch, Size: 10970 bytes --]
From 629788caa86773199fc06af1797cf8fd67a7884a Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Thu, 17 Oct 2024 17:33:47 -0700
Subject: [PATCH] Add options to skip extra processing in
org-babel-comint-with-output
This patch adds options to org-babel-comint-with-output to skip prompt
removal. Allowing individual languages to handle this cleanup can be
more robust than relying on the generic ob-comint implementation.
This allows ob-python to switch back to using
`org-babel-comint-with-output' rather than its own bespoke
reimplementation, reducing code duplication. Furthermore, this adds a
new implementation of ob-R non-async session output evaluation, that
is similar to the ob-python approach in that it avoids leaking
prompts, rather than relying on the cleanup from
`org-babel-comint-with-output'. A test is added to test-ob-R.el to
demonstrate the improved robustness of the new approach; previously,
this test would fail due to a false positive prompt, but now passes.
* lisp/ob-comint.el (org-babel-comint-with-output): Add new arguments
to prevent extra processing for prompt cleanup. Also, search for the
end-of-execution sentinel within the collected output rather than the
comint buffer, which allows for evaluation via
`process-send-string' (or similar functions like
`python-shell-send-string' and `'ess-send-string') rather than
directly entering the code block into the comint buffer.
* lisp/ob-python.el (org-babel-python-send-string): Switch to using
`org-babel-comint-with-output', rather than bespoke reimplementation.
* lisp/ob-R.el (ess-send-string): Declare external function.
(org-babel-R-evaluate-session): New implementation of output
evaluation that avoids leaking prompts, by writing the code block to a
tmp file and then sourcing it.
* testing/lisp/test-ob-R.el (test-ob-r/session-output-with->-bol): New
test for robustness against false positive prompts at the beginning of
a line.
---
lisp/ob-R.el | 33 +++++++++++-----------------
lisp/ob-comint.el | 46 ++++++++++++++++++++++-----------------
lisp/ob-python.el | 26 ++++++----------------
testing/lisp/test-ob-R.el | 12 ++++++++++
4 files changed, 58 insertions(+), 59 deletions(-)
diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 481212202..fb29590e0 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -42,6 +42,8 @@ (declare-function ess-make-buffer-current "ext:ess-inf" ())
(declare-function ess-eval-buffer "ext:ess-inf" (vis))
(declare-function ess-wait-for-process "ext:ess-inf"
(&optional proc sec-prompt wait force-redisplay))
+(declare-function ess-send-string "ext:ess-inf"
+ (process string &optional visibly message type))
(defvar ess-current-process-name) ; ess-custom.el
(defvar ess-local-process-name) ; ess-custom.el
@@ -448,26 +450,17 @@ (defun org-babel-R-evaluate-session
(org-babel-import-elisp-from-file tmp-file '(16)))
column-names-p)))
(output
- (mapconcat
- 'org-babel-chomp
- (butlast
- (delq nil
- (mapcar
- (lambda (line) (when (> (length line) 0) line))
- (mapcar
- (lambda (line) ;; cleanup extra prompts left in output
- (if (string-match
- "^\\([>+.]\\([ ][>.+]\\)*[ ]\\)"
- (car (split-string line "\n")))
- (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"))))
+ (let ((tmp-src-file (org-babel-temp-file "R-")))
+ (with-temp-file tmp-src-file
+ (insert (concat
+ (org-babel-chomp body) "\n" org-babel-R-eoe-indicator)))
+ (with-current-buffer session
+ (org-babel-comint-with-output
+ (session org-babel-R-eoe-output nil nil t)
+ (ess-send-string (get-buffer-process (current-buffer))
+ (format "source('%s', echo=F, print.eval=T)"
+ (org-babel-process-file-name
+ tmp-src-file 'noquote)))))))))
(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 b88ac445a..a21c50ff1 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -105,11 +105,15 @@ (defmacro org-babel-comint-with-output (meta &rest body)
"Evaluate BODY in BUFFER and return process output.
Will wait until EOE-INDICATOR appears in the output, then return
all process output. If REMOVE-ECHO and FULL-BODY are present and
-non-nil, then strip echo'd body from the returned output. META
-should be a list containing the following where the last two
-elements are optional.
+non-nil, then strip echo'd body from the returned output. If
+NO-CLEANUP-PROMPT is nil, prompts are detected in the output, and
+the returned value is a list of the output split on the prompt
+positions; if non-nil, suppress that behavior, and just return a
+single string of all the output up to EOE-INDICATOR. META should
+be a list containing the following where the last three elements
+are optional.
- (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY)
+ (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY NO-CLEANUP-PROMPT)
This macro ensures that the filter is removed in case of an error
or user `keyboard-quit' during execution of body."
@@ -117,7 +121,8 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(let ((buffer (nth 0 meta))
(eoe-indicator (nth 1 meta))
(remove-echo (nth 2 meta))
- (full-body (nth 3 meta)))
+ (full-body (nth 3 meta))
+ (no-cleanup-prompt (nth 4 meta)))
`(org-babel-comint-in-buffer ,buffer
(let* ((string-buffer "")
(comint-output-filter-functions
@@ -125,7 +130,7 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(setq string-buffer (concat string-buffer text)))
comint-output-filter-functions))
dangling-text)
- ;; got located, and save dangling text
+ ;; got located, and save dangling text
(goto-char (process-mark (get-buffer-process (current-buffer))))
(let ((start (point))
(end (point-max)))
@@ -135,13 +140,11 @@ (defmacro org-babel-comint-with-output (meta &rest body)
,@body
;; wait for end-of-evaluation indicator
(let ((start-time (current-time)))
- (while (progn
- (goto-char comint-last-input-end)
- (not (save-excursion
- (and (re-search-forward
- (regexp-quote ,eoe-indicator) nil t)
- (re-search-forward
- comint-prompt-regexp nil t)))))
+ (while (not (save-excursion
+ (and (string-match
+ (regexp-quote ,eoe-indicator) string-buffer)
+ (string-match
+ comint-prompt-regexp string-buffer))))
(accept-process-output
(get-buffer-process (current-buffer))
org-babel-comint-fallback-regexp-threshold)
@@ -152,21 +155,24 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(goto-char comint-last-input-end)
(save-excursion
(and
- (re-search-forward
- (regexp-quote ,eoe-indicator) nil t)
- (re-search-forward
- org-babel-comint-prompt-regexp-fallback nil t)))))
+ (string-match
+ (regexp-quote ,eoe-indicator) string-buffer)
+ (string-match
+ org-babel-comint-prompt-regexp-fallback string-buffer)))))
(org-babel-comint--set-fallback-prompt))))
;; replace cut dangling text
- (goto-char (process-mark (get-buffer-process (current-buffer))))
+ (goto-char (process-mark (get-buffer-process (current-buffer))))
(insert dangling-text)
;; remove echo'd FULL-BODY from input
(and ,remove-echo ,full-body
(setq string-buffer (org-babel-comint--echo-filter string-buffer ,full-body)))
- ;; Filter out prompts.
- (org-babel-comint--prompt-filter string-buffer)))))
+ (if ,no-cleanup-prompt
+ (save-match-data
+ (string-match (regexp-quote ,eoe-indicator) string-buffer)
+ (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))
+ (org-babel-comint--prompt-filter string-buffer))))))
(defun org-babel-comint-input-command (buffer cmd)
"Pass CMD to BUFFER.
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 8a3c24f70..ceade40ee 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -451,31 +451,19 @@ (defun org-babel-python-evaluate-external-process
(defun org-babel-python-send-string (session body)
"Pass BODY to the Python process in SESSION.
Return output."
- (with-current-buffer session
- (let* ((string-buffer "")
- (comint-output-filter-functions
- (cons (lambda (text) (setq string-buffer
- (concat string-buffer text)))
- comint-output-filter-functions))
- (body (format "\
+ (org-babel-comint-with-output
+ ((org-babel-session-buffer:python session)
+ org-babel-python-eoe-indicator
+ nil nil t)
+ (python-shell-send-string (format "\
try:
%s
except:
raise
finally:
print('%s')"
- (org-babel-python--shift-right body 4)
- org-babel-python-eoe-indicator)))
- (let ((python-shell-buffer-name
- (org-babel-python-without-earmuffs session)))
- (python-shell-send-string body))
- ;; same as `python-shell-comint-end-of-output-p' in emacs-25.1+
- (while (not (and (python-shell-comint-end-of-output-p string-buffer)
- (string-match
- org-babel-python-eoe-indicator
- string-buffer)))
- (accept-process-output (get-buffer-process (current-buffer))))
- (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))))
+ (org-babel-python--shift-right body 4)
+ org-babel-python-eoe-indicator))))
(defun org-babel-python-evaluate-session
(session body &optional result-type result-params graphics-file)
diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index 0d291bf54..b8dcaa973 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -126,6 +126,18 @@ (ert-deftest test-ob-r/output-with-<> ()
))))
+(ert-deftest test-ob-r/session-output-with->-bol ()
+ "make sure prompt-like strings are well formatted, even when at beginning of line."
+ (let (ess-ask-for-ess-directory ess-history-file)
+ (should (string="abc
+def> <ghi"
+ (org-test-with-temp-text "#+begin_src R :results output :session R
+ cat(\"abc
+ def> <ghi\")
+ #+end_src
+"
+ (org-babel-execute-src-block))
+))))
;; (ert-deftest test-ob-r/output-with-error ()
--
2.46.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ob-comint,R,python: Options for more robust non-async session output
2024-10-21 5:45 ` Jack Kamm
@ 2024-10-23 17:24 ` Ihor Radchenko
2024-11-03 18:05 ` Jack Kamm
0 siblings, 1 reply; 5+ messages in thread
From: Ihor Radchenko @ 2024-10-23 17:24 UTC (permalink / raw)
To: Jack Kamm; +Cc: emacs-orgmode, jeremiejuste
Jack Kamm <jackkamm@gmail.com> writes:
> I've reattached the patch with this option removed, and also rebased
> onto latest main.
Thanks!
I think that we need to do the same things as with your
`org-babel-comint-async-register' patch: (1) ORG-NEWS; (2) possibly use
more descriptive values.
> - ;; Filter out prompts.
> - (org-babel-comint--prompt-filter string-buffer)))))
> + (if ,no-cleanup-prompt
> + (save-match-data
> + (string-match (regexp-quote ,eoe-indicator) string-buffer)
> + (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))
> + (org-babel-comint--prompt-filter string-buffer))))))
May you please explain this additional filtering of eoe-indicator here?
Why did it become necessary and why you did not change anything when
NO-CLEANUP-PROMPT is nil - `org-babel-comint--prompt-filter' does
nothing about eoe-endicator.
--
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ob-comint,R,python: Options for more robust non-async session output
2024-10-23 17:24 ` Ihor Radchenko
@ 2024-11-03 18:05 ` Jack Kamm
0 siblings, 0 replies; 5+ messages in thread
From: Jack Kamm @ 2024-11-03 18:05 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode, jeremiejuste
[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]
Ihor Radchenko <yantar92@posteo.net> writes:
> Thanks!
> I think that we need to do the same things as with your
> `org-babel-comint-async-register' patch: (1) ORG-NEWS; (2) possibly use
> more descriptive values.
I'm attaching an updated patch with NEWS entry and more descriptive
values for the prompt handling, consistent with the
`org-babel-comint-async-register' patch.
For the NEWS entry, I actually just modified the previous NEWS entry for
`org-babel-comint-async-register' since there is so much overlap. But
let me know if it would be better to have a separate NEWS entry for
each.
>> - ;; Filter out prompts.
>> - (org-babel-comint--prompt-filter string-buffer)))))
>> + (if ,no-cleanup-prompt
>> + (save-match-data
>> + (string-match (regexp-quote ,eoe-indicator) string-buffer)
>> + (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))
>> + (org-babel-comint--prompt-filter string-buffer))))))
>
> May you please explain this additional filtering of eoe-indicator here?
> Why did it become necessary and why you did not change anything when
> NO-CLEANUP-PROMPT is nil - `org-babel-comint--prompt-filter' does
> nothing about eoe-endicator.
Indeed `org-babel-comint--prompt-filter' does nothing about
eoe-indicator, leaving it to the language-specific processing (e.g. in
org-babel-execute:<LANG>) to clean it (e.g. with `butlast').
Since it can be confusing to have 2 different behaviors, I've updated
the `disable-prompt-filtering' case to be more consistent, and leave the
cleaning of the eoe-indicator to the language-specific implementations
in ob-python and ob-R.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-options-to-skip-extra-processing-in-org-babel-co.patch --]
[-- Type: text/x-patch, Size: 14796 bytes --]
From b96c5b9db5696e2e73c20d8f2ee056c0efa52580 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Thu, 17 Oct 2024 17:33:47 -0700
Subject: [PATCH] Add options to skip extra processing in
org-babel-comint-with-output
This patch adds options to org-babel-comint-with-output to skip prompt
removal. Allowing individual languages to handle this cleanup can be
more robust than relying on the generic ob-comint implementation.
This allows ob-python to switch back to using
`org-babel-comint-with-output' rather than its own bespoke
reimplementation, reducing code duplication. Furthermore, this adds a
new implementation of ob-R non-async session output evaluation, that
is similar to the ob-python approach in that it avoids leaking
prompts, rather than relying on the cleanup from
`org-babel-comint-with-output'. A test is added to test-ob-R.el to
demonstrate the improved robustness of the new approach; previously,
this test would fail due to a false positive prompt, but now passes.
* lisp/ob-comint.el (org-babel-comint--remove-prompts-p): New helper
function to parse the prompt-handling argument in
`org-babel-comint-with-output' and `org-babel-comint-async-register'.
(org-babel-comint-with-output): Add a new argument
to prevent extra processing for prompt cleanup. Also, search for the
end-of-execution sentinel within the collected output rather than the
comint buffer, which doesn't depend on the position of point during
evaluation.
(org-babel-comint-async-register): Move parsing of prompt-handling
argument to `org-babel-comint--remove-prompts-p'.
* lisp/ob-python.el: Require subr-x for Emacs 27 compatibility.
(org-babel-python-send-string): Switch to using
`org-babel-comint-with-output', rather than bespoke reimplementation.
* lisp/ob-R.el: Require subr-x for Emacs 27 compatibility.
(ess-send-string): Declare external function.
(org-babel-R-evaluate-session): New implementation of output
evaluation that avoids leaking prompts, by writing the code block to a
tmp file and then sourcing it.
* testing/lisp/test-ob-R.el (test-ob-r/session-output-with->-bol): New
test for robustness against false positive prompts at the beginning of
a line.
---
etc/ORG-NEWS | 12 ++++----
lisp/ob-R.el | 38 +++++++++++------------
lisp/ob-comint.el | 65 +++++++++++++++++++++++----------------
lisp/ob-python.el | 31 ++++++++-----------
testing/lisp/test-ob-R.el | 12 ++++++++
5 files changed, 87 insertions(+), 71 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index de4f11b25..92bfe3501 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -186,13 +186,13 @@ 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
+*** ob-comint: New optional arguments 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
+The new argument ~prompt-handling~ in ~org-babel-comint-with-output~
+and ~org-babel-comint-async-register~ allows Babel languages to
+specify how prompts should be handled in comint output. If equal to
+~filter-prompts~, prompts are removed from output before it is passed
+on to language-specific processing. 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.
diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 3214bf72c..1dbbd83f2 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -35,6 +35,8 @@ (org-assert-version)
(require 'cl-lib)
(require 'ob)
+(require 'subr-x) ; For `string-trim-right', Emacs < 28
+
(declare-function orgtbl-to-tsv "org-table" (table params))
(declare-function run-ess-r "ext:ess-r-mode" (&optional start-args))
(declare-function inferior-ess-send-input "ext:ess-inf" ())
@@ -42,6 +44,8 @@ (declare-function ess-make-buffer-current "ext:ess-inf" ())
(declare-function ess-eval-buffer "ext:ess-inf" (vis))
(declare-function ess-wait-for-process "ext:ess-inf"
(&optional proc sec-prompt wait force-redisplay))
+(declare-function ess-send-string "ext:ess-inf"
+ (process string &optional visibly message type))
(defvar ess-current-process-name) ; ess-custom.el
(defvar ess-local-process-name) ; ess-custom.el
@@ -448,26 +452,20 @@ (defun org-babel-R-evaluate-session
(org-babel-import-elisp-from-file tmp-file '(16)))
column-names-p)))
(output
- (mapconcat
- 'org-babel-chomp
- (butlast
- (delq nil
- (mapcar
- (lambda (line) (when (> (length line) 0) line))
- (mapcar
- (lambda (line) ;; cleanup extra prompts left in output
- (if (string-match
- "^\\([>+.]\\([ ][>.+]\\)*[ ]\\)"
- (car (split-string line "\n")))
- (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"))))
+ (let ((tmp-src-file (org-babel-temp-file "R-")))
+ (with-temp-file tmp-src-file
+ (insert (concat
+ (org-babel-chomp body) "\n" org-babel-R-eoe-indicator)))
+ (with-current-buffer session
+ (org-babel-chomp
+ (string-trim-right
+ (org-babel-comint-with-output
+ (session org-babel-R-eoe-output nil nil 'disable-prompt-filtering)
+ (ess-send-string (get-buffer-process (current-buffer))
+ (format "source('%s', echo=F, print.eval=T)"
+ (org-babel-process-file-name
+ tmp-src-file 'noquote))))
+ (rx (literal org-babel-R-eoe-output) (zero-or-more anychar)))))))))
(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 65f18fa80..2128b9638 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -101,15 +101,33 @@ (defun org-babel-comint--echo-filter (string &optional echo)
(setq string (substring string (match-end 0))))
string)
+(defun org-babel-comint--remove-prompts-p (prompt-handling)
+ "Helper to decide whether to remove prompts from comint output.
+Parses the symbol in PROMPT-HANDLING, which can be
+`filter-prompts', in which case prompts should be removed; or
+`disable-prompt-filtering', in which case prompt filtering is
+skipped. For backward-compatibility, the default value of `nil'
+is equivalent to `filter-prompts'."
+ (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)))))
+
(defmacro org-babel-comint-with-output (meta &rest body)
"Evaluate BODY in BUFFER and return process output.
Will wait until EOE-INDICATOR appears in the output, then return
all process output. If REMOVE-ECHO and FULL-BODY are present and
-non-nil, then strip echo'd body from the returned output. META
-should be a list containing the following where the last two
-elements are optional.
+non-nil, then strip echo'd body from the returned output.
+PROMPT-HANDLING may be either of the symbols `filter-prompts', in
+which case the output is split by `comint-prompt-regexp' and
+returned as a list; or, `disable-prompt-filtering', which
+suppresses this behavior and returns the full output as a string.
+META should be a list containing the following where the last
+three elements are optional.
- (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY)
+ (BUFFER EOE-INDICATOR REMOVE-ECHO FULL-BODY PROMPT-HANDLING)
This macro ensures that the filter is removed in case of an error
or user `keyboard-quit' during execution of body."
@@ -117,7 +135,8 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(let ((buffer (nth 0 meta))
(eoe-indicator (nth 1 meta))
(remove-echo (nth 2 meta))
- (full-body (nth 3 meta)))
+ (full-body (nth 3 meta))
+ (prompt-handling (nth 4 meta)))
`(org-babel-comint-in-buffer ,buffer
(let* ((string-buffer "")
(comint-output-filter-functions
@@ -125,7 +144,7 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(setq string-buffer (concat string-buffer text)))
comint-output-filter-functions))
dangling-text)
- ;; got located, and save dangling text
+ ;; got located, and save dangling text
(goto-char (process-mark (get-buffer-process (current-buffer))))
(let ((start (point))
(end (point-max)))
@@ -135,13 +154,11 @@ (defmacro org-babel-comint-with-output (meta &rest body)
,@body
;; wait for end-of-evaluation indicator
(let ((start-time (current-time)))
- (while (progn
- (goto-char comint-last-input-end)
- (not (save-excursion
- (and (re-search-forward
- (regexp-quote ,eoe-indicator) nil t)
- (re-search-forward
- comint-prompt-regexp nil t)))))
+ (while (not (save-excursion
+ (and (string-match
+ (regexp-quote ,eoe-indicator) string-buffer)
+ (string-match
+ comint-prompt-regexp string-buffer))))
(accept-process-output
(get-buffer-process (current-buffer))
org-babel-comint-fallback-regexp-threshold)
@@ -152,21 +169,22 @@ (defmacro org-babel-comint-with-output (meta &rest body)
(goto-char comint-last-input-end)
(save-excursion
(and
- (re-search-forward
- (regexp-quote ,eoe-indicator) nil t)
- (re-search-forward
- org-babel-comint-prompt-regexp-fallback nil t)))))
+ (string-match
+ (regexp-quote ,eoe-indicator) string-buffer)
+ (string-match
+ org-babel-comint-prompt-regexp-fallback string-buffer)))))
(org-babel-comint--set-fallback-prompt))))
;; replace cut dangling text
- (goto-char (process-mark (get-buffer-process (current-buffer))))
+ (goto-char (process-mark (get-buffer-process (current-buffer))))
(insert dangling-text)
;; remove echo'd FULL-BODY from input
(and ,remove-echo ,full-body
(setq string-buffer (org-babel-comint--echo-filter string-buffer ,full-body)))
- ;; Filter out prompts.
- (org-babel-comint--prompt-filter string-buffer)))))
+ (if (org-babel-comint--remove-prompts-p ,prompt-handling)
+ (org-babel-comint--prompt-filter string-buffer)
+ string-buffer)))))
(defun org-babel-comint-input-command (buffer cmd)
"Pass CMD to BUFFER.
@@ -379,12 +397,7 @@ (defun org-babel-comint-async-register
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)))))
+ (org-babel-comint--remove-prompts-p 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 9975e83be..f21cf2e65 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -36,6 +36,8 @@ (require 'ob)
(require 'org-macs)
(require 'python)
+(require 'subr-x) ; For `string-trim-right', Emacs < 28
+
(defvar org-babel-tangle-lang-exts)
(add-to-list 'org-babel-tangle-lang-exts '("python" . "py"))
@@ -451,31 +453,22 @@ (defun org-babel-python-evaluate-external-process
(defun org-babel-python-send-string (session body)
"Pass BODY to the Python process in SESSION.
Return output."
- (with-current-buffer session
- (let* ((string-buffer "")
- (comint-output-filter-functions
- (cons (lambda (text) (setq string-buffer
- (concat string-buffer text)))
- comint-output-filter-functions))
- (body (format "\
+ (org-babel-chomp
+ (string-trim-right
+ (org-babel-comint-with-output
+ ((org-babel-session-buffer:python session)
+ org-babel-python-eoe-indicator
+ nil nil 'disable-prompt-filtering)
+ (python-shell-send-string (format "\
try:
%s
except:
raise
finally:
print('%s')"
- (org-babel-python--shift-right body 4)
- org-babel-python-eoe-indicator)))
- (let ((python-shell-buffer-name
- (org-babel-python-without-earmuffs session)))
- (python-shell-send-string body))
- ;; same as `python-shell-comint-end-of-output-p' in emacs-25.1+
- (while (not (and (python-shell-comint-end-of-output-p string-buffer)
- (string-match
- org-babel-python-eoe-indicator
- string-buffer)))
- (accept-process-output (get-buffer-process (current-buffer))))
- (org-babel-chomp (substring string-buffer 0 (match-beginning 0))))))
+ (org-babel-python--shift-right body 4)
+ org-babel-python-eoe-indicator)))
+ (rx (literal org-babel-python-eoe-indicator) (zero-or-more anychar)))))
(defun org-babel-python-evaluate-session
(session body &optional result-type result-params graphics-file)
diff --git a/testing/lisp/test-ob-R.el b/testing/lisp/test-ob-R.el
index 2e171b2eb..cecdc7de8 100644
--- a/testing/lisp/test-ob-R.el
+++ b/testing/lisp/test-ob-R.el
@@ -126,6 +126,18 @@ (ert-deftest test-ob-r/output-with-<> ()
))))
+(ert-deftest test-ob-r/session-output-with->-bol ()
+ "make sure prompt-like strings are well formatted, even when at beginning of line."
+ (let (ess-ask-for-ess-directory ess-history-file)
+ (should (string="abc
+def> <ghi"
+ (org-test-with-temp-text "#+begin_src R :results output :session R
+ cat(\"abc
+ def> <ghi\")
+ #+end_src
+"
+ (org-babel-execute-src-block))
+))))
;; (ert-deftest test-ob-r/output-with-error ()
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-03 18:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-18 21:20 [PATCH] ob-comint,R,python: Options for more robust non-async session output Jack Kamm
2024-10-19 7:20 ` Ihor Radchenko
2024-10-21 5:45 ` Jack Kamm
2024-10-23 17:24 ` Ihor Radchenko
2024-11-03 18:05 ` Jack Kamm
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).