From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1.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 yCMVMQ8UDmcOFgAA62LTzQ:P1 (envelope-from ) for ; Tue, 15 Oct 2024 07:04:47 +0000 Received: from aspmx1.migadu.com ([2001:41d0:403:58f0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1.migadu.com with LMTPS id yCMVMQ8UDmcOFgAA62LTzQ (envelope-from ) for ; Tue, 15 Oct 2024 09:04:47 +0200 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ASCVVsiJ; 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=1728975887; 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:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=MB9qpGi+NrZoduvEs8tD7Bq9Id3tyfW0yePFUJf4x1o=; b=EjnTjQIcMxEEZoMFI0BkRLdj+tCpnicpkdNc4q5N2Dome4KN8gKnuZHqxGpuKIQ605/IYE 6/GwnJeOyMQ80GijTg0BpE4oWNY1axM8mzED80Ce+ecWj/HcC6i7D/93MvpYHv9Ix7K+ZE Ke5uPOJIyf8V2UnTPH+7MNYSgA9vt7KvtpdSAWrbGHmWmZoXAZvSj4+YqTrdCjcp/6fYxM TTMxnYENVVn4JInNF7297Kkbx0UxwOa/nH7GfnDNcmWApYcZsZtsDR3QRfwnuGaCB9bOtI Ght/1+0YMzPWQ1xEWoHLDkbjciUFE8RNNJuqcQW3Bn6FrY+qk6eyy65OlqSqBg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ASCVVsiJ; 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=1728975887; a=rsa-sha256; cv=none; b=udu9UC+iuNzKYn6hRmkJtpYKLYt8RU59ftv1rXmUF/j8xDTB6ZX/JwpA6Oty/wVqOipyQQ h1OvhCYXgsP3Bkkgk58hUiDhyyBGcX+hTKn3O71qXVQby7ceqnOadjVvdAEpTGQpfMjwLY sX09W84+0cydA5nBH+MrKGgvrEsqgdY5EzXzpzhRfow9L575ytO92L60N0Fj98SHUkaJnK gYKimqklATnTEt3COjZ1XBgPIb1bIbm4111WoSsZDh/Aay5bU+pVw+zBqspPcMX15akB/m toV8Paep+Bh101Uyp/zWLyQ0Je38EC2WrV3gSW0+oumZGX2fXYWxLquU646Gvw== 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 2D6A588709 for ; Tue, 15 Oct 2024 09:04:47 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t0bau-0007Dg-54; Tue, 15 Oct 2024 03:03:48 -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 1t0bas-0007DL-G9 for emacs-orgmode@gnu.org; Tue, 15 Oct 2024 03:03:46 -0400 Received: from mail-pg1-x52d.google.com ([2607:f8b0:4864:20::52d]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1t0bai-0007aT-GV for emacs-orgmode@gnu.org; Tue, 15 Oct 2024 03:03:38 -0400 Received: by mail-pg1-x52d.google.com with SMTP id 41be03b00d2f7-7c1324be8easo4113398a12.1 for ; Tue, 15 Oct 2024 00:03:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1728975815; x=1729580615; darn=gnu.org; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=MB9qpGi+NrZoduvEs8tD7Bq9Id3tyfW0yePFUJf4x1o=; b=ASCVVsiJojFUmJeekIW8ERr0Cc9msok8DmqoVxxE7KRq3FS+Ea/BGzmXldBRh+uCLo B3xIs3aTCUlXw/gj1h0ZS7MyvCCahiQU7DIoDTPU9XBRijqJBSGi+kqlFta1U56FZ+id JpncPXoqk441/DQ77gxDQHKQQcW0/EINWinka8eFCPUalWw0ZXjiibxrnb9n5Fbwqm9y 7SMRln0hA5Rba/blcbMPczGHsl5cXi1GEGHK/Maoo5VS2+luke5/fEnBc4vGbY6KfAGQ MgINcT82gNll2jdhfI9Q2UWJ37sPveDwaDFxqaCBy+1JHrRNL6aQqQwEFA1TWI9gwHR1 JztA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728975815; x=1729580615; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MB9qpGi+NrZoduvEs8tD7Bq9Id3tyfW0yePFUJf4x1o=; b=MiZE2d2/4ob6R45wjQCJ5S07jeYTJCdNek/jPKxF93Sv8/pIbi/TJwukqTBt39yV0+ u5U+KN+hsNTzoZMvtJsEKrolDptpRdTPLDIoFqlD/J8bwnQ47f2Q+0K003oMpa2OrhbR puk5AdryFakwOCdhlqCyn/15lhrhrAlh9BLf/ieDaPCWtTXxbdVuMTTAnidY2Mi6Hi6T 4EPdnGZbRepkopCbpge+YdeZx8yBUvJZNtWccEGYLGePz3ISAU80IeDwWN2cIv3PiaId RIyWNmXZIZvxhHeXDsj498qhYfJ4w2Ol8rv5MTTVO/xOuk9h94lUjG9rjVXI7ZXdl5ak n2Tg== X-Gm-Message-State: AOJu0YzfBq7JSy4L0ce6qkz9MBzss4cyl61Yx2689/xVLvZKvSFJ2IK3 dk3JFbiJ4HfnNrfctwUwmpXpWsPbqm2+RXzSZp4P7A0NYGZaXbsTB9zT4A18 X-Google-Smtp-Source: AGHT+IHuSqve0qKiR2O503VE0e9QEYIkD9Gh12dmyru+zvCi+kovvnIvZyV4lErlhxSuKLe6aB2b9Q== X-Received: by 2002:a05:6a21:2d89:b0:1d8:aa1d:b30c with SMTP id adf61e73a8af0-1d8bc7d9000mr21077486637.1.1728975814629; Tue, 15 Oct 2024 00:03:34 -0700 (PDT) Received: from localhost ([198.27.183.102]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7ea9c6d392esm650126a12.44.2024.10.15.00.03.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Oct 2024 00:03:33 -0700 (PDT) From: Jack Kamm To: Ihor Radchenko Cc: emacs-orgmode@gnu.org, matt@excalamus.com, jeremiejuste@gmail.com Subject: Re: [PATCH] Async sessions: Fix prompt removal regression in ob-R In-Reply-To: <87wmiqigfp.fsf@localhost> References: <87setrqs4z.fsf@gmail.com> <87wmiqigfp.fsf@localhost> Date: Tue, 15 Oct 2024 16:03:29 +0900 Message-ID: <87wmi9etku.fsf@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Received-SPF: pass client-ip=2607:f8b0:4864:20::52d; envelope-from=jackkamm@gmail.com; helo=mail-pg1-x52d.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.59 X-Spam-Score: -1.59 X-Migadu-Queue-Id: 2D6A588709 X-TUID: t2I/VQhjVBc3 --=-=-= Content-Type: text/plain Ihor Radchenko 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 --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Disable-async-prompt-removal-in-ob-R-python.patch >From 5c9d6f28f14c51fc542c997ed6aa6792e59857c6 Mon Sep 17 00:00:00 2001 From: Jack Kamm 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[ ]+[ ]* +:\\([ ]+2\\)[ ]+1[ ]+2")) + (should (equal (length (match-string 1)) (length (match-string 2)))) + (kill-buffer session-name))))) (provide 'test-ob-R) -- 2.47.0 --=-=-=--