bug#49822: 28.0.50; python-shell-send functions show no output

bug#49822: 28.0.50; python-shell-send functions show no output

[dalanicolai]

M-x run-python
type e.g.: 5+5
then send using `C-c C-c`

The comint shows no output. Same for ipython comint.
This is due to the 'coding cookie' in the `python-shell-send-...`
commands (more exactly in `python-shell-buffer-substring`).

When using e.g. comint-simple-send, things work correctly.
It would be great if sending the 'coding cookie' (and the addtional
line
sent for the case of a trace) could be made optional.

Here is some package where @fernandomayer fixes it for ipython.
(But it is equally problematic for default python)

This bug has probably been mentioned somewhere but I could not quickly
find any mention of it on in the bug archive or even on google.
So at least it has been reported now.



In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.25, cairo version 1.16.0)
 of 2021-02-18 built on daniel-fedora
Repository revision: 185121da6978553d538d37d6d0e67dc52e13311f
Repository branch: feature/native-comp
Windowing system distributor 'The X.Org Foundation', version
11.0.12011000
System Description: Fedora 34 (Workstation Edition)

Configured using:
 'configure --with-nativecomp'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP
NOTIFY INOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml easymenu mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core eieio-loaddefs
password-cache json map cl-macs text-property-search time-date subr-x
seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils thingatpt
cl-loaddefs cl-lib iso-transl tooltip eldoc electric uniquify ediff-
hook
vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core term/tty-colors frame
minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray cl-preloaded nadvice button loaddefs faces
cus-face pcase macroexp files window text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-
readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting cairo move-toolbar gtk x-
toolkit
x multi-tty make-network-process nativecomp emacs)

Memory information:
((conses 16 70942 10482)
 (symbols 48 6768 0)
 (strings 32 19439 1610)
 (string-bytes 1 667085)
 (vectors 16 12671)
 (vector-slots 8 278285 13081)
 (floats 8 24 53)
 (intervals 56 203 0)
 (buffers 992 11))

bug#49822: forgot link (and mentioned wrong user)

[dalanicolai]

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

So the link should have been this one
https://github.com/jackkamm/ipython-shell-send-el
and @fernandomayer should have been @jackkamm

[-- Attachment #2: Type: text/html, Size: 256 bytes --]

bug#49822: 28.0.50; python-shell-send functions show no output

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

dalanicolai@gmail.com writes:

> M-x run-python
> type e.g.: 5+5
> then send using `C-c C-c`
>
> The comint shows no output. Same for ipython comint.
> This is due to the 'coding cookie' in the `python-shell-send-...`
> commands (more exactly in `python-shell-buffer-substring`).
>
> When using e.g. comint-simple-send, things work correctly.
> It would be great if sending the 'coding cookie' (and the addtional
> line
> sent for the case of a trace) could be made optional.
>
> Here is some package where @fernandomayer fixes it for ipython.
> (But it is equally problematic for default python)
>
> This bug has probably been mentioned somewhere but I could not quickly
> find any mention of it on in the bug archive or even on google.
> So at least it has been reported now.
>
>
>
> In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
> 3.24.25, cairo version 1.16.0)
>  of 2021-02-18 built on daniel-fedora
> Repository revision: 185121da6978553d538d37d6d0e67dc52e13311f
> Repository branch: feature/native-comp

Hi,

just to mention that feature/native-comp does not exists anymore as a
branch, one should point to master as native-comp was merged into it.
185121da6 is a 6 months old commit.

Regards

  Andrea

bug#49822: 28.0.50; python-shell-send functions show no output

[Lars Ingebrigtsen]

dalanicolai@gmail.com writes:

> M-x run-python
> type e.g.: 5+5
> then send using `C-c C-c`
>
> The comint shows no output. Same for ipython comint.

I'm unable to reproduce this on the current trunk -- I seem to recall
this was fixed a few months ago.

[...]

> In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
> 3.24.25, cairo version 1.16.0)
>  of 2021-02-18 built on daniel-fedora
> Repository revision: 185121da6978553d538d37d6d0e67dc52e13311f
> Repository branch: feature/native-comp

As Andrea said -- this branch no longer exists (it's been merged into
the trunk), which might explain why your Emacs is half a year behind.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49822: 28.0.50; python-shell-send functions show no output

[dalanicolai]

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

Okay, I have switched to the master branch now and rebuild emacs.

So I find it works differently than I thought. As reported in my original
bug-report,
the comint still does not show the output when sending using `C-c C-c` but
I see now
that actually that is bound to `python-shell-send-buffer`. So now I notice
that the only
function that effects in the comint printing the output is
`python-shell-send-statement`.

Probably I was misled by the Spacemacs function `pyhton-shell-send-line`
which uses
`python-shell-send-region` and therefore when used the comint does not
print the output.
(And I was blindly assuming that `C-c C-c` was using shell send
line/statement)

Thanks for your replies!


On Wed, 4 Aug 2021 at 09:18, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> dalanicolai@gmail.com writes:
>
> > M-x run-python
> > type e.g.: 5+5
> > then send using `C-c C-c`
> >
> > The comint shows no output. Same for ipython comint.
>
> I'm unable to reproduce this on the current trunk -- I seem to recall
> this was fixed a few months ago.
>
> [...]
>
> > In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
> > 3.24.25, cairo version 1.16.0)
> >  of 2021-02-18 built on daniel-fedora
> > Repository revision: 185121da6978553d538d37d6d0e67dc52e13311f
> > Repository branch: feature/native-comp
>
> As Andrea said -- this branch no longer exists (it's been merged into
> the trunk), which might explain why your Emacs is half a year behind.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[-- Attachment #2: Type: text/html, Size: 2273 bytes --]

bug#49822: 28.0.50; python-shell-send functions show no output

[Augusto Stoffel]

The specific test described above works for me too.  But the display of
a result in the Python shell is very brittle.  For instance, if one
evaluates a buffer with contents

```
(5
 + 5)
```

then no result is shown.

I guess it would also be desirable to have the behavior of
https://github.com/jackkamm/ipython-shell-send-el work with the regular
interpreter, so evaluating

```
x = 5 + 5
x
```

prints the result `10`.

On Wed,  4 Aug 2021 at 09:18, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> dalanicolai@gmail.com writes:
>
>> M-x run-python
>> type e.g.: 5+5
>> then send using `C-c C-c`
>>
>> The comint shows no output. Same for ipython comint.
>
> I'm unable to reproduce this on the current trunk -- I seem to recall
> this was fixed a few months ago.
>
> [...]
>
>> In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
>> 3.24.25, cairo version 1.16.0)
>>  of 2021-02-18 built on daniel-fedora
>> Repository revision: 185121da6978553d538d37d6d0e67dc52e13311f
>> Repository branch: feature/native-comp
>
> As Andrea said -- this branch no longer exists (it's been merged into
> the trunk), which might explain why your Emacs is half a year behind.

bug#49822: 28.0.50; python-shell-send functions show no output

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> The specific test described above works for me too.  But the display of
> a result in the Python shell is very brittle.  For instance, if one
> evaluates a buffer with contents
>
> ```
> (5
>  + 5)
> ```
>
> then no result is shown.

Ah, right -- I can reproduce this, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49822: 28.0.50; python-shell-send functions show no output

[Andreas Röhler]

On 13.08.21 13:10, Lars Ingebrigtsen wrote:
> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> The specific test described above works for me too.  But the display of
>> a result in the Python shell is very brittle.  For instance, if one
>> evaluates a buffer with contents
>>
>> ```
>> (5
>>   + 5)
>> ```
>>
>> then no result is shown.
> Ah, right -- I can reproduce this, too.
>
Hmm, a buffer is not an interactive shell. Putting a print() around 
should do it.

Whereas in a shell

 >>> (4 +
  5)
... 9

works as expected.

bug#49822: 28.0.50; python-shell-send functions show no output

[Augusto Stoffel]

On Fri, 13 Aug 2021 at 18:44, Andreas Röhler <andreas.roehler@easy-emacs.de> wrote:

> Hmm, a buffer is not an interactive shell. Putting a print() around
> should do it.

I'm not sure I get your point.  Evaluating an expression should print
the result, no?  It's the P in REPL.

bug#49822: 28.0.50; python-shell-send functions show no output

[Andreas Röhler]

On 14.08.21 11:29, Augusto Stoffel wrote:
> On Fri, 13 Aug 2021 at 18:44, Andreas Röhler <andreas.roehler@easy-emacs.de> wrote:
>
>> Hmm, a buffer is not an interactive shell. Putting a print() around
>> should do it.
> I'm not sure I get your point.  Evaluating an expression should print
> the result, no?  It's the P in REPL.

Python's interactive prompt offers a slightly different behavior than 
running code through a plain Python interpreter.

I understand the code in a buffer as something which is executed as a 
program. If you run a program by a python interpreter, if it outputs 
something, that depends.

bug#49822: 28.0.50; python-shell-send functions show no output

[Augusto Stoffel]

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

On Sun, 15 Aug 2021 at 08:41, Andreas Röhler <andreas.roehler@easy-emacs.de> wrote:

> On 14.08.21 11:29, Augusto Stoffel wrote:
>> On Fri, 13 Aug 2021 at 18:44, Andreas Röhler <andreas.roehler@easy-emacs.de> wrote:
>>
>>> Hmm, a buffer is not an interactive shell. Putting a print() around
>>> should do it.
>> I'm not sure I get your point.  Evaluating an expression should print
>> the result, no?  It's the P in REPL.
>
> Python's interactive prompt offers a slightly different behavior than
> running code through a plain Python interpreter.
>
> I understand the code in a buffer as something which is executed as a
> program. If you run a program by a python interpreter, if it outputs 
> something, that depends.

Well, maybe that depends a bit on the use-case and taste.  But I would
like "send to the shell" to be more or less equivalent to "copy and
paste in the shell buffer".  Other people would prefer this as well,
cf. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29592

Anyway, I played around a bit and it's not hard to recover the Jupyter
behavior where evaluating some statements returns the last value.  The
attached file showing how it works.  I think Elpy does something
similar, in fact.

I'm struggling a bit on how this could be integrated with python.el.
Just dropping these new commands there would be easy, but not ideal; one
should probably integrate more tightly with the existing python-shell
commands, say by introducing a 'python-sell-echo-result' defcustom.  But
then I feel tempted to change various other things I don't like about
the Python shell, in particular the use of temporary files (as opposed
to escaped strings) to transfer pieces of code, which is very slow via
Tramp.

By the way, I also dislike how the 'python-shell-send-*' functions
don't even print a newline on the shell buffer, so that you see
    ">>> >>> >>>"
instead of
    ">>>\n>>>\n>>>"
after calling them thrice.  This means (1) a result may be printed right
next to the input prompt ">>>", and (2) it's impossible to tell if the
inferior process is busy by looking at the relative position of the
point and prompt.


[-- Attachment #2: python-shell-eval.el --]
[-- Type: application/emacs-lisp, Size: 2504 bytes --]

bug#49822: 28.0.50; python-shell-send functions show no output

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> I'm struggling a bit on how this could be integrated with python.el.
> Just dropping these new commands there would be easy, but not ideal; one
> should probably integrate more tightly with the existing python-shell
> commands, say by introducing a 'python-sell-echo-result' defcustom.  But
> then I feel tempted to change various other things I don't like about
> the Python shell, in particular the use of temporary files (as opposed
> to escaped strings) to transfer pieces of code, which is very slow via
> Tramp.

If you could fix this (i.e., not use temporary files), that would be
very nice.

> By the way, I also dislike how the 'python-shell-send-*' functions
> don't even print a newline on the shell buffer, so that you see
>     ">>> >>> >>>"
> instead of
>     ">>>\n>>>\n>>>"
> after calling them thrice.  This means (1) a result may be printed right
> next to the input prompt ">>>", and (2) it's impossible to tell if the
> inferior process is busy by looking at the relative position of the
> point and prompt.

Yes, that's pretty annoying.  I'm sure plenty of people would appreciate
it if this was fixed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49822: [RFC PATCH] python-shell-send functions show no output

[Augusto Stoffel]

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

On Thu, 26 Aug 2021 at 16:09, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> If you could fix this (i.e., not use temporary files), that would be
> very nice.

All right, I'm attaching a proposal for 'python-shell-send-string'
(affecting also all other 'python-shell-send-' functions except
'python-shell-send-buffer').

The main user-visible change is that now a result is printed in the
shell in all possible cases, i.e.:

1. If the string being evaluated is a one-line expression.
2. If the string being evaluated is a multiline expression such as
   "(1+\n1)".
3. The string being evaluated is contains multiple statements, the last
   of which is an expression.  E.g., "x=1\nx".

Currently, only case 1 prints a result (this is the actual topic of this
bug).

By avoiding to use 'python-shell-send-file', the following additional
issues are fixed:

1. The plumbing code used to execute the user's code could crop up in a
   stack trace (Bug#32042).  This is now almost completely fixed (the
   only exception I see is in the plain python3 interpreter, where a
   not-too-annoying mention to __PYTHON_EL_eval still appears.)
2. Sending temporary files over Tramp has a noticeable delay (and lots
   of echo area messages).  In particular, completion is now faster on
   remote Python shells.
3. Some global symbols (os, codecs, etc.) used to be redefined; some
   others (compile, etc.) used to be assumed to not have been redefined
   by the user.  We now only assume this is the case for 'exec' (because
   it's a keyword in Python 2).

Note, however, that 'python-shell-send-file' remains unchanged.

This is a reasonably sensitive change and it would be good to collect
feedback from others before merging it (a properly formatted patch will
then follow).  In particular, since I'm not an expert in Python's
internals, there might be some possible refinements to the
__PYTHON_EL_eval function.  Is this perhaps a better topic for the
emacs-devel list?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-python-shell-send-functions.patch --]
[-- Type: text/x-patch, Size: 4613 bytes --]

From 10b474c9f82f13b3ee9c451a6aea0d3660041095 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 28 Aug 2021 11:16:40 +0200
Subject: [PATCH] Improve python-shell-send functions

---
 lisp/progmodes/python.el | 64 +++++++++++++++++++++++++++++++---------
 1 file changed, 50 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d5209d8d2f..bed709bb65 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3081,6 +3081,45 @@ python-shell--save-temp-file
       (delete-trailing-whitespace))
     temp-file-name))
 
+(defvar 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))
+                 'json-serialize
+               (require 'json)
+               'json-encode-string)))
+    (lambda (text)
+      (if (stringp text)
+          (funcall fun text)
+        (signal 'wrong-type-argument (list 'stringp text)))))
+  "Encode TEXT as a valid Python string.")
+
 (defun python-shell-send-string (string &optional process msg)
   "Send STRING to inferior Python PROCESS.
 When optional argument MSG is non-nil, forces display of a
@@ -3088,16 +3127,12 @@ python-shell-send-string
 t when called interactively."
   (interactive
    (list (read-string "Python command: ") nil t))
-  (let ((process (or process (python-shell-get-process-or-error msg))))
-    (if (string-match ".\n+." string)   ;Multiline.
-        (let* ((temp-file-name (with-current-buffer (process-buffer process)
-                                 (python-shell--save-temp-file string)))
-               (file-name (or (buffer-file-name) temp-file-name)))
-          (python-shell-send-file file-name process temp-file-name t))
-      (comint-send-string process string)
-      (when (or (not (string-match "\n\\'" string))
-                (string-match "\n[ \t].*\n?\\'" string))
-        (comint-send-string process "\n")))))
+  (comint-send-string
+   (or process (python-shell-get-process-or-error msg))
+   (format "exec(%s);__PYTHON_EL_eval(%s, %s)\n"
+           (python-shell--encode-string python-shell-eval-setup-code)
+           (python-shell--encode-string string)
+           (python-shell--encode-string (or (buffer-file-name) "<string>")))))
 
 (defvar python-shell-output-filter-in-progress nil)
 (defvar python-shell-output-filter-buffer nil)
@@ -3139,7 +3174,8 @@ python-shell-send-string-no-output
         (inhibit-quit t))
     (or
      (with-local-quit
-       (python-shell-send-string string process)
+       (comint-send-string process (format "exec(%s)\n"
+                                           (python-shell--encode-string string)))
        (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
@@ -3362,7 +3398,8 @@ python-shell-send-file
          (temp-file-name (when temp-file-name
                            (file-local-name (expand-file-name
                                              temp-file-name)))))
-    (python-shell-send-string
+    (comint-send-string
+     process
      (format
       (concat
        "import codecs, os;"
@@ -3372,8 +3409,7 @@ python-shell-send-file
        (when (and delete temp-file-name)
          (format "os.remove('''%s''');" temp-file-name))
        "exec(compile(__code, '''%s''', 'exec'));")
-      (or temp-file-name file-name) encoding encoding file-name)
-     process)))
+      (or temp-file-name file-name) encoding encoding file-name))))
 
 (defun python-shell-switch-to-shell (&optional msg)
   "Switch to inferior Python process buffer.
-- 
2.31.1

bug#49822: [RFC PATCH] python-shell-send functions show no output

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> The main user-visible change is that now a result is printed in the
> shell in all possible cases, i.e.:
>
> 1. If the string being evaluated is a one-line expression.
> 2. If the string being evaluated is a multiline expression such as
>    "(1+\n1)".
> 3. The string being evaluated is contains multiple statements, the last
>    of which is an expression.  E.g., "x=1\nx".
>
> Currently, only case 1 prints a result (this is the actual topic of this
> bug).

Sounds like a definite improvement.  :-)

> This is a reasonably sensitive change and it would be good to collect
> feedback from others before merging it (a properly formatted patch will
> then follow).  In particular, since I'm not an expert in Python's
> internals, there might be some possible refinements to the
> __PYTHON_EL_eval function.  Is this perhaps a better topic for the
> emacs-devel list?

Yes, it might be a good idea to raise this topic on emacs-devel and see
if there's any feedback.

Some minor comments on the patch:

> +(defvar 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

I think it might be a better long-term maintenance solution to put the
Python code here (and from `python-shell-completion-native-setup') into
a .py file under etc (for instance in a new "python-mode"
sub-directory), and then read it in run-time (accessed via
`data-directory', which points to etc), and cache it in an internal
variable.

Editing Python that's inside a Lisp string is awkward.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49822: [PATCH] python-shell-send functions show no output

[Augusto Stoffel]

On Sat, 28 Aug 2021 at 17:41, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Yes, it might be a good idea to raise this topic on emacs-devel and see
> if there's any feedback.

Okay, nobody chimed in on this thread (and I think I forgot to mention
it in emacs-devel), but this patch works well for me so far.  So I would
suggest to merge it.

>
> Some minor comments on the patch:
>
>> +(defvar 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
>
> I think it might be a better long-term maintenance solution to put the
> Python code here (and from `python-shell-completion-native-setup') into
> a .py file under etc (for instance in a new "python-mode"
> sub-directory), and then read it in run-time (accessed via
> `data-directory', which points to etc), and cache it in an internal
> variable.
>
> Editing Python that's inside a Lisp string is awkward.

Yes, that makes sense.  On the other hand, it would make the entire
setup a bit more complicated, and I'm unsure about some of the details.
I do want to refactor `python-shell-send-file', though, so that at least
it doesn't generate Python code strings on the fly (and at the same time
solve the remaining part of Bug#32042).

bug#49822: [PATCH] python-shell-send functions show no output

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> On Sat, 28 Aug 2021 at 17:41, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> Yes, it might be a good idea to raise this topic on emacs-devel and see
>> if there's any feedback.
>
> Okay, nobody chimed in on this thread (and I think I forgot to mention
> it in emacs-devel), but this patch works well for me so far.  So I would
> suggest to merge it.

OK; now done (with some trivial white-space changes to get lines under
80 chars...  and I changed the defvar to a defconst).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Stefan Monnier]

>     Change Python eval to send directly instead of using temporary files

FWIW, sending large amounts of text via pty can be troublesome (some
OSes have been known to insert additional control chars every 256
bytes, or to do weird things after 4096 bytes leading the send to never
complete, ...), which is why it's very common for comint modes to send
regions of text via temp files.


        Stefan

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

[Augusto Stoffel]

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

On Fri,  3 Sep 2021 at 19:04, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>>     Change Python eval to send directly instead of using temporary files
>
> FWIW, sending large amounts of text via pty can be troublesome (some
> OSes have been known to insert additional control chars every 256
> bytes, or to do weird things after 4096 bytes leading the send to never
> complete, ...), which is why it's very common for comint modes to send
> regions of text via temp files.
>
>
>         Stefan

Okay, the 4096 bytes limit indeed exists in IPython (but not in the
standard Python interpreters, apparently).

I've attached a patch that reverts to using temporary files for
sufficiently long strings (would this magic 4096 ever require
customization?).  The patch also solves bug#32042, which is mostly
unrelated.

I would like pass text inline as much as possible because the
back-and-forth of temp files is pretty slow over Tramp, which makes the
Python shell rather annoying to use.

As to some OSes inserting additional control characters every 256 bytes,
I've never encountered this, but it seems a rather tricky problem to
work around.  Before this commit, the "plumbing code" sent to the
interpreter could already be above 256 bytes (e.g., if the generated
temp file names are modestly long).  Inserting newline characters at
strategic places would print random prompt strings, and therefore also
introduce complications.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fixes-for-python-shell-send-string-and-python-shell-.patch --]
[-- Type: text/x-patch, Size: 3852 bytes --]

From 6fdbecb66d4409e2ce03c559861138966abb4acd Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 4 Sep 2021 11:16:11 +0200
Subject: [PATCH] Fixes for 'python-shell-send-string' and
 'python-shell-send-file'

* lisp/progmodes/python.el (python-shell-send-string): use a temporary
file for sufficiently long strings.
(python-shell-send-file, python-shell-eval-file-setup-code): Avoid
showing "plumbing code" in the traceback (bug#32042).
---
 lisp/progmodes/python.el | 46 +++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 306cc3a542..d8ec032402 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3127,12 +3127,18 @@ python-shell-send-string
 t when called interactively."
   (interactive
    (list (read-string "Python command: ") nil t))
-  (comint-send-string
-   (or process (python-shell-get-process-or-error msg))
-   (format "exec(%s);__PYTHON_EL_eval(%s, %s)\n"
-           (python-shell--encode-string python-shell-eval-setup-code)
-           (python-shell--encode-string string)
-           (python-shell--encode-string (or (buffer-file-name) "<string>")))))
+  (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)
+                      (python-shell--encode-string string)
+                      (python-shell--encode-string (or (buffer-file-name)
+                                                       "<string>")))))
+    (if (<= (string-bytes code) 4096)
+        (comint-send-string process code)
+      (let* ((temp-file-name (with-current-buffer (process-buffer process)
+                               (python-shell--save-temp-file string)))
+             (file-name (or (buffer-file-name) temp-file-name)))
+        (python-shell-send-file file-name process temp-file-name t)))))
 
 (defvar python-shell-output-filter-in-progress nil)
 (defvar python-shell-output-filter-buffer nil)
@@ -3372,6 +3378,18 @@ 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.
@@ -3401,15 +3419,13 @@ python-shell-send-file
     (comint-send-string
      process
      (format
-      (concat
-       "import codecs, os;"
-       "__pyfile = codecs.open('''%s''', encoding='''%s''');"
-       "__code = __pyfile.read().encode('''%s''');"
-       "__pyfile.close();"
-       (when (and delete temp-file-name)
-         (format "os.remove('''%s''');" temp-file-name))
-       "exec(compile(__code, '''%s''', 'exec'));")
-      (or temp-file-name file-name) encoding encoding file-name))))
+      "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-shell--encode-string file-name)
+      (python-shell--encode-string (or temp-file-name ""))
+      (python-shell--encode-string (symbol-name encoding))
+      (if delete "True" "False")))))
 
 (defun python-shell-switch-to-shell (&optional msg)
   "Switch to inferior Python process buffer.
-- 
2.31.1

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> As to some OSes inserting additional control characters every 256 bytes,
> I've never encountered this, but it seems a rather tricky problem to
> work around.

IIRC this was on some version of macos, and maybe the 256B limit applied
only to text without newlines or something like that [it was too long ago].


        Stefan

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

[Stefan Monnier]

> As to some OSes inserting additional control characters every 256 bytes,
> I've never encountered this, but it seems a rather tricky problem to
> work around.

IIRC this was on some version of macos, and maybe the 256B limit applied
only to text without newlines or something like that [it was too long ago].


        Stefan

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

[Barton, Mark]

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


On Sep 4, 2021, at 2:49 AM, Augusto Stoffel <arstoffel@gmail.com<mailto:arstoffel@gmail.com>> wrote:

On Fri,  3 Sep 2021 at 19:04, Stefan Monnier <monnier@iro.umontreal.ca<mailto:monnier@iro.umontreal.ca>> wrote:

   Change Python eval to send directly instead of using temporary files

FWIW, sending large amounts of text via pty can be troublesome (some
OSes have been known to insert additional control chars every 256
bytes, or to do weird things after 4096 bytes leading the send to never
complete, ...), which is why it's very common for comint modes to send
regions of text via temp files.


       Stefan

Okay, the 4096 bytes limit indeed exists in IPython (but not in the
standard Python interpreters, apparently).

I've attached a patch that reverts to using temporary files for
sufficiently long strings (would this magic 4096 ever require
customization?).  The patch also solves bug#32042, which is mostly
unrelated.

I would like pass text inline as much as possible because the
back-and-forth of temp files is pretty slow over Tramp, which makes the
Python shell rather annoying to use.

As to some OSes inserting additional control characters every 256 bytes,
I've never encountered this, but it seems a rather tricky problem to
work around.  Before this commit, the "plumbing code" sent to the
interpreter could already be above 256 bytes (e.g., if the generated
temp file names are modestly long).  Inserting newline characters at
strategic places would print random prompt strings, and therefore also
introduce complications.

<0001-Fixes-for-python-shell-send-string-and-python-shell-.patch>

Today I was trying to export my monthly org document that uses org babel python blocks to produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d - Change Python eval to send directly instead of using temporary files.

Below I show the python session buffer, first where it is working and then where it is broken for me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.

In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t understand the problem. Any ideas on what other information I can gather that would be useful?


Working session buffer when using previous python.el to export document

Python 3.9.6 (default, Jun 29 2021, 05:25:02)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> org_babel_python_eoe
>>> python.el: native completion setup loaded
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>>

Broken session buffer with current python.el in e32c7d2a8d

Python 3.9.6 (default, Jun 29 2021, 05:25:02)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>
org_babel_python_eoe
>>>   File "<stdin>", line 1
    exec("def __PYTHON_EL_eval(source, filename):\n    import ast, sys\n    if sys.version_info[0] == 2:\n        from __builtin__ import compile, eval, globals\n    else:\n        from builtins import compile, eval, globals\n    sys.stdout.write('\\n')\n    try:\n        p, e = ast.parse(source, filename), None\n    except SyntaxError:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)\n        return\n    if p.body and isinstance(p.body[-1], ast.Expr):\n        e = p.body.pop()\n    try:\n        g = globals()\n        exec(compile(p, filename, 'exec'), g, g)\n        if e:\n            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)\n    except Exception:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)");__PYTHON_EL_eval("\ndef __PYTHON_EL_native_completion_setup():\n    try:\n        import readline\n\n        try:\n            import __builtin__\n        except ImportError:\n            # Python 3\n            import builtins as __bexec("def __PYTHON_EL_eval(source, filename):\n    import ast, sys\n    if sys.version_info[0] == 2:\n        from __builtin__ import compile, eval, globals\n    else:\n        from builtins import compile, eval, globals\n    sys.stdout.write('\\n')\n    try:\n        p, e = ast.parse(source, filename), None\n    except SyntaxError:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)\n        return\n    if p.body and isinstance(p.body[-1], ast.Expr):\n        e = p.body.pop()\n    try:\n        g = globals()\n        exec(compile(p, filename, 'exec'), g, g)\n        if e:\n            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)\n    except Exception:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)");__PYTHON_EL_eval("try:\n    import ast\n    with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-VMz5YY/python-Xmy5oT') as __org_babel_python_tmpfile:\n        __org_babel_python_ast = ast.parse(__org_babel_python_tmpfile.read())\n    __org_babel_python_final = __org_babel_python_ast.body[-1]\n    if isinstance(__org_babel_python_final, ast.Expr):\n        __org_babel_python_ast.body = __org_babel_python_ast.body[:-1]\n        exec(compile(__org_babel_python_ast, '<string>', 'exec'))\n        __org_babel_python_final = eval(compile(ast.Expression(\n            __org_babel_python_final.value), '<string>', 'eval'))\n        with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-VMz5YY/python-hzPBKk', 'w') as __org_babel_python_tmpfile:\n            if False:\n                import pprint\n                __org_babel_python_tmpfile.write(pprint.pformat(__org_babel_python_final))\n            else:\n                __org_babel_python_tmpfile.write(str(__org_babel_python_final))\n    else:\n        exec(compile(__org_babel_python_ast, '<string>', 'exec'))\n        __org_babel_python_final = None\nexcept:\n    raise\nfinally:\n    print('org_babel_python_eoe')", "<string>")
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          ^
SyntaxError: invalid syntax
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfMembership' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfMembership' is not defined
>>>
2021-08-24org_babel_python_eoe
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfYPTdata' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfTrainingRpt' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfCompareSBwithTM' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfAdvData' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfCompareSBwithTM' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 22, in <module>
NameError: name 'dfAdvData' is not defined
>>>


[-- Attachment #2: Type: text/html, Size: 50519 bytes --]

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

[Barton, Mark]

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


On Sep 4, 2021, at 2:49 AM, Augusto Stoffel <arstoffel@gmail.com<mailto:arstoffel@gmail.com>> wrote:

On Fri,  3 Sep 2021 at 19:04, Stefan Monnier <monnier@iro.umontreal.ca<mailto:monnier@iro.umontreal.ca>> wrote:

   Change Python eval to send directly instead of using temporary files

FWIW, sending large amounts of text via pty can be troublesome (some
OSes have been known to insert additional control chars every 256
bytes, or to do weird things after 4096 bytes leading the send to never
complete, ...), which is why it's very common for comint modes to send
regions of text via temp files.


       Stefan

Okay, the 4096 bytes limit indeed exists in IPython (but not in the
standard Python interpreters, apparently).

I've attached a patch that reverts to using temporary files for
sufficiently long strings (would this magic 4096 ever require
customization?).  The patch also solves bug#32042, which is mostly
unrelated.

I would like pass text inline as much as possible because the
back-and-forth of temp files is pretty slow over Tramp, which makes the
Python shell rather annoying to use.

As to some OSes inserting additional control characters every 256 bytes,
I've never encountered this, but it seems a rather tricky problem to
work around.  Before this commit, the "plumbing code" sent to the
interpreter could already be above 256 bytes (e.g., if the generated
temp file names are modestly long).  Inserting newline characters at
strategic places would print random prompt strings, and therefore also
introduce complications.

<0001-Fixes-for-python-shell-send-string-and-python-shell-.patch>

Today I was trying to export my monthly org document that uses org babel python blocks to produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d - Change Python eval to send directly instead of using temporary files.

Below I show the python session buffer, first where it is working and then where it is broken for me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.

In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t understand the problem. Any ideas on what other information I can gather that would be useful?


Working session buffer when using previous python.el to export document

Python 3.9.6 (default, Jun 29 2021, 05:25:02)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> org_babel_python_eoe
>>> python.el: native completion setup loaded
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-24org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>> 2021-08-27org_babel_python_eoe
>>>

Broken session buffer with current python.el in e32c7d2a8d

Python 3.9.6 (default, Jun 29 2021, 05:25:02)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>
org_babel_python_eoe
>>>   File "<stdin>", line 1
    exec("def __PYTHON_EL_eval(source, filename):\n    import ast, sys\n    if sys.version_info[0] == 2:\n        from __builtin__ import compile, eval, globals\n    else:\n        from builtins import compile, eval, globals\n    sys.stdout.write('\\n')\n    try:\n        p, e = ast.parse(source, filename), None\n    except SyntaxError:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)\n        return\n    if p.body and isinstance(p.body[-1], ast.Expr):\n        e = p.body.pop()\n    try:\n        g = globals()\n        exec(compile(p, filename, 'exec'), g, g)\n        if e:\n            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)\n    except Exception:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)");__PYTHON_EL_eval("\ndef __PYTHON_EL_native_completion_setup():\n    try:\n        import readline\n\n        try:\n            import __builtin__\n        except ImportError:\n            # Python 3\n            import builtins as __bexec("def __PYTHON_EL_eval(source, filename):\n    import ast, sys\n    if sys.version_info[0] == 2:\n        from __builtin__ import compile, eval, globals\n    else:\n        from builtins import compile, eval, globals\n    sys.stdout.write('\\n')\n    try:\n        p, e = ast.parse(source, filename), None\n    except SyntaxError:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)\n        return\n    if p.body and isinstance(p.body[-1], ast.Expr):\n        e = p.body.pop()\n    try:\n        g = globals()\n        exec(compile(p, filename, 'exec'), g, g)\n        if e:\n            return eval(compile(ast.Expression(e.value), filename, 'eval'), g, g)\n    except Exception:\n        t, v, tb = sys.exc_info()\n        sys.excepthook(t, v, tb.tb_next)");__PYTHON_EL_eval("try:\n    import ast\n    with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-VMz5YY/python-Xmy5oT') as __org_babel_python_tmpfile:\n        __org_babel_python_ast = ast.parse(__org_babel_python_tmpfile.read())\n    __org_babel_python_final = __org_babel_python_ast.body[-1]\n    if isinstance(__org_babel_python_final, ast.Expr):\n        __org_babel_python_ast.body = __org_babel_python_ast.body[:-1]\n        exec(compile(__org_babel_python_ast, '<string>', 'exec'))\n        __org_babel_python_final = eval(compile(ast.Expression(\n            __org_babel_python_final.value), '<string>', 'eval'))\n        with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-VMz5YY/python-hzPBKk', 'w') as __org_babel_python_tmpfile:\n            if False:\n                import pprint\n                __org_babel_python_tmpfile.write(pprint.pformat(__org_babel_python_final))\n            else:\n                __org_babel_python_tmpfile.write(str(__org_babel_python_final))\n    else:\n        exec(compile(__org_babel_python_ast, '<string>', 'exec'))\n        __org_babel_python_final = None\nexcept:\n    raise\nfinally:\n    print('org_babel_python_eoe')", "<string>")
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          ^
SyntaxError: invalid syntax
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfMembership' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfMembership' is not defined
>>>
2021-08-24org_babel_python_eoe
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfYPTdata' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfTrainingRpt' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 2, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfCompareSBwithTM' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfAdvData' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 1, in <module>
NameError: name 'dfCompareSBwithTM' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 3, in <module>
NameError: name 'dfScoutbookScoutsRB' is not defined
>>>
org_babel_python_eoe
Traceback (most recent call last):
  File "<string>", line 18, in __PYTHON_EL_eval
  File "<string>", line 8, in <module>
  File "<string>", line 22, in <module>
NameError: name 'dfAdvData' is not defined
>>>


[-- Attachment #2: Type: text/html, Size: 50519 bytes --]

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

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> I've attached a patch that reverts to using temporary files for
> sufficiently long strings (would this magic 4096 ever require
> customization?).  The patch also solves bug#32042, which is mostly
> unrelated.

Thanks; pushed to Emacs 28 (and I guess I'll close bug#32042, then).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> I've attached a patch that reverts to using temporary files for
> sufficiently long strings (would this magic 4096 ever require
> customization?).  The patch also solves bug#32042, which is mostly
> unrelated.

Thanks; pushed to Emacs 28 (and I guess I'll close bug#32042, then).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> I've attached a patch that reverts to using temporary files for
> sufficiently long strings (would this magic 4096 ever require
> customization?).  The patch also solves bug#32042, which is mostly
> unrelated.

Thanks; pushed to Emacs 28 (and I guess I'll close bug#32042, then).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Augusto Stoffel]

On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com> wrote:

> Today I was trying to export my monthly org document that uses org babel python blocks to
> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
> - Change Python eval to send directly instead of using temporary files.
>
> Below I show the python session buffer, first where it is working and then where it is broken for
> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>
> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
> understand the problem. Any ideas on what other information I can gather that would be
> useful?

I can run all the examples in
https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-python.html
after the changes of e32c7d2a8d.

Can you provide a small example to reproduce the problem you encountered?

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

[Augusto Stoffel]

On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com> wrote:

> Today I was trying to export my monthly org document that uses org babel python blocks to
> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
> - Change Python eval to send directly instead of using temporary files.
>
> Below I show the python session buffer, first where it is working and then where it is broken for
> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>
> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
> understand the problem. Any ideas on what other information I can gather that would be
> useful?

Two more things you could try:

1) set `python-shell-completion-native-enable' to nil
2) pull the current master, after commit 1fdd898704

I'd be curious to see the effect of 1) before doing 2), if you have the
time for this quick test.

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

[Augusto Stoffel]

On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com> wrote:

> Today I was trying to export my monthly org document that uses org babel python blocks to
> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
> - Change Python eval to send directly instead of using temporary files.
>
> Below I show the python session buffer, first where it is working and then where it is broken for
> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>
> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
> understand the problem. Any ideas on what other information I can gather that would be
> useful?

Two more things you could try:

1) set `python-shell-completion-native-enable' to nil
2) pull the current master, after commit 1fdd898704

I'd be curious to see the effect of 1) before doing 2), if you have the
time for this quick test.

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

[Andreas Schwab]

On Sep 04 2021, Augusto Stoffel wrote:

> I've attached a patch that reverts to using temporary files for
> sufficiently long strings (would this magic 4096 ever require
> customization?).

On BSD-like systems, the limit is 1024.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Mark Barton]

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



> On Sep 5, 2021, at 1:10 AM, Augusto Stoffel <arstoffel@gmail.com> wrote:
> 
> On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com> wrote:
> 
>> Today I was trying to export my monthly org document that uses org babel python blocks to
>> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
>> - Change Python eval to send directly instead of using temporary files.
>> 
>> Below I show the python session buffer, first where it is working and then where it is broken for
>> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>> 
>> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
>> understand the problem. Any ideas on what other information I can gather that would be
>> useful?
> 
> Two more things you could try:
> 
> 1) set `python-shell-completion-native-enable' to nil
> 2) pull the current master, after commit 1fdd898704
> 
> I'd be curious to see the effect of 1) before doing 2), if you have the
> time for this quick test.


I created a test org and csv file to test. Before I added the inline call to the test file, changing the python-shell-completion-native-enable to nil did get the my def pd2org to work. Otherwise it would work on the second export if I did not kill the python-chain session. The inline call to file_date returns an error “Inline error: multiline result cannot be used”

The attached pdf shows the successful export when I use the previous python.el.

I just compiled from master at commit c5b654b3f1

Recent commits
c5b654b3f1 master origin/master Autoload cl-struct-slot-info
7c7a47b86e ; * etc/NEWS: Fix a recent change.
3d0276e98b Improve the documentation around the read-key/minibuffer prompting
73a90cda4a Clarify completion-list-mode NEWS entry
8f2e8add98 ; * doc/emacs/maintaining.texi (Looking Up Identifiers): Fix last change.
0972cbe42f * lisp/progmodes/xref.el: Fix defcustoms (bug#50067)
2ed2999ce5 Improve documentation of new Xref options
1fdd898704 Fixes for 'python-shell-send-string' and 'python-shell-send-file'
ba84ec8bd9 Fix error handling in 'ispell-init-process'
e6f936eb4d Cleanup tramp-tests.el

Setting the python-shell-completion-native-enable to nil cleans up the session buffer python-chain

Python 3.9.6 (default, Jun 29 2021, 05:25:02) 
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> 
org_babel_python_eoe
>>> 
2021-08-24org_babel_python_eoe
>>> 
org_babel_python_eoe
>>> 
org_babel_python_eoe
>>> 
2021-09-05org_babel_python_eoe
>>> 

But I now get the error that prevents the pdf export: “Inline error: multiline result cannot be used"

[-- Attachment #2.1: Type: text/html, Size: 4383 bytes --]

[-- Attachment #2.2: ts_org_babel_python.csv --]
[-- Type: text/csv, Size: 23 bytes --]

1, one
2, two
3, three

[-- Attachment #2.3: Type: text/html, Size: 197 bytes --]

[-- Attachment #2.4: ts_org_babel_python.org --]
[-- Type: application/octet-stream, Size: 1782 bytes --]

#+TITLE: Test Org Babel Export with Python Blocks
#+AUTHOR: Mark Barton
#+DATE: 2021-09-05
#+OPTIONS: toc:nil H:4 num:nil ^:nil
#+EXCLUDE_TAGS: ignore
#+LATEX_CLASS: article
#+LaTeX_HEADER: \usepackage[letterpaper,margin=0.5in, bottom=1in]{geometry}
#+latex_header: \usepackage{float}
#+LaTeX_HEADER: \usepackage{enumitem}\setlist[itemize]{nosep}
#+LaTeX_HEADER: \usepackage{needspace}
#+LaTeX_HEADER: \usepackage{booktabs}
#+LaTeX_HEADER: \usepackage{xcolor}
#+LaTeX_HEADER: \let\OldRule\rule
#+LaTeX_HEADER: \renewcommand{\rule}[2]{\OldRule{\linewidth}{#2}}
#+LaTeX_HEADER: \setlength{\parindent}{0pt}
#+LaTeX_HEADER: \setlength{\parskip}{12pt}
#+PROPERTY: header-args:python :session python-chain :exports results :results raw replace

      
*** Initialize python                                              :ignore:
#+BEGIN_SRC python
import pandas as pd
from tabulate import tabulate

#+END_SRC

#+RESULTS:

#+NAME: file_date
#+begin_src python :var file="Roster_Report.csv" :exports none
import os
import time
print(time.strftime("%Y-%m-%d",time.localtime(os.path.getmtime(file))), end="")
#+end_src


*** Process downloaded csv files with Python Pandas for custom reports :ignore:
#+begin_src python :exports none
def pd2org(dataframe):
    return tabulate(dataframe, headers="keys", tablefmt="orgtbl", showindex="always")


dfTest = pd.read_csv("./ts_org_babel_python.csv")
#+end_src

*** This is a test                                                 :ignore:
#+NAME: Test_Table
#+begin_src python 
pd2org(dfTest)

#+end_src

*** Summary
Include multiple source blocks and an inline call to create data for document.

#+ATTR_LATEX: :placement [H]
#+caption: Test Table call_file_date[:results output :results raw](file="./ts_org_babel_python.csv")
#+RESULTS: Test_Table


[-- Attachment #2.5: Type: text/html, Size: 197 bytes --]

[-- Attachment #2.6: ts_org_babel_python.pdf --]
[-- Type: application/pdf, Size: 58501 bytes --]

[-- Attachment #2.7: Type: text/html, Size: 206 bytes --]

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

[Mark Barton]

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



> On Sep 5, 2021, at 10:18 AM, Mark Barton <mbarton98@gmail.com> wrote:
> 
> 
> 
>> On Sep 5, 2021, at 1:10 AM, Augusto Stoffel <arstoffel@gmail.com <mailto:arstoffel@gmail.com>> wrote:
>> 
>> On Sun,  5 Sep 2021 at 06:13, "Barton, Mark" <Mark.Barton@disney.com <mailto:Mark.Barton@disney.com>> wrote:
>> 
>>> Today I was trying to export my monthly org document that uses org babel python blocks to
>>> produce tables. I can get it to export fine if I revert python.el to the version before e32c7d2a8d
>>> - Change Python eval to send directly instead of using temporary files.
>>> 
>>> Below I show the python session buffer, first where it is working and then where it is broken for
>>> me. I’m running Emacs on macOS 11.5.1 and compile from the master branch.
>>> 
>>> In the “Broken session buffer” below, I recognize the code from that commit, but I really don’t
>>> understand the problem. Any ideas on what other information I can gather that would be
>>> useful?
>> 
>> Two more things you could try:
>> 
>> 1) set `python-shell-completion-native-enable' to nil
>> 2) pull the current master, after commit 1fdd898704
>> 
>> I'd be curious to see the effect of 1) before doing 2), if you have the
>> time for this quick test.
> 
> 
> I created a test org and csv file to test. Before I added the inline call to the test file, changing the python-shell-completion-native-enable to nil did get the my def pd2org to work. Otherwise it would work on the second export if I did not kill the python-chain session. The inline call to file_date returns an error “Inline error: multiline result cannot be used”
> 
> The attached pdf shows the successful export when I use the previous python.el.
> 
> I just compiled from master at commit c5b654b3f1
> 
> Recent commits
> c5b654b3f1 master origin/master Autoload cl-struct-slot-info
> 7c7a47b86e ; * etc/NEWS: Fix a recent change.
> 3d0276e98b Improve the documentation around the read-key/minibuffer prompting
> 73a90cda4a Clarify completion-list-mode NEWS entry
> 8f2e8add98 ; * doc/emacs/maintaining.texi (Looking Up Identifiers): Fix last change.
> 0972cbe42f * lisp/progmodes/xref.el: Fix defcustoms (bug#50067)
> 2ed2999ce5 Improve documentation of new Xref options
> 1fdd898704 Fixes for 'python-shell-send-string' and 'python-shell-send-file'
> ba84ec8bd9 Fix error handling in 'ispell-init-process'
> e6f936eb4d Cleanup tramp-tests.el
> 
> Setting the python-shell-completion-native-enable to nil cleans up the session buffer python-chain
> 
> Python 3.9.6 (default, Jun 29 2021, 05:25:02) 
> [Clang 12.0.5 (clang-1205.0.22.9)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> 
> org_babel_python_eoe
> >>> 
> 2021-08-24org_babel_python_eoe
> >>> 
> org_babel_python_eoe
> >>> 
> org_babel_python_eoe
> >>> 
> 2021-09-05org_babel_python_eoe
> >>> 
> 
> But I now get the error that prevents the pdf export: “Inline error: multiline result cannot be used"
> <ts_org_babel_python.csv>
> <ts_org_babel_python.org>
> <ts_org_babel_python.pdf>

Here is a simpler one to attempt to reproduce. If I comment out the #+PROPERTY line to not use a session then it works. With the #+PROPERTY line, I get the “Inline error: multiline result cannot be used”.


#+TITLE: Test Org Babel Export with Python Blocks
#+AUTHOR: Mark Barton
#+DATE: 2021-09-05
#+OPTIONS: toc:nil H:4 num:nil ^:nil
#+EXCLUDE_TAGS: ignore
#+LATEX_CLASS: article
#+LaTeX_HEADER: \usepackage[letterpaper,margin=0.5in, bottom=1in]{geometry}
#+latex_header: \usepackage{float}
#+LaTeX_HEADER: \usepackage{enumitem}\setlist[itemize]{nosep}
#+LaTeX_HEADER: \usepackage{needspace}
#+LaTeX_HEADER: \usepackage{booktabs}
#+LaTeX_HEADER: \usepackage{xcolor}
#+LaTeX_HEADER: \let\OldRule\rule
#+LaTeX_HEADER: \renewcommand{\rule}[2]{\OldRule{\linewidth}{#2}}
#+LaTeX_HEADER: \setlength{\parindent}{0pt}
#+LaTeX_HEADER: \setlength{\parskip}{12pt}
#+PROPERTY: header-args:python :session python-chain :exports results :results raw replace

      
*** Initialize python                                              :ignore:

#+NAME: file_date
#+begin_src python :var file="Roster_Report.csv" :exports none
import os
import time
print(time.strftime("%Y-%m-%d",time.localtime(os.path.getmtime(file))), end="")
#+end_src


*** Summary

Test inline call_file_date[:results output :results raw](file="./ts_org_babel_python.csv")


[-- Attachment #2: Type: text/html, Size: 8078 bytes --]

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

[Augusto Stoffel]

On Sun,  5 Sep 2021 at 10:18, Mark Barton <mbarton98@gmail.com> wrote:

> I created a test org and csv file to test. Before I added the inline call to the test file, changing
> the python-shell-completion-native-enable to nil did get the my def pd2org to work. Otherwise
> it would work on the second export if I did not kill the python-chain session. The inline call to
> file_date returns an error “Inline error: multiline result cannot be
> used”

Then I'm pretty sure this problem has to do with line length limits in
the TTY, as Stefan had already alerted.

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

[Augusto Stoffel]

On Sun,  5 Sep 2021 at 10:18, Mark Barton <mbarton98@gmail.com> wrote:

> I created a test org and csv file to test. Before I added the inline call to the test file, changing
> the python-shell-completion-native-enable to nil did get the my def pd2org to work. Otherwise
> it would work on the second export if I did not kill the python-chain session. The inline call to
> file_date returns an error “Inline error: multiline result cannot be
> used”

Then I'm pretty sure this problem has to do with line length limits in
the TTY, as Stefan had already alerted.

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

[Augusto Stoffel]

On Sun,  5 Sep 2021 at 18:36, Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Sep 04 2021, Augusto Stoffel wrote:
>
>> I've attached a patch that reverts to using temporary files for
>> sufficiently long strings (would this magic 4096 ever require
>> customization?).
>
> On BSD-like systems, the limit is 1024.
>
> Andreas.

Okay, so is the TTY line length limit of the OS available in Lisp
somewhere?  Otherwise, can we at least trust that 1024 is the universal
lower bound?

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

[Michael Albinus]

Augusto Stoffel <arstoffel@gmail.com> writes:

> Okay, so is the TTY line length limit of the OS available in Lisp
> somewhere?  Otherwise, can we at least trust that 1024 is the universal
> lower bound?

On systems with a POSIX shell:

--8<---------------cut here---------------start------------->8---
(let ((default-directory "/"))
  (read (shell-command-to-string "getconf LINE_MAX")))
--8<---------------cut here---------------end--------------->8---

Note, that this can differ when you connect to remote hosts. That's why
I have bound default-directory to a local file name in the example.

Best regards, Michael.

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

[Michael Albinus]

Augusto Stoffel <arstoffel@gmail.com> writes:

> Okay, so is the TTY line length limit of the OS available in Lisp
> somewhere?  Otherwise, can we at least trust that 1024 is the universal
> lower bound?

On systems with a POSIX shell:

--8<---------------cut here---------------start------------->8---
(let ((default-directory "/"))
  (read (shell-command-to-string "getconf LINE_MAX")))
--8<---------------cut here---------------end--------------->8---

Note, that this can differ when you connect to remote hosts. That's why
I have bound default-directory to a local file name in the example.

Best regards, Michael.

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

[Andreas Schwab]

On Sep 06 2021, Michael Albinus wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> Okay, so is the TTY line length limit of the OS available in Lisp
>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>> lower bound?
>
> On systems with a POSIX shell:
>
> (let ((default-directory "/"))
>   (read (shell-command-to-string "getconf LINE_MAX")))

LINE_MAX has nothing to do with the tty line discipline.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Andreas Schwab]

On Sep 06 2021, Michael Albinus wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> Okay, so is the TTY line length limit of the OS available in Lisp
>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>> lower bound?
>
> On systems with a POSIX shell:
>
> (let ((default-directory "/"))
>   (read (shell-command-to-string "getconf LINE_MAX")))

LINE_MAX has nothing to do with the tty line discipline.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Michael Albinus]

Andreas Schwab <schwab@linux-m68k.org> writes:

>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>> lower bound?
>>
>> On systems with a POSIX shell:
>>
>> (let ((default-directory "/"))
>>   (read (shell-command-to-string "getconf LINE_MAX")))
>
> LINE_MAX has nothing to do with the tty line discipline.

I'll never learn it :-(

We've discussed this issue some years ago already. IIRC, the outcome was
to use "getconf PIPE_BUF /" instead.

> Andreas.

Best regards, Michael.

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

[Michael Albinus]

Andreas Schwab <schwab@linux-m68k.org> writes:

>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>> lower bound?
>>
>> On systems with a POSIX shell:
>>
>> (let ((default-directory "/"))
>>   (read (shell-command-to-string "getconf LINE_MAX")))
>
> LINE_MAX has nothing to do with the tty line discipline.

I'll never learn it :-(

We've discussed this issue some years ago already. IIRC, the outcome was
to use "getconf PIPE_BUF /" instead.

> Andreas.

Best regards, Michael.

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

[Andreas Schwab]

On Sep 06 2021, Michael Albinus wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>> lower bound?
>>>
>>> On systems with a POSIX shell:
>>>
>>> (let ((default-directory "/"))
>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>
>> LINE_MAX has nothing to do with the tty line discipline.
>
> I'll never learn it :-(
>
> We've discussed this issue some years ago already. IIRC, the outcome was
> to use "getconf PIPE_BUF /" instead.

Neither has PIPE_BUF.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Michael Albinus]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Neither has PIPE_BUF.

Great that you always say what's not working. Perhaps you have a better
answer to Augusto?

> Andreas.

Best regards, Michael.

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

[Augusto Stoffel]

On Mon,  6 Sep 2021 at 14:00, Michael Albinus <michael.albinus@gmx.de> wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Neither has PIPE_BUF.
>
> Great that you always say what's not working. Perhaps you have a better
> answer to Augusto?
>
>> Andreas.
>
> Best regards, Michael.

Thanks for the ideas.  If there's no simple and platform-independent
way to obtain the line length limit in a PTY, then I think there's no
reason to attempt anything fancy in python.el (or any derived comint
mode, for that matter).

So the options seem to be: either hard-code a limit of 1024 chars, or
create a defcustom with a platform-dependent default.  But such a
defcustom should probably be part of comint.el, and should be added by
someone who (unlike me) actually understands this stuff.

Working around a limit of less than 1024 characters is too awkward, so I
hope this is a safe assumption.  The juggling of temp files that existed
in python.el for many years already required lines of ~350 chars,
depending on how the OS names temp files.

Sorry for introducing a bug in master.  I think I got it fixed here, and
I'll submit the patch when I'm confident about it.

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

[Augusto Stoffel]

On Mon,  6 Sep 2021 at 14:00, Michael Albinus <michael.albinus@gmx.de> wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Neither has PIPE_BUF.
>
> Great that you always say what's not working. Perhaps you have a better
> answer to Augusto?
>
>> Andreas.
>
> Best regards, Michael.

Thanks for the ideas.  If there's no simple and platform-independent
way to obtain the line length limit in a PTY, then I think there's no
reason to attempt anything fancy in python.el (or any derived comint
mode, for that matter).

So the options seem to be: either hard-code a limit of 1024 chars, or
create a defcustom with a platform-dependent default.  But such a
defcustom should probably be part of comint.el, and should be added by
someone who (unlike me) actually understands this stuff.

Working around a limit of less than 1024 characters is too awkward, so I
hope this is a safe assumption.  The juggling of temp files that existed
in python.el for many years already required lines of ~350 chars,
depending on how the OS names temp files.

Sorry for introducing a bug in master.  I think I got it fixed here, and
I'll submit the patch when I'm confident about it.

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

[Andy Moreton]

On Mon 06 Sep 2021, Andreas Schwab wrote:

> On Sep 06 2021, Michael Albinus wrote:
>
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>
>>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>>> lower bound?
>>>>
>>>> On systems with a POSIX shell:
>>>>
>>>> (let ((default-directory "/"))
>>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>>
>>> LINE_MAX has nothing to do with the tty line discipline.
>>
>> I'll never learn it :-(
>>
>> We've discussed this issue some years ago already. IIRC, the outcome was
>> to use "getconf PIPE_BUF /" instead.
>
> Neither has PIPE_BUF.
>
> Andreas.

Why not stop being so unhelpful and actually share your knowledge of
the correct answer ?

What precisely is the limitation, and how can it be discovered ?

Please explain.

    AndyM

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

[Andreas Schwab]

On Sep 06 2021, Andy Moreton wrote:

> On Mon 06 Sep 2021, Andreas Schwab wrote:
>
>> On Sep 06 2021, Michael Albinus wrote:
>>
>>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>>
>>>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>>>> lower bound?
>>>>>
>>>>> On systems with a POSIX shell:
>>>>>
>>>>> (let ((default-directory "/"))
>>>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>>>
>>>> LINE_MAX has nothing to do with the tty line discipline.
>>>
>>> I'll never learn it :-(
>>>
>>> We've discussed this issue some years ago already. IIRC, the outcome was
>>> to use "getconf PIPE_BUF /" instead.
>>
>> Neither has PIPE_BUF.
>>
>> Andreas.
>
> Why not stop being so unhelpful and actually share your knowledge of
> the correct answer ?

Why not stop being so unhelpful and actually share your knowledge of the
correct answer?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Augusto Stoffel]

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

On Mon,  6 Sep 2021 at 13:53, Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Sep 06 2021, Michael Albinus wrote:
>
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>
>>>>> Okay, so is the TTY line length limit of the OS available in Lisp
>>>>> somewhere?  Otherwise, can we at least trust that 1024 is the universal
>>>>> lower bound?
>>>>
>>>> On systems with a POSIX shell:
>>>>
>>>> (let ((default-directory "/"))
>>>>   (read (shell-command-to-string "getconf LINE_MAX")))
>>>
>>> LINE_MAX has nothing to do with the tty line discipline.
>>
>> I'll never learn it :-(
>>
>> We've discussed this issue some years ago already. IIRC, the outcome was
>> to use "getconf PIPE_BUF /" instead.
>
> Neither has PIPE_BUF.
>
> Andreas.

Okay then. Since there seem to be no better alternatives, I have
attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
some day someone adds a variable specifying a more precise limit, then
we can change this.

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.

It would be great if someone with access to a BSD-like OS could test
this.  I can only test locally on Linux and over ssh between Linux
machines.

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?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch --]
[-- Type: text/x-patch, Size: 3680 bytes --]

From 05947ad82219af3f4bcb228f076995f181a7255d Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Mon, 6 Sep 2021 23:34:48 +0200
Subject: [PATCH] Reduce limit of line length sent to the Python inferior

* 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-send-string-no-output): Revert changes of e32c7d2a8d
(python-shell-send-string): Assume a smaller TTY line length limit,
as required by some OSes.
(python-shell-send-file): Ditto.
---
 lisp/progmodes/python.el | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d8ec032402..347c3ffe00 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2826,6 +2826,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))))))
@@ -3128,12 +3137,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) 1024)
         (comint-send-string process code)
       (let* ((temp-file-name (with-current-buffer (process-buffer process)
                                (python-shell--save-temp-file string)))
@@ -3180,8 +3188,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
@@ -3419,9 +3426,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

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

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Tue, 07 Sep 2021 19:37:41 +0200
> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>  Andreas Schwab <schwab@linux-m68k.org>,
>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
> 
> Okay then. Since there seem to be no better alternatives, I have
> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
> some day someone adds a variable specifying a more precise limit, then
> we can change this.

I thought the conclusion was that we do know the limits, but they are
different on different OSes?  If so, why not use the correct limit for
each OS, instead of using the most strict limit?  The underlying OS is
available in system-type.

> -    (if (<= (string-bytes code) 4096)
> +    (if (<= (string-bytes code) 1024)

In any case, IMO this should be a defconst with a suitable doc string,
so that we'd have this stuff documented for posterity.

Thanks.

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

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Tue, 07 Sep 2021 19:37:41 +0200
> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>  Andreas Schwab <schwab@linux-m68k.org>,
>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
> 
> Okay then. Since there seem to be no better alternatives, I have
> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
> some day someone adds a variable specifying a more precise limit, then
> we can change this.

I thought the conclusion was that we do know the limits, but they are
different on different OSes?  If so, why not use the correct limit for
each OS, instead of using the most strict limit?  The underlying OS is
available in system-type.

> -    (if (<= (string-bytes code) 4096)
> +    (if (<= (string-bytes code) 1024)

In any case, IMO this should be a defconst with a suitable doc string,
so that we'd have this stuff documented for posterity.

Thanks.

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

[Augusto Stoffel]

On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Tue, 07 Sep 2021 19:37:41 +0200
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>>  Andreas Schwab <schwab@linux-m68k.org>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
>> 
>> Okay then. Since there seem to be no better alternatives, I have
>> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
>> some day someone adds a variable specifying a more precise limit, then
>> we can change this.
>
> I thought the conclusion was that we do know the limits, but they are
> different on different OSes?  If so, why not use the correct limit for
> each OS, instead of using the most strict limit?  The underlying OS is
> available in system-type.
>
>> -    (if (<= (string-bytes code) 4096)
>> +    (if (<= (string-bytes code) 1024)
>
> In any case, IMO this should be a defconst with a suitable doc string,
> so that we'd have this stuff documented for posterity.
>
> Thanks.

Yes, I agree this should be a defconst, defined in comint.el or maybe
even in C, if there turns out to be a sensible way to query the OS.

I'm afraid I've already proved that I am not up to this particular task :-)

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

[Augusto Stoffel]

On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Tue, 07 Sep 2021 19:37:41 +0200
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>>  Andreas Schwab <schwab@linux-m68k.org>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
>> 
>> Okay then. Since there seem to be no better alternatives, I have
>> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
>> some day someone adds a variable specifying a more precise limit, then
>> we can change this.
>
> I thought the conclusion was that we do know the limits, but they are
> different on different OSes?  If so, why not use the correct limit for
> each OS, instead of using the most strict limit?  The underlying OS is
> available in system-type.
>
>> -    (if (<= (string-bytes code) 4096)
>> +    (if (<= (string-bytes code) 1024)
>
> In any case, IMO this should be a defconst with a suitable doc string,
> so that we'd have this stuff documented for posterity.
>
> Thanks.

Yes, I agree this should be a defconst, defined in comint.el or maybe
even in C, if there turns out to be a sensible way to query the OS.

I'm afraid I've already proved that I am not up to this particular task :-)

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

[Augusto Stoffel]

On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Tue, 07 Sep 2021 19:37:41 +0200
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>>  Andreas Schwab <schwab@linux-m68k.org>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
>> 
>> Okay then. Since there seem to be no better alternatives, I have
>> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
>> some day someone adds a variable specifying a more precise limit, then
>> we can change this.
>
> I thought the conclusion was that we do know the limits, but they are
> different on different OSes?  If so, why not use the correct limit for
> each OS, instead of using the most strict limit?  The underlying OS is
> available in system-type.
>
>> -    (if (<= (string-bytes code) 4096)
>> +    (if (<= (string-bytes code) 1024)
>
> In any case, IMO this should be a defconst with a suitable doc string,
> so that we'd have this stuff documented for posterity.
>
> Thanks.

Another open question (at least to me) is what happens when you put an
SSH connection in the game.  Presumably it has some buffer limits as
well, and this seems even harder to determine exactly.

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

[Augusto Stoffel]

On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Tue, 07 Sep 2021 19:37:41 +0200
>> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
>>  Andreas Schwab <schwab@linux-m68k.org>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
>>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
>> 
>> Okay then. Since there seem to be no better alternatives, I have
>> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
>> some day someone adds a variable specifying a more precise limit, then
>> we can change this.
>
> I thought the conclusion was that we do know the limits, but they are
> different on different OSes?  If so, why not use the correct limit for
> each OS, instead of using the most strict limit?  The underlying OS is
> available in system-type.
>
>> -    (if (<= (string-bytes code) 4096)
>> +    (if (<= (string-bytes code) 1024)
>
> In any case, IMO this should be a defconst with a suitable doc string,
> so that we'd have this stuff documented for posterity.
>
> Thanks.

Another open question (at least to me) is what happens when you put an
SSH connection in the game.  Presumably it has some buffer limits as
well, and this seems even harder to determine exactly.

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

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: Mark.Barton@disney.com,  andrewjmoreton@gmail.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 19:59:06 +0200
> 
> On Tue,  7 Sep 2021 at 20:48, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> From: Augusto Stoffel <arstoffel@gmail.com>
> >> Date: Tue, 07 Sep 2021 19:37:41 +0200
> >> Cc: Andy Moreton <andrewjmoreton@gmail.com>, emacs-devel@gnu.org,
> >>  Andreas Schwab <schwab@linux-m68k.org>,
> >>  Stefan Monnier <monnier@iro.umontreal.ca>, 49822@debbugs.gnu.org,
> >>  Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>
> >> 
> >> Okay then. Since there seem to be no better alternatives, I have
> >> attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
> >> some day someone adds a variable specifying a more precise limit, then
> >> we can change this.
> >
> > I thought the conclusion was that we do know the limits, but they are
> > different on different OSes?  If so, why not use the correct limit for
> > each OS, instead of using the most strict limit?  The underlying OS is
> > available in system-type.
> >
> >> -    (if (<= (string-bytes code) 4096)
> >> +    (if (<= (string-bytes code) 1024)
> >
> > In any case, IMO this should be a defconst with a suitable doc string,
> > so that we'd have this stuff documented for posterity.
> >
> > Thanks.
> 
> Yes, I agree this should be a defconst, defined in comint.el or maybe
> even in C, if there turns out to be a sensible way to query the OS.
> 
> I'm afraid I've already proved that I am not up to this particular task :-)

I didn't mean to say anything like that, not at all.  Mine was a minor
comment, it's supposed to be easy to augment your patch to avoid those
minor issues.  Nothing complicated or fancy is expected, just a
defconst with a value conditioned on system-type.

As for where should that defconst live, I personally have no problem
with its living in python.el for now.  We can always move it later if
the need arises.

Thanks.

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

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 20:13:36 +0200
> 
> >> -    (if (<= (string-bytes code) 4096)
> >> +    (if (<= (string-bytes code) 1024)
> >
> > In any case, IMO this should be a defconst with a suitable doc string,
> > so that we'd have this stuff documented for posterity.
> >
> > Thanks.
> 
> Another open question (at least to me) is what happens when you put an
> SSH connection in the game.  Presumably it has some buffer limits as
> well, and this seems even harder to determine exactly.

SSH connection between what and what?

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

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 20:13:36 +0200
> 
> >> -    (if (<= (string-bytes code) 4096)
> >> +    (if (<= (string-bytes code) 1024)
> >
> > In any case, IMO this should be a defconst with a suitable doc string,
> > so that we'd have this stuff documented for posterity.
> >
> > Thanks.
> 
> Another open question (at least to me) is what happens when you put an
> SSH connection in the game.  Presumably it has some buffer limits as
> well, and this seems even harder to determine exactly.

SSH connection between what and what?

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

[Augusto Stoffel]

On Tue,  7 Sep 2021 at 21:31, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>>   michael.albinus@gmx.de,  larsi@gnus.org
>> Date: Tue, 07 Sep 2021 20:13:36 +0200
>> 
>> >> -    (if (<= (string-bytes code) 4096)
>> >> +    (if (<= (string-bytes code) 1024)
>> >
>> > In any case, IMO this should be a defconst with a suitable doc string,
>> > so that we'd have this stuff documented for posterity.
>> >
>> > Thanks.
>> 
>> Another open question (at least to me) is what happens when you put an
>> SSH connection in the game.  Presumably it has some buffer limits as
>> well, and this seems even harder to determine exactly.
>
> SSH connection between what and what?

I'm sitting at machine A but my inferior process is running on
machine B and the machines talk via SSH.

There are several steps in the communication here: Emacs -> SSH client
-> SSH server -> inferior process.  Each step has potential limitations.
If the SSH server talks to the inferior process via a PTY (I'm just
pondering, and I don't know if this is usual setup or not), then we
would have to take machine B's PTY restrictions into account, which
would defeat the purpose of knowing the exact characteristics of machine
A's PTYs.

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

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 21:00:28 +0200
> 
> >> Another open question (at least to me) is what happens when you put an
> >> SSH connection in the game.  Presumably it has some buffer limits as
> >> well, and this seems even harder to determine exactly.
> >
> > SSH connection between what and what?
> 
> I'm sitting at machine A but my inferior process is running on
> machine B and the machines talk via SSH.

You mean, you run a remote subprocess via Tramp?

> There are several steps in the communication here: Emacs -> SSH client
> -> SSH server -> inferior process.  Each step has potential limitations.
> If the SSH server talks to the inferior process via a PTY (I'm just
> pondering, and I don't know if this is usual setup or not), then we
> would have to take machine B's PTY restrictions into account, which
> would defeat the purpose of knowing the exact characteristics of machine
> A's PTYs.

I don't see how B's PTY restrictions are relevant, but maybe Micheal
will (assuming you are talking about remote subprocesses).

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

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: andrewjmoreton@gmail.com,  Mark.Barton@disney.com,  emacs-devel@gnu.org,
>   schwab@linux-m68k.org,  monnier@iro.umontreal.ca,  49822@debbugs.gnu.org,
>   michael.albinus@gmx.de,  larsi@gnus.org
> Date: Tue, 07 Sep 2021 21:00:28 +0200
> 
> >> Another open question (at least to me) is what happens when you put an
> >> SSH connection in the game.  Presumably it has some buffer limits as
> >> well, and this seems even harder to determine exactly.
> >
> > SSH connection between what and what?
> 
> I'm sitting at machine A but my inferior process is running on
> machine B and the machines talk via SSH.

You mean, you run a remote subprocess via Tramp?

> There are several steps in the communication here: Emacs -> SSH client
> -> SSH server -> inferior process.  Each step has potential limitations.
> If the SSH server talks to the inferior process via a PTY (I'm just
> pondering, and I don't know if this is usual setup or not), then we
> would have to take machine B's PTY restrictions into account, which
> would defeat the purpose of knowing the exact characteristics of machine
> A's PTYs.

I don't see how B's PTY restrictions are relevant, but maybe Micheal
will (assuming you are talking about remote subprocesses).

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

[Barton, Mark]

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



On Sep 7, 2021, at 10:37 AM, Augusto Stoffel <arstoffel@gmail.com<mailto:arstoffel@gmail.com>> wrote:

On Mon,  6 Sep 2021 at 13:53, Andreas Schwab <schwab@linux-m68k.org<mailto:schwab@linux-m68k.org>> wrote:

On Sep 06 2021, Michael Albinus wrote:

Andreas Schwab <schwab@linux-m68k.org<mailto:schwab@linux-m68k.org>> writes:

Okay, so is the TTY line length limit of the OS available in Lisp
somewhere?  Otherwise, can we at least trust that 1024 is the universal
lower bound?

On systems with a POSIX shell:

(let ((default-directory "/"))
 (read (shell-command-to-string "getconf LINE_MAX")))

LINE_MAX has nothing to do with the tty line discipline.

I'll never learn it :-(

We've discussed this issue some years ago already. IIRC, the outcome was
to use "getconf PIPE_BUF /" instead.

Neither has PIPE_BUF.

Andreas.

Okay then. Since there seem to be no better alternatives, I have
attached a new patch reducing the limit to a hard-coded 1024 bytes.  If
some day someone adds a variable specifying a more precise limit, then
we can change this.

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.

It would be great if someone with access to a BSD-like OS could test
this.  I can only test locally on Linux and over ssh between Linux
machines.

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?

<0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch>

I tested the patch, 0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch, on macOS 11.5.1 and although my minimal example runs, my original document using org babel session with multiple python blocks is failing with the message in the mini buffer “Inline error: multiline result cannot be used”. I can try some lower value than 1024 and report back, but below is what I saw with attempting to use the debugger to see what the string looks like in my use case.

Setting debug-on-entry to comint-send-string and running a src block, I get:

Debugger entered--returning value: nil
  comint-send-string(#<process filedate> "__PYTHON_EL_eval_file(\"/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/pybQ7oBh\", \"/var/folders/kf/...")
  python-shell-send-file("/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/p..." #<process filedate> "/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/p..." t)
  python-shell-send-string("try:\n    import ast\n    with open('/var/folders/kf...")
  org-babel-python--send-string("*filedate*" "import ast\nwith open('/var/folders/kf/zdpzgs9d30b3...")
  org-babel-python-evaluate-session("*filedate*" "file=\"Roster_Report.csv\"\nimport os\nimport time\npri..." value ("raw" "replace"))
  org-babel-python-evaluate("*filedate*" "file=\"Roster_Report.csv\"\nimport os\nimport time\npri..." value ("raw" "replace") nil)
  org-babel-execute:python("import os\nimport time\nprint(time.strftime(\"%Y-%m-%..." ((:var file . "Roster_Report.csv") (:colname-names) (:rowname-names) (:result-params "raw" "replace") (:result-type . value) (:results . "raw replace") (:exports . "results") (:cache . "no") (:noweb . "no") (:hlines . "no") (:async . "yes") (:tangle . "reports.py") (:comments . "both") (:session . "filedate")))
  org-babel-execute-src-block(nil ("python" "import os\nimport time\nprint(time.strftime(\"%Y-%m-%..." ((:var file . "Roster_Report.csv") (:colname-names) (:rowname-names) (:result-params "replace" "raw") (:result-type . value) (:results . "replace raw") (:exports . "results") (:session . "filedate") (:comments . "both") (:tangle . "reports.py") (:async . "yes") (:hlines . "no") (:noweb . "no") (:cache . "no")) "" "file_date" 7735 "(ref:%s)"))
  org-ctrl-c-ctrl-c(nil)
  funcall-interactively(org-ctrl-c-ctrl-c nil)
  command-execute(org-ctrl-c-ctrl-c)

If I look at the file passed, pybQ7oBh, the contents looks like it will open another file.

try:
    import ast
    with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-K4MbU2/python-hNII4l') as __org_babel_python_tmpfile:
        __org_babel_python_ast = ast.parse(__org_babel_python_tmpfile.read())
    __org_babel_python_final = __org_babel_python_ast.body[-1]
    if isinstance(__org_babel_python_final, ast.Expr):
        __org_babel_python_ast.body = __org_babel_python_ast.body[:-1]
        exec(compile(__org_babel_python_ast, '<string>', 'exec'))
        __org_babel_python_final = eval(compile(ast.Expression(
            __org_babel_python_final.value), '<string>', 'eval'))
        with open('/var/folders/kf/zdpzgs9d30b3jj4bdkdjf1vw0000gn/T/babel-K4MbU2/python-ra4ysF', 'w') as __org_babel_python_tmpfile:
            if False:
                import pprint
                __org_babel_python_tmpfile.write(pprint.pformat(__org_babel_python_final))
            else:
                __org_babel_python_tmpfile.write(str(__org_babel_python_final))
    else:
        exec(compile(__org_babel_python_ast, '<string>', 'exec'))
        __org_babel_python_final = None
except:
    raise
finally:
    print('org_babel_python_eoe')

[-- Attachment #2: Type: text/html, Size: 10818 bytes --]

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

[Augusto Stoffel]

On Wed,  8 Sep 2021 at 03:17, "Barton, Mark" <Mark.Barton@disney.com> wrote:

> I tested the patch, 0001-Reduce-limit-of-line-length-sent-to-the-Python-infer.patch, on
> macOS 11.5.1 and although my minimal example runs, my original document using org babel
> session with multiple python blocks is failing with the message in the mini buffer “Inline error:
> multiline result cannot be used”. I can try some lower value than 1024 and report back, but
> below is what I saw with attempting to use the debugger to see what the string looks like in my
> use case.

This looks like a different matter, and the patch below might solve it.
But I think it would make sense to open a new bug for this point, to we
get a chance of getting through with the TTY line limit discussion in
this thread.

diff --git a/lisp/org/ob-python.el b/lisp/org/ob-python.el
index 7911205d08..e3714964b7 100644
--- a/lisp/org/ob-python.el
+++ b/lisp/org/ob-python.el
@@ -352,7 +352,7 @@ org-babel-python--send-string
 		   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-chomp (substring string-buffer 1 (match-beginning 0))))))
 
 (defun org-babel-python-evaluate-session
     (session body &optional result-type result-params)

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi,

>> I'm sitting at machine A but my inferior process is running on
>> machine B and the machines talk via SSH.
>
> You mean, you run a remote subprocess via Tramp?
>
>> There are several steps in the communication here: Emacs -> SSH client
>> -> SSH server -> inferior process.  Each step has potential limitations.
>> If the SSH server talks to the inferior process via a PTY (I'm just
>> pondering, and I don't know if this is usual setup or not), then we
>> would have to take machine B's PTY restrictions into account, which
>> would defeat the purpose of knowing the exact characteristics of machine
>> A's PTYs.
>
> I don't see how B's PTY restrictions are relevant, but maybe Micheal
> will (assuming you are talking about remote subprocesses).

Tramp tries to handle such cases. But of course, a respective test
wouldn't hurt. It could be on the machine which has shown the
limitations, accessing it via "/ssh::".

Best regards, Michael.

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi,

>> I'm sitting at machine A but my inferior process is running on
>> machine B and the machines talk via SSH.
>
> You mean, you run a remote subprocess via Tramp?
>
>> There are several steps in the communication here: Emacs -> SSH client
>> -> SSH server -> inferior process.  Each step has potential limitations.
>> If the SSH server talks to the inferior process via a PTY (I'm just
>> pondering, and I don't know if this is usual setup or not), then we
>> would have to take machine B's PTY restrictions into account, which
>> would defeat the purpose of knowing the exact characteristics of machine
>> A's PTYs.
>
> I don't see how B's PTY restrictions are relevant, but maybe Micheal
> will (assuming you are talking about remote subprocesses).

Tramp tries to handle such cases. But of course, a respective test
wouldn't hurt. It could be on the machine which has shown the
limitations, accessing it via "/ssh::".

Best regards, Michael.

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

[Lars Ingebrigtsen]

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'.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

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'.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Augusto Stoffel]

[-- 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

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

[Augusto Stoffel]

[-- 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

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

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

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

Thanks; applied to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no