From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0.migadu.com ([2001:41d0:403:58f0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms13.migadu.com with LMTPS id 0OB9NW3REmdTLQAAqHPOHw:P1 (envelope-from ) for ; Fri, 18 Oct 2024 21:21:50 +0000 Received: from aspmx1.migadu.com ([2001:41d0:403:58f0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0.migadu.com with LMTPS id 0OB9NW3REmdTLQAAqHPOHw (envelope-from ) for ; Fri, 18 Oct 2024 23:21:50 +0200 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="SRCzW/j2"; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1729286509; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=P9wEm6zKnhnri5A2DbH3s809aG0rX2MguxazJ2Xq0So=; b=vAtc5Pd323/gJYhWYKWeBA+QxFhCrtWKfSKxNHJyhA6UA0t5d3KG+/7ypWgTAb6S6E0f+K MD8I5TM9I42bJ9B97iV9ZHk4QGxANpVuNZpogpCa+kw4iQbfbNK5Ygm7x9c3fZldyONhtA yR84LKDZuqzsxRJKqnGBeP5uO/6ott9rCZ9S88EiYWKs9LzfFq++LLN699RqPO752oZ2a/ Yc9b+Ph746ZZHaOlGtKFdglW3tbFAhnJ8v5OdjcEZG48IgRv3LQDQ33Wa/Qrdhg4eEf5A7 Dm2GKxE0v3JvIK6YQiPMdvamd+GUgsMo3FHln7Xlsq3V3Ds5IhEcObg76LGoHQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="SRCzW/j2"; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=key1; d=yhetil.org; t=1729286509; a=rsa-sha256; cv=none; b=j7IaLtSyq5OEj/MOeNeL8e4XD6jVKV9FR03/NSYzhpy/1HWDpUiYfq/danqeJt59W2w0yl CgMiRS+idJPplcnUdV31TpOLtG4jNFQ2KfkkgjSbH2lGCSSu8P9IqHBJvRW27/Vw5UAZ73 fRE7nquVck7d5xa8OlbbClLaLPdT9KmgVtb14G57izwvigYCumRDlBpbQXenX2XNcN9613 cYUuBUH8GhN/kWvs0LsBTYvcetXLxFIPd8uE2IIt6vNMbVswhWj/TR77ACZFtr390tiiyD zopfZ7BLyTxAczIWDOJI6Pj0yV44lveNiteO347Ne6Q+/jjdr9PaxxPT32VrvA== Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 6924E7F4CD for ; Fri, 18 Oct 2024 23:21:49 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t1uOz-000855-12; Fri, 18 Oct 2024 17:20:53 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t1uOy-00084x-Bx for emacs-orgmode@gnu.org; Fri, 18 Oct 2024 17:20:52 -0400 Received: from mail-pf1-x436.google.com ([2607:f8b0:4864:20::436]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1t1uOv-0006n9-Ve for emacs-orgmode@gnu.org; Fri, 18 Oct 2024 17:20:52 -0400 Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-71e4fa3ea7cso2092435b3a.0 for ; Fri, 18 Oct 2024 14:20:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729286447; x=1729891247; darn=gnu.org; h=mime-version:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=P9wEm6zKnhnri5A2DbH3s809aG0rX2MguxazJ2Xq0So=; b=SRCzW/j26D6bFp1Yzj6fLgfemJ1XyCZz9kACpciyi+Z41QbFz5+Bob9wMRtpLJ6ovl 764g89rQEDvf/SYmXdqhcKuq7jvYL4G0FnRE/DrS3uVtLwzhmlVSprZB391+ATwJOGvZ S1URKFsVVynrnS7KFRQRp6uDOVn3bHa80HEKSIPF8wsx7y7uqiRfP6SxSuzF9EKwAN4H TW4OGBdErfehYAQSz5QVKCUq6ALYNlrXLDiF87n9S7m+2Rzs1YvPI55AyjZ5EoNkivcX ECysSrUa6JYIJ9CtjlYfChYvW27mbKhK6vfDJJSsX3w4gZi9BfbnOYw5XIZM7D4yzmZg 601A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729286447; x=1729891247; h=mime-version:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=P9wEm6zKnhnri5A2DbH3s809aG0rX2MguxazJ2Xq0So=; b=qecG/f3GlmflRWphAP070u5S/c+nYzR9dZ4Gm3huisTr34b5TGfG05Lvs6BctowxBo QC5x3pQMM6A31FwyxvZKnlR+66/1nfOpw2hGuHiiGWa7eBD9IIuKPxi6N7jPXHNlS5o3 P4COTMhSWSb13ZdLYHcUm8ZOCZeJTZySzvQsMfpAYMhTfWmwPUMcrzrkUDOC6n48o4fh SFcNRxW57rbHNCQnUtwGrFILl8NkTnFlLPzjoi1K7Sx9l4ckim1madl+xD1vy5dxozCD lKr5j5Ij4TBadQZ67skY1thdx4QSazBzkaddQWFwQPgcQ4C4EVXfUvV6WzUy6KSNAsRh UJuQ== X-Gm-Message-State: AOJu0YyBPa5dA5epIW5NCsapgnoExIN9b59eB/lbaorRAkExcWfzcguA 3mm6SNbC9Qb3h8TUjm2ueOgDCCWnoH6GT9vsoVg+SxTYvAV64pVWOfaTCA== X-Google-Smtp-Source: AGHT+IHsP0EP9oEP8UG80x+Ro+nHQ5U8tG4lCOkwzIdA5SVA8dtqPQkArWIoaBdffLzGGiJSW7ZDVA== X-Received: by 2002:a05:6a00:2d1a:b0:71d:fb29:9f07 with SMTP id d2e1a72fcca58-71ea3212103mr5394186b3a.15.1729286447381; Fri, 18 Oct 2024 14:20:47 -0700 (PDT) Received: from localhost ([198.27.183.102]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71ea333eb18sm1927734b3a.70.2024.10.18.14.20.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Oct 2024 14:20:46 -0700 (PDT) From: Jack Kamm To: emacs-orgmode@gnu.org Cc: yantar92@posteo.net, jeremiejuste@gmail.com Subject: [PATCH] ob-comint,R,python: Options for more robust non-async session output Date: Fri, 18 Oct 2024 14:20:45 -0700 Message-ID: <87h6993y6q.fsf@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Received-SPF: pass client-ip=2607:f8b0:4864:20::436; envelope-from=jackkamm@gmail.com; helo=mail-pf1-x436.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: emacs-orgmode-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN X-Migadu-Scanner: mx11.migadu.com X-Migadu-Spam-Score: -1.57 X-Spam-Score: -1.57 X-Migadu-Queue-Id: 6924E7F4CD X-TUID: Oskc0+doIrxd --=-=-= Content-Type: text/plain 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. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Add-options-to-skip-extra-processing-in-org-babel-co.patch >From 232f4c60323b13e3d46cb377095a8b2751f7ed74 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 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>