Re: bug#49822: master e32c7d2: Change Python eval to send directly instead of using temporary files

[Augusto Stoffel <arstoffel@gmail.com>]

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

On Wed,  8 Sep 2021 at 09:50, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> I have also rearranged things a bit so that the setup code is sent to
>> the inferior process just once, rather than of on every call to
>> `python-shell-send-string'.  This way, the smaller line length limit
>> doesn't increase too much the use of temp files, which, as I mentioned,
>> is slow over ssh.
>
> This leads to compilation warnings about
> python-shell-eval-file-setup-code etc.
>
> Can you adjust the patch to fix that, and also add the defconst for
> 1024, like Eli discussed?
>
>> PS: I have some more suggestions around the Python shell.  Is the ideal
>> workflow to keep creating bugs with a small patch to each improvement,
>> or do you prefer to review a larger collection of changes bundled
>> together?
>
> Smaller patches are fine by me, and you can submit them with `M-x
> submit-emacs-patch'.

Hi Lars,

I've attached a new patch addressing these 2 points.

I've added the max line length defconst to comint.el, since this
something general about PTYs.  The value reflects my knowledge of the
situation (4096 on Linux, possibly 1024 on other OSes).  Hopefully other
people will fill in more cases eventually.

FWIW, when I read the man page for termios, it seems pretty clear that
MAX_CANON is the name of this parameter we're discussing here.  But if I
do 'getconf MAX_CANON /', I get 255, which is definitely not the right
value for my system.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Better-treatment-of-line-length-limits-for-the-Pytho.patch --]
[-- Type: text/x-patch, Size: 8015 bytes --]

From 3eab275464f4a53bf86153f5f3693e522a240b28 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Mon, 6 Sep 2021 23:34:48 +0200
Subject: [PATCH] Better treatment of line length limits for the Python
 inferior

* lisp/comint.el (comint-max-line-length): new constant reflecting a
safe maximum line size that can be sent to an inferior process.
* lisp/progmodes/python.el
(python-shell-comint-watch-for-first-prompt-output-filter): Send setup
code to the inferior process only once and at this stage.
(python-shell-eval-setup-code, python-shell-eval-file-setup-code):
Move, unchanged, to an earlier point to avoid byte-compiler warnings.
(python-shell-send-string-no-output): Revert changes of e32c7d2a8d
(python-shell-send-string): Use 'comint-max-line-length' to decide
when to resort to temp files.
(python-shell-send-string, python-shell-send-file): Don't send setup
code each time.
---
 lisp/comint.el           |  6 +++
 lisp/progmodes/python.el | 98 +++++++++++++++++++++-------------------
 2 files changed, 57 insertions(+), 47 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index e058e6b8cf..c2e528a5d5 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -479,6 +479,12 @@ comint-terminfo-terminal
   :group 'comint
   :version "26.1")
 
+(defconst comint-max-line-length
+  (pcase system-type
+    ('gnu/linux 4096)
+    (_ 1024))
+  "Maximum line length, in bytes, accepted by the inferior process.")
+
 (defvar comint-mode-map
   (let ((map (make-sparse-keymap)))
     ;; Keys:
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d8ec032402..4f222b4cf5 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2811,6 +2811,44 @@ python-shell-first-prompt-hook
   :type 'hook
   :group 'python)
 
+(defconst python-shell-eval-setup-code
+  "\
+def __PYTHON_EL_eval(source, filename):
+    import ast, sys
+    if sys.version_info[0] == 2:
+        from __builtin__ import compile, eval, globals
+    else:
+        from builtins import compile, eval, globals
+    sys.stdout.write('\\n')
+    try:
+        p, e = ast.parse(source, filename), None
+    except SyntaxError:
+        t, v, tb = sys.exc_info()
+        sys.excepthook(t, v, tb.tb_next)
+        return
+    if p.body and isinstance(p.body[-1], ast.Expr):
+        e = p.body.pop()
+    try:
+        g = globals()
+        exec(compile(p, filename, 'exec'), g, g)
+        if e:
+            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)
+    except Exception:
+        t, v, tb = sys.exc_info()
+        sys.excepthook(t, v, tb.tb_next)"
+  "Code used to evaluate statements in inferior Python processes.")
+
+(defconst python-shell-eval-file-setup-code
+  "\
+def __PYTHON_EL_eval_file(filename, tempname, encoding, delete):
+    import codecs, os
+    with codecs.open(tempname or filename, encoding=encoding) as file:
+        source = file.read().encode(encoding)
+    if delete and tempname:
+        os.remove(tempname)
+    return __PYTHON_EL_eval(source, filename)"
+  "Code used to evaluate files in inferior Python processes.")
+
 (defun python-shell-comint-watch-for-first-prompt-output-filter (output)
   "Run `python-shell-first-prompt-hook' when first prompt is found in OUTPUT."
   (when (not python-shell--first-prompt-received)
@@ -2826,6 +2864,15 @@ python-shell-comint-watch-for-first-prompt-output-filter
           (setq python-shell--first-prompt-received-output-buffer nil)
         (setq-local python-shell--first-prompt-received t)
         (setq python-shell--first-prompt-received-output-buffer nil)
+        (cl-letf (((symbol-function 'python-shell-send-string)
+                   (lambda (string process)
+                     (comint-send-string
+                      process
+                      (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))
         (with-current-buffer (current-buffer)
           (let ((inhibit-quit nil))
             (run-hooks 'python-shell-first-prompt-hook))))))
@@ -3081,33 +3128,6 @@ python-shell--save-temp-file
       (delete-trailing-whitespace))
     temp-file-name))
 
-(defconst python-shell-eval-setup-code
-  "\
-def __PYTHON_EL_eval(source, filename):
-    import ast, sys
-    if sys.version_info[0] == 2:
-        from __builtin__ import compile, eval, globals
-    else:
-        from builtins import compile, eval, globals
-    sys.stdout.write('\\n')
-    try:
-        p, e = ast.parse(source, filename), None
-    except SyntaxError:
-        t, v, tb = sys.exc_info()
-        sys.excepthook(t, v, tb.tb_next)
-        return
-    if p.body and isinstance(p.body[-1], ast.Expr):
-        e = p.body.pop()
-    try:
-        g = globals()
-        exec(compile(p, filename, 'exec'), g, g)
-        if e:
-            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)
-    except Exception:
-        t, v, tb = sys.exc_info()
-        sys.excepthook(t, v, tb.tb_next)"
-  "Code used to evaluate statements in inferior Python processes.")
-
 (defalias 'python-shell--encode-string
   (let ((fun (if (and (fboundp 'json-serialize)
                       (>= emacs-major-version 28))
@@ -3128,12 +3148,11 @@ python-shell-send-string
   (interactive
    (list (read-string "Python command: ") nil t))
   (let ((process (or process (python-shell-get-process-or-error msg)))
-        (code (format "exec(%s);__PYTHON_EL_eval(%s, %s)\n"
-                      (python-shell--encode-string python-shell-eval-setup-code)
+        (code (format "__PYTHON_EL_eval(%s, %s)\n"
                       (python-shell--encode-string string)
                       (python-shell--encode-string (or (buffer-file-name)
                                                        "<string>")))))
-    (if (<= (string-bytes code) 4096)
+    (if (<= (string-bytes code) comint-max-line-length)
         (comint-send-string process code)
       (let* ((temp-file-name (with-current-buffer (process-buffer process)
                                (python-shell--save-temp-file string)))
@@ -3180,8 +3199,7 @@ python-shell-send-string-no-output
         (inhibit-quit t))
     (or
      (with-local-quit
-       (comint-send-string
-        process (format "exec(%s)\n" (python-shell--encode-string string)))
+       (python-shell-send-string string process)
        (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
@@ -3378,18 +3396,6 @@ python-shell-send-defun
        nil ;; noop
        msg))))
 
-
-(defconst python-shell-eval-file-setup-code
-  "\
-def __PYTHON_EL_eval_file(filename, tempname, encoding, delete):
-    import codecs, os
-    with codecs.open(tempname or filename, encoding=encoding) as file:
-        source = file.read().encode(encoding)
-    if delete and tempname:
-        os.remove(tempname)
-    return __PYTHON_EL_eval(source, filename)"
-  "Code used to evaluate files in inferior Python processes.")
-
 (defun python-shell-send-file (file-name &optional process temp-file-name
                                          delete msg)
   "Send FILE-NAME to inferior Python PROCESS.
@@ -3419,9 +3425,7 @@ python-shell-send-file
     (comint-send-string
      process
      (format
-      "exec(%s);exec(%s);__PYTHON_EL_eval_file(%s, %s, %s, %s)\n"
-      (python-shell--encode-string python-shell-eval-setup-code)
-      (python-shell--encode-string python-shell-eval-file-setup-code)
+      "__PYTHON_EL_eval_file(%s, %s, %s, %s)\n"
       (python-shell--encode-string file-name)
       (python-shell--encode-string (or temp-file-name ""))
       (python-shell--encode-string (symbol-name encoding))
-- 
2.31.1