From 629788caa86773199fc06af1797cf8fd67a7884a Mon Sep 17 00:00:00 2001 From: Jack Kamm 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>