[PATCH] python.el: Returning inferior Python output without inhibiting it

[PATCH] python.el: Returning inferior Python output without inhibiting it

[Jack Kamm]

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

Hello, maintainer of ob-python.el in Org-mode here.

It would be very useful to ob-python, if python.el had a function that
evaluated Python code and returned output, without inhibiting the
output.

So, this patch modifies `python-shell-send-string-no-output', adding
an argument to show the output. I also rename it to
`python-shell-send-string-return-output' since that is a more accurate
name for the new behavior.

The motivation is that ob-python already implements this in
`org-babel-python-send-string', but we have noticed occasional issues
with leaky prompts in our CI [1], and suspect that the
`python-shell-send-string-no-output' implementation may be more
robust. Also, in Org-mode we have a long term goal to be more modular
and integrate better with the rest of Emacs [2]; and replacing custom
ob-python code with python.el functionality would help with that.

[1] https://list.orgmode.org/873506j7ky.fsf@localhost/
[2] https://list.orgmode.org/orgmode/E1kIPh1-0001Lu-Rg@fencepost.gnu.org/


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-python.el-Function-to-return-output-without-necessar.patch --]
[-- Type: text/x-patch, Size: 9283 bytes --]

From ffdbf2961c9d159b2c1d586667469cf7020240c2 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sat, 30 Sep 2023 21:04:12 -0700
Subject: [PATCH] python.el: Function to return output without necessarily
 inhibiting

Rename `python-shell-send-string-no-output' to
`python-shell-send-string-return-output', and add option to not
inhibit the output.

*
lisp/progmodes/python.el (python-shell-output-filter-inhibit-output):
Variable to determine whether output currently inhibited.
(python-shell-comint-watch-for-first-prompt-output-filter,
python-shell-internal-send-string,
python-shell-completion-native-setup,
python-shell-completion-get-completions, python-ffap-module-path,
python-eldoc--get-doc-at-point): Replace
`python-shell-send-string-no-output' with
`python-shell-send-string-return-output'.
(python-shell-send-string): Check
`python-shell-output-filter-inhibit-output' instead of
`python-shell-output-filter-in-progress' to determine whether output
is inhibited.
(python-shell-output-filter): Set
`python-shell-output-filter-inhibit-output' to nil after reaching end
of output.
(python-shell-send-string-no-output): Obsolete function alias for
`python-shell-send-string-return-output'.
(python-shell-send-string-return-output): Rename from
`python-shell-send-string-no-output'.  Add function argument on
whether output should be inhibited, and set
`python-shell-output-filter-inhibit-output' accordingly.  If not
inhibited, then the original filter function is run in addition to
`python-shell-output-filter', and `comint-last-prompt' is not reset
after evaluation (since the original output filter will set it
approriately).
---
 lisp/progmodes/python.el | 52 ++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d3cb5a77e22..67a79192372 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2736,6 +2736,7 @@ python-shell-dedicated
 
 (defvar python-shell-output-filter-in-progress nil)
 (defvar python-shell-output-filter-buffer nil)
+(defvar python-shell-output-filter-inhibit-output nil)
 
 (defmacro python-shell--add-to-path-with-priority (pathvar paths)
   "Modify PATHVAR and ensure PATHS are added only once at beginning."
@@ -3445,8 +3446,8 @@ python-shell-comint-watch-for-first-prompt-output-filter
                       (format "exec(%s)\n" (python-shell--encode-string string))))))
           ;; Bootstrap: the normal definition of `python-shell-send-string'
           ;; depends on the Python code sent here.
-          (python-shell-send-string-no-output python-shell-eval-setup-code)
-          (python-shell-send-string-no-output python-shell-eval-file-setup-code))
+          (python-shell-send-string-return-output python-shell-eval-setup-code)
+          (python-shell-send-string-return-output python-shell-eval-file-setup-code))
         (with-current-buffer (current-buffer)
           (let ((inhibit-quit nil))
             (run-hooks 'python-shell-first-prompt-hook))))))
@@ -3750,7 +3751,7 @@ python-shell-send-string
                       (python-shell--encode-string string)
                       (python-shell--encode-string (or (buffer-file-name)
                                                        "<string>")))))
-    (unless python-shell-output-filter-in-progress
+    (unless python-shell-output-filter-inhibit-output
       (with-current-buffer (process-buffer process)
         (save-excursion
           (goto-char (process-mark process))
@@ -3766,7 +3767,7 @@ python-shell-send-string
         (python-shell-send-file file-name process temp-file-name t)))))
 
 (defun python-shell-output-filter (string)
-  "Filter used in `python-shell-send-string-no-output' to grab output.
+  "Filter used in `python-shell-send-string-return-output' to grab output.
 STRING is the output received to this point from the process.
 This filter saves received output from the process in
 `python-shell-output-filter-buffer' and stops receiving it after
@@ -3780,6 +3781,7 @@ python-shell-output-filter
     ;; Output ends when `python-shell-output-filter-buffer' contains
     ;; the prompt attached at the end of it.
     (setq python-shell-output-filter-in-progress nil
+          python-shell-output-filter-inhibit-output nil
           python-shell-output-filter-buffer
           (substring python-shell-output-filter-buffer
                      0 (match-beginning 0)))
@@ -3792,15 +3794,23 @@ python-shell-output-filter
             (substring python-shell-output-filter-buffer (match-end 0)))))
   "")
 
-(defun python-shell-send-string-no-output (string &optional process)
-  "Send STRING to PROCESS and inhibit output.
-Return the output."
+(define-obsolete-function-alias
+  'python-shell-send-string-no-output
+  #'python-shell-send-string-return-output "30.1")
+
+(defun python-shell-send-string-return-output (string &optional process show-output)
+  "Send STRING to PROCESS and return the output.
+Inhibit printing the output unless SHOW-OUTPUT is non-nil."
   (or process (setq process (python-shell-get-process-or-error)))
-  (cl-letf* (((process-filter process)
+  (cl-letf* ((original-filter-fn (process-filter process))
+             ((process-filter process)
               (lambda (_proc str)
                 (with-current-buffer (process-buffer process)
-                  (python-shell-output-filter str))))
+                  (python-shell-output-filter str))
+                (when show-output
+                  (funcall original-filter-fn process str))))
              (python-shell-output-filter-in-progress t)
+             (python-shell-output-filter-inhibit-output (not show-output))
              (inhibit-quit t)
              (buffer (process-buffer process))
              (last-prompt (cond ((boundp 'comint-last-prompt-overlay)
@@ -3812,13 +3822,15 @@ python-shell-send-string-no-output
      (with-local-quit
        (unwind-protect
            (python-shell-send-string string process)
-         (when (not (null last-prompt))
-           (with-current-buffer buffer
-             (set last-prompt last-prompt-value))))
+         (unless show-output
+           (when (not (null last-prompt))
+             (with-current-buffer buffer
+               (set last-prompt last-prompt-value)))))
        (while python-shell-output-filter-in-progress
          ;; `python-shell-output-filter' takes care of setting
-         ;; `python-shell-output-filter-in-progress' to NIL after it
-         ;; detects end of output.
+         ;; `python-shell-output-filter-in-progress' and
+         ;; `python-shell-output-filter-inhibit-output' to NIL after
+         ;; it detects end of output.
          (accept-process-output process))
        (prog1
            python-shell-output-filter-buffer
@@ -3828,11 +3840,11 @@ python-shell-send-string-no-output
 
 (defun python-shell-internal-send-string (string)
   "Send STRING to the Internal Python interpreter.
-Returns the output.  See `python-shell-send-string-no-output'."
+Returns the output.  See `python-shell-send-string-return-output'."
   ;; XXX Remove `python-shell-internal-last-output' once CEDET is
   ;; updated to support this new mode.
   (setq python-shell-internal-last-output
-        (python-shell-send-string-no-output
+        (python-shell-send-string-return-output
          ;; Makes this function compatible with the old
          ;; python-send-receive. (At least for CEDET).
          (replace-regexp-in-string "_emacs_out +" "" string)
@@ -4217,7 +4229,7 @@ python-shell-completion-native-try
 (defun python-shell-completion-native-setup ()
   "Try to setup native completion, return non-nil on success."
   (let* ((process (python-shell-get-process))
-         (output (python-shell-send-string-no-output "
+         (output (python-shell-send-string-return-output "
 def __PYTHON_EL_native_completion_setup():
     try:
         import readline
@@ -4454,7 +4466,7 @@ python-shell-completion-get-completions
   (with-current-buffer (process-buffer process)
     (let ((completions
            (python-util-strip-string
-            (python-shell-send-string-no-output
+            (python-shell-send-string-return-output
              (format
               "%s\nprint(';'.join(__PYTHON_EL_get_completions(%s)))"
               python-shell-completion-setup-code
@@ -5198,7 +5210,7 @@ python-ffap-module-path
              (ready (python-shell-with-shell-buffer
                       (python-util-comint-end-of-output-p)))
              (module-file
-              (python-shell-send-string-no-output
+              (python-shell-send-string-return-output
                (format "%s\nprint(__FFAP_get_module_path(%s))"
                        python-ffap-setup-code
                        (python-shell--encode-string module)))))
@@ -5327,7 +5339,7 @@ python-eldoc--get-doc-at-point
                 ;; Prevent resizing the echo area when iPython is
                 ;; enabled.  Bug#18794.
                 (python-util-strip-string
-                 (python-shell-send-string-no-output
+                 (python-shell-send-string-return-output
                   (format
                    "%s\nprint(__PYDOC_get_help(%s))"
                    python-eldoc-setup-code
-- 
2.42.0

Re: [PATCH] python.el: Returning inferior Python output without inhibiting it

[Eli Zaretskii]

> From: Jack Kamm <jackkamm@gmail.com>
> Date: Sun, 01 Oct 2023 09:05:23 -0700
> 
> It would be very useful to ob-python, if python.el had a function that
> evaluated Python code and returned output, without inhibiting the
> output.
> 
> So, this patch modifies `python-shell-send-string-no-output', adding
> an argument to show the output. I also rename it to
> `python-shell-send-string-return-output' since that is a more accurate
> name for the new behavior.

I think instead of renaming (which then requires us to add an obsolete
alias), it would be better to leave python-shell-send-string-no-output
available, but just make it a thin wrapper around
python-shell-send-string-return-output (which I would suggest to name
python-shell-send-string-maybe-output instead).  This way, we don't
annoy Lisp programs that use python-shell-send-string-no-output with
obsolescence warnings.

Thanks.

Re: [PATCH] python.el: Returning inferior Python output without inhibiting it

[Jack Kamm]

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

Eli Zaretskii <eliz@gnu.org> writes:

> I think instead of renaming (which then requires us to add an obsolete
> alias), it would be better to leave python-shell-send-string-no-output
> available, but just make it a thin wrapper around
> python-shell-send-string-return-output (which I would suggest to name
> python-shell-send-string-maybe-output instead).  This way, we don't
> annoy Lisp programs that use python-shell-send-string-no-output with
> obsolescence warnings.

Thanks for the feedback -- I've updated the patch accordingly (attached).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-python.el-Function-to-return-output-without-necessar.patch --]
[-- Type: text/x-patch, Size: 4948 bytes --]

From c071d5a531a6f17e229b0f4a9239f0f054321bc9 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sat, 30 Sep 2023 21:04:12 -0700
Subject: [PATCH] python.el: Function to return output without necessarily
 inhibiting

*
lisp/progmodes/python.el (python-shell-output-filter-inhibit-output):
Variable to determine whether output currently inhibited.
(python-shell-send-string): Check
`python-shell-output-filter-inhibit-output' instead of
`python-shell-output-filter-in-progress' to determine whether output
is inhibited.
(python-shell-output-filter): Set
`python-shell-output-filter-inhibit-output' to nil after reaching end
of output.
(python-shell-send-string-no-output): Turned into a thin wrapper
around `python-shell-send-string-maybe-output'.
(python-shell-send-string-maybe-output): Adapted from
`python-shell-send-string-no-output'.  Add function argument on
whether output should be inhibited, and set
`python-shell-output-filter-inhibit-output' accordingly.  If not
inhibited, then the original filter function is run in addition to
`python-shell-output-filter', and `comint-last-prompt' is not reset
after evaluation (since the original output filter will set it
approriately).
---
 lisp/progmodes/python.el | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d3cb5a77e22..30d0416d035 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2736,6 +2736,7 @@ python-shell-dedicated
 
 (defvar python-shell-output-filter-in-progress nil)
 (defvar python-shell-output-filter-buffer nil)
+(defvar python-shell-output-filter-inhibit-output nil)
 
 (defmacro python-shell--add-to-path-with-priority (pathvar paths)
   "Modify PATHVAR and ensure PATHS are added only once at beginning."
@@ -3750,7 +3751,7 @@ python-shell-send-string
                       (python-shell--encode-string string)
                       (python-shell--encode-string (or (buffer-file-name)
                                                        "<string>")))))
-    (unless python-shell-output-filter-in-progress
+    (unless python-shell-output-filter-inhibit-output
       (with-current-buffer (process-buffer process)
         (save-excursion
           (goto-char (process-mark process))
@@ -3780,6 +3781,7 @@ python-shell-output-filter
     ;; Output ends when `python-shell-output-filter-buffer' contains
     ;; the prompt attached at the end of it.
     (setq python-shell-output-filter-in-progress nil
+          python-shell-output-filter-inhibit-output nil
           python-shell-output-filter-buffer
           (substring python-shell-output-filter-buffer
                      0 (match-beginning 0)))
@@ -3795,12 +3797,21 @@ python-shell-output-filter
 (defun python-shell-send-string-no-output (string &optional process)
   "Send STRING to PROCESS and inhibit output.
 Return the output."
+  (python-shell-send-string-maybe-output string process))
+
+(defun python-shell-send-string-maybe-output (string &optional process show-output)
+  "Send STRING to PROCESS and return the output.
+Inhibit printing the output unless SHOW-OUTPUT is non-nil."
   (or process (setq process (python-shell-get-process-or-error)))
-  (cl-letf* (((process-filter process)
+  (cl-letf* ((original-filter-fn (process-filter process))
+             ((process-filter process)
               (lambda (_proc str)
                 (with-current-buffer (process-buffer process)
-                  (python-shell-output-filter str))))
+                  (python-shell-output-filter str))
+                (when show-output
+                  (funcall original-filter-fn process str))))
              (python-shell-output-filter-in-progress t)
+             (python-shell-output-filter-inhibit-output (not show-output))
              (inhibit-quit t)
              (buffer (process-buffer process))
              (last-prompt (cond ((boundp 'comint-last-prompt-overlay)
@@ -3812,13 +3823,15 @@ python-shell-send-string-no-output
      (with-local-quit
        (unwind-protect
            (python-shell-send-string string process)
-         (when (not (null last-prompt))
-           (with-current-buffer buffer
-             (set last-prompt last-prompt-value))))
+         (unless show-output
+           (when (not (null last-prompt))
+             (with-current-buffer buffer
+               (set last-prompt last-prompt-value)))))
        (while python-shell-output-filter-in-progress
          ;; `python-shell-output-filter' takes care of setting
-         ;; `python-shell-output-filter-in-progress' to NIL after it
-         ;; detects end of output.
+         ;; `python-shell-output-filter-in-progress' and
+         ;; `python-shell-output-filter-inhibit-output' to NIL after
+         ;; it detects end of output.
          (accept-process-output process))
        (prog1
            python-shell-output-filter-buffer
-- 
2.42.0

Re: [PATCH] python.el: Returning inferior Python output without inhibiting it

[Eli Zaretskii]

> From: Jack Kamm <jackkamm@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 01 Oct 2023 13:23:29 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I think instead of renaming (which then requires us to add an obsolete
> > alias), it would be better to leave python-shell-send-string-no-output
> > available, but just make it a thin wrapper around
> > python-shell-send-string-return-output (which I would suggest to name
> > python-shell-send-string-maybe-output instead).  This way, we don't
> > annoy Lisp programs that use python-shell-send-string-no-output with
> > obsolescence warnings.
> 
> Thanks for the feedback -- I've updated the patch accordingly (attached).

Thanks.  kobarity, any comments or suggestions?

Re: [PATCH] python.el: Returning inferior Python output without inhibiting it

[kobarity]

Eli Zaretskii wrote:
> 
> > From: Jack Kamm <jackkamm@gmail.com>
> > Cc: emacs-devel@gnu.org
> > Date: Sun, 01 Oct 2023 13:23:29 -0700
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > I think instead of renaming (which then requires us to add an obsolete
> > > alias), it would be better to leave python-shell-send-string-no-output
> > > available, but just make it a thin wrapper around
> > > python-shell-send-string-return-output (which I would suggest to name
> > > python-shell-send-string-maybe-output instead).  This way, we don't
> > > annoy Lisp programs that use python-shell-send-string-no-output with
> > > obsolescence warnings.
> > 
> > Thanks for the feedback -- I've updated the patch accordingly (attached).
> 
> Thanks.  kobarity, any comments or suggestions?

I agree with leaving `python-shell-send-string-no-output' as it is.
The updated patch looks good to me.

Re: [PATCH] python.el: Returning inferior Python output without inhibiting it

[Eli Zaretskii]

> Date: Mon, 02 Oct 2023 21:42:43 +0900
> From: kobarity <kobarity@gmail.com>
> Cc: Jack Kamm <jackkamm@gmail.com>,
> 	emacs-devel@gnu.org
> 
> 
> Eli Zaretskii wrote:
> > 
> > > From: Jack Kamm <jackkamm@gmail.com>
> > > Cc: emacs-devel@gnu.org
> > > Date: Sun, 01 Oct 2023 13:23:29 -0700
> > > 
> > > Eli Zaretskii <eliz@gnu.org> writes:
> > > 
> > > > I think instead of renaming (which then requires us to add an obsolete
> > > > alias), it would be better to leave python-shell-send-string-no-output
> > > > available, but just make it a thin wrapper around
> > > > python-shell-send-string-return-output (which I would suggest to name
> > > > python-shell-send-string-maybe-output instead).  This way, we don't
> > > > annoy Lisp programs that use python-shell-send-string-no-output with
> > > > obsolescence warnings.
> > > 
> > > Thanks for the feedback -- I've updated the patch accordingly (attached).
> > 
> > Thanks.  kobarity, any comments or suggestions?
> 
> I agree with leaving `python-shell-send-string-no-output' as it is.
> The updated patch looks good to me.

Thanks.  I will wait for a few days to give others a chance to chime
in and post comments or suggestions, and will install this after that,
barring any objections.

Re: [PATCH] python.el: Returning inferior Python output without inhibiting it

[Jack Kamm]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  I will wait for a few days to give others a chance to chime
> in and post comments or suggestions, and will install this after that,
> barring any objections.

Since submitting this patch, I started to have doubts that
`python-shell-send-string-maybe-output' is suitable for ob-python.

So, I suggest not to merge this after all, since ob-python was the main
motivating use case.

The reason is that `python-shell-send-string-no-output' can end
prematurely if the output contains a substring that looks like a prompt,
and ob-python should remain robust to this edge case -- it's not too
far-fetched that some user code executed by ob-python would produce a
substring like ">>>".