bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[pmercatoris]

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

Hello,

First of all, this is my first bug report, I'm not sure I am doing this
correctly, so forgive my ignorance.

I am unable to get correct behavious when sending a region from 
indented code
to the python shell. Consider this python code:

```python
if True:
    if True:
        a = "test"
```

If I select the first `True` and `python-shell-send-region`, it will
correctly send it to the consol and print `True`.

Now if I do the same thing with the second `True` it will throw an
error: `IndentationError: expected an indented block after 'if'
statement on line 1` Although it would also reffer to unexpected block
after `if` even it it were `for`, `try`, ...

If I select the `a` or `a = "test"` it will correctly send it to the
console, however it won't echo the evaluation of the statement.

If I select the string "test" and send, it will throw the same error as
the second True.

My python config is very minimal:

  (use-package python
    :ensure nil
    :mode
    ("\\.py\\'" . python-mode)

    :init
    (setq-default indent-tabs-mode nil)

    :config
    (setq python-indent-offset 4
          python-indent-guess-indent-offset-verbose nil
          )

    (setq python-shell-interpreter "jupyter"
          python-shell-interpreter-args "console --simple-prompt"
          python-shell-prompt-detect-failure-warning nil)

    (add-to-list 'python-shell-completion-native-disabled-interpreters
                 "jupyter"))

Kind regards,
Pierre

In GNU Emacs 28.1 (build 1, x86_64-redhat-linux-gnu, GTK+ Version 
3.24.34, cairo version 1.17.6)
 of 2022-07-21 built on buildvm-x86-04.iad2.fedoraproject.org
Windowing system distributor 'The X.Org Foundation', version 
11.0.12201005
System Description: Fedora Linux 37 (Workstation Edition)

Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xpm --with-x-toolkit=gtk3 --with-gpm=no
 --with-xwidgets --with-modules --with-harfbuzz --with-cairo --with-json
 --with-native-compilation build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O2
 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches
 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
 -Wp,-D_GLIBCXX_ASSERTIONS
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection'
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

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

Important settings:
  value of $LC_MONETARY: en_GB.UTF-8
  value of $LC_NUMERIC: en_GB.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Python

Minor modes in effect:
  which-key-mode: t
  display-line-numbers-mode: t
  rainbow-delimiters-mode: t
  envrc-global-mode: t
  envrc-mode: t
  tree-sitter-hl-mode: t
  global-tree-sitter-mode: t
  tree-sitter-mode: t
  eglot--managed-mode: t
  flymake-mode: t
  marginalia-mode: t
  global-corfu-mode: t
  corfu-mode: t
  vertico-mode: t
  doom-modeline-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-evil-surround-mode: t
  evil-surround-mode: t
  global-evil-collection-unimpaired-mode: t
  evil-collection-unimpaired-mode: t
  shell-dirtrack-mode: t
  evil-mode: t
  evil-local-mode: t
  windmove-mode: t
  recentf-mode: t
  savehist-mode: t
  save-place-mode: t
  global-hl-line-mode: t
  electric-pair-mode: t
  delete-selection-mode: t
  override-global-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/pmercatoris/.config/emacs/elpa/transient-20221202.1727/transient 
hides /usr/share/emacs/28.1/lisp/transient

Features:
(shadow sort mail-extr emacsbug sendmail cua-base misearch multi-isearch
avy evil-nerd-commenter evil-nerd-commenter-operator
evil-nerd-commenter-sdk sgml-mode facemenu mule-util
evil-collection-markdown-mode markdown-mode vc-mtn vc-hg vc-bzr vc-src
vc-sccs vc-svn vc-cvs vc-rcs vc evil-collection-which-key which-key
face-remap display-line-numbers rainbow-delimiters envrc inheritenv
tree-sitter-langs tree-sitter-langs-build evil-collection-tar-mode
tar-mode evil-collection-arc-mode arc-mode archive-mode tree-sitter-hl
tree-sitter tree-sitter-load tree-sitter-cli tsc tsc-dyn tsc-dyn-get
dired-aux tsc-obsolete ob-python evil-collection-python python tramp-sh
tramp tramp-loaddefs trampver tramp-integration files-x tramp-compat
ls-lisp hydra lv consult-eglot evil-collection-consult consult-vertico
consult compat-28 evil-collection-bookmark bookmark
evil-collection-eglot eglot array filenotify jsonrpc evil-collection-ert
ert pp ewoc evil-collection-debug debug backtrace evil-collection-xref
xref evil-collection-flymake flymake-proc flymake
evil-collection-compile compile pcase project evil-collection-imenu
imenu orderless marginalia evil-collection-corfu corfu
evil-collection-vertico vertico doom-modeline doom-modeline-segments
doom-modeline-env doom-modeline-core shrink-path f f-shortdoc
evil-collection-shortdoc shortdoc s all-the-icons all-the-icons-faces
data-material data-weathericons data-octicons data-fileicons
data-faicons data-alltheicons rainbow-mode xterm-color color
ef-duo-dark-theme ef-themes undo-tree diff queue evil-surround
evil-collection-unimpaired evil-collection-vc-git
evil-collection-tabulated-list evil-collection-tab-bar
evil-collection-speedbar evil-collection-simple evil-collection-replace
evil-collection-process-menu evil-collection-package-menu
evil-collection-outline evil-collection-org evil-collection-info
evil-collection-indent evil-collection-image evil-collection-help
evil-collection-gnus evil-collection-eww evil-collection-epa
evil-collection-elisp-mode evil-collection-eldoc
evil-collection-doc-view evil-collection-dired evil-collection-diff-mode
evil-collection-comint evil-collection-calendar calc-ext
evil-collection-calc evil-collection-buff-menu
evil-collection-auto-package-update evil-collection annalist evil
evil-integration evil-maps evil-commands reveal flyspell ispell
evil-jumps evil-command-window evil-search evil-ex shell evil-types
evil-macros evil-repeat evil-states evil-core comp comp-cstr warnings
evil-common windmove calc calc-loaddefs calc-macs rect evil-digraphs
evil-vars edmacro kmacro recentf tree-widget savehist saveplace hl-line
elec-pair delsel no-littering compat compat-macs diary-lib
diary-loaddefs auto-package-update dash use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
use-package-core finder-inf vc-git diff-mode vc-dispatcher org-element
avl-tree generator ol-eww eww xdg url-queue thingatpt mm-url ol-rmail
ol-mhe ol-irc ol-info ol-gnus nnselect gnus-search eieio-opt speedbar
ezimage dframe gnus-art mm-uu mml2015 mm-view mml-smime smime dig
gnus-sum shr kinsoku svg dom gnus-group gnus-undo gnus-start gnus-dbus
dbus xml gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo parse-time
gnus-spec gnus-int gnus-range message rmc puny rfc822 mml mml-sec epa
derived epg rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader gnus-win gnus nnheader gnus-util
rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums text-property-search
mail-utils mm-util mail-prsvr wid-edit ol-docview doc-view jka-compr
image-mode exif dired dired-loaddefs ol-bibtex ol-bbdb ol-w3m ol-doi
org-link-doi cl-extra help-mode org ob ob-tangle ob-ref ob-lob ob-table
ob-exp org-macro org-footnote org-src ob-comint org-pcomplete pcomplete
comint ansi-color ring org-list org-faces org-entities noutline outline
easy-mmode org-version ob-emacs-lisp ob-core ob-eval org-table oc-basic
bibtex iso8601 time-date ol rx org-keys oc org-compat advice org-macs
org-loaddefs format-spec find-func cal-menu calendar cal-loaddefs info
package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode 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 lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax 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 emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice
button loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads xwidget-internal dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 773438 128306)
 (symbols 48 46851 49)
 (strings 32 207758 12187)
 (string-bytes 1 6926856)
 (vectors 16 89108)
 (vector-slots 8 2338965 214323)
 (floats 8 1053 928)
 (intervals 56 7474 3578)
 (buffers 992 27))



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

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[Eli Zaretskii]

> Date: Fri, 16 Dec 2022 23:54:23 +0100
> From: pmercatoris <mercatorispierre@gmail.com>
> 
> I am unable to get correct behavious when sending a region from indented code
> to the python shell. Consider this python code:
> 
> ```python
> if True:
>     if True:
>         a = "test"
> ```
> 
> If I select the first `True` and `python-shell-send-region`, it will
> correctly send it to the consol and print `True`.
> 
> Now if I do the same thing with the second `True` it will throw an
> error: `IndentationError: expected an indented block after 'if'
> statement on line 1` Although it would also reffer to unexpected block
> after `if` even it it were `for`, `try`, ...
> 
> If I select the `a` or `a = "test"` it will correctly send it to the
> console, however it won't echo the evaluation of the statement.
> 
> If I select the string "test" and send, it will throw the same error as
> the second True.
> 
> My python config is very minimal:
> 
>   (use-package python
>     :ensure nil
>     :mode
>     ("\\.py\\'" . python-mode)
> 
>     :init
>     (setq-default indent-tabs-mode nil)
> 
>     :config
>     (setq python-indent-offset 4
>           python-indent-guess-indent-offset-verbose nil
>           )
> 
>     (setq python-shell-interpreter "jupyter"
>           python-shell-interpreter-args "console --simple-prompt"
>           python-shell-prompt-detect-failure-warning nil)
> 
>     (add-to-list 'python-shell-completion-native-disabled-interpreters
>                  "jupyter"))

Could you two please look into this?  It sounds to me like Emacs works
as expected here, but I'm not an expert on Python and our interfaces
with it.

Thanks.

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[Pierre Mercatoris]

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

I wanted to give more details. But I basically get the same issue with
Emacs -Q and the default python console.

On Sun, 18 Dec 2022 at 11:38, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Fri, 16 Dec 2022 23:54:23 +0100
> > From: pmercatoris <mercatorispierre@gmail.com>
> >
> > I am unable to get correct behavious when sending a region from indented
> code
> > to the python shell. Consider this python code:
> >
> > ```python
> > if True:
> >     if True:
> >         a = "test"
> > ```
> >
> > If I select the first `True` and `python-shell-send-region`, it will
> > correctly send it to the consol and print `True`.
> >
> > Now if I do the same thing with the second `True` it will throw an
> > error: `IndentationError: expected an indented block after 'if'
> > statement on line 1` Although it would also reffer to unexpected block
> > after `if` even it it were `for`, `try`, ...
> >
> > If I select the `a` or `a = "test"` it will correctly send it to the
> > console, however it won't echo the evaluation of the statement.
> >
> > If I select the string "test" and send, it will throw the same error as
> > the second True.
> >
> > My python config is very minimal:
> >
> >   (use-package python
> >     :ensure nil
> >     :mode
> >     ("\\.py\\'" . python-mode)
> >
> >     :init
> >     (setq-default indent-tabs-mode nil)
> >
> >     :config
> >     (setq python-indent-offset 4
> >           python-indent-guess-indent-offset-verbose nil
> >           )
> >
> >     (setq python-shell-interpreter "jupyter"
> >           python-shell-interpreter-args "console --simple-prompt"
> >           python-shell-prompt-detect-failure-warning nil)
> >
> >     (add-to-list 'python-shell-completion-native-disabled-interpreters
> >                  "jupyter"))
>
> Could you two please look into this?  It sounds to me like Emacs works
> as expected here, but I'm not an expert on Python and our interfaces
> with it.
>
> Thanks.
>

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

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[kobarity]

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

Pierre Mercatoris <mercatorispierre@gmail.com>:

> > ```python
>> > if True:
>> >     if True:
>> >         a = "test"
>> > ```
>> >
>> > If I select the first `True` and `python-shell-send-region`, it will
>> > correctly send it to the consol and print `True`.
>> >
>> > Now if I do the same thing with the second `True` it will throw an
>> > error: `IndentationError: expected an indented block after 'if'
>> > statement on line 1` Although it would also reffer to unexpected block
>> > after `if` even it it were `for`, `try`, ...
>> >
>> > If I select the `a` or `a = "test"` it will correctly send it to the
>> > console, however it won't echo the evaluation of the statement.
>> >
>> > If I select the string "test" and send, it will throw the same error as
>> > the second True.
>
>
Hi,

I could reproduce and workaround the bug.  But could you give me a few more
days to consider well and revise the patch?

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

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[Eli Zaretskii]

> From: kobarity <kobarity@gmail.com>
> Date: Mon, 19 Dec 2022 07:25:46 +0900
> Cc: Eli Zaretskii <eliz@gnu.org>, 60142@debbugs.gnu.org, 
> 	Augusto Stoffel <arstoffel@gmail.com>
> 
> I could reproduce and workaround the bug.  But could you give me a few more days to consider well and
> revise the patch?

No problem, take your time.

Thanks.

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[Augusto Stoffel]

On Sun, 18 Dec 2022 at 12:39, Eli Zaretskii wrote:

>> If I select the `a` or `a = "test"` it will correctly send it to the
>> console, however it won't echo the evaluation of the statement.

I can at least explain why this happens and is expected.

An evaluation result is printed only if you send a bunch of statements,
the last of which is an expression.  OTOH, since whitespace is
significant in Python, if you evaluate anything that's not a "toplevel
form" it gets wrapper in a `if True:` statement, so the actually
evaluated code is not a simple expression anymore.

It seems hard to work around this limitation.

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[kobarity]

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

Augusto Stoffel wrote:
> On Sun, 18 Dec 2022 at 12:39, Eli Zaretskii wrote:
> >> If I select the `a` or `a = "test"` it will correctly send it to the
> >> console, however it won't echo the evaluation of the statement.
> 
> I can at least explain why this happens and is expected.
> 
> An evaluation result is printed only if you send a bunch of statements,
> the last of which is an expression.  OTOH, since whitespace is
> significant in Python, if you evaluate anything that's not a "toplevel
> form" it gets wrapper in a `if True:` statement, so the actually
> evaluated code is not a simple expression anymore.
> 
> It seems hard to work around this limitation.

Sorry for the late reply.

As Augusto explained, `if True:` is added to the indented region.
This behavior is documented in `python-shell-buffer-substring'. Maybe
it's better to add a reference to `python-shell-buffer-substring' in
the docstring of `python-shell-send-region'.

What I'm trying to do is to improve the behavior in some use cases.
Specifically, there is no need to add `if True:` if the region
consists of a single statement such as `a` or `a = "test"`.  Removing
leading spaces should be enough to avoid an indentation error even if
the single statement spans multiple lines.

The corner case is the following use case.

#+begin_src python
if True:
    s = """
    a = 1
    b = 2
"""
#+end_src

Let's assume we want to send the lines "a = 1" and "b = 1" only.
Although they are part of a single statement (multiline string), `if
True:` should be added to avoid an indentation error.  In other words,
they should not be considered as a single statement.  To address such
situation, the single statement check should be done after calling
`narrow-to-region'.

Attached is a patch to achieve this.  I appreciate your comments.

Thanks,

[-- Attachment #2: 0001-Fix-python-shell-buffer-substring-when-retrieving-a-.patch --]
[-- Type: application/octet-stream, Size: 6917 bytes --]

From 5512a76bf696e33003cf3c2be7de6fe7c0464b98 Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Thu, 22 Dec 2022 23:08:40 +0900
Subject: [PATCH] Fix python-shell-buffer-substring when retrieving a single
 statement

* lisp/progmodes/python.el (python-shell-buffer-substring): Do not add
"if True:" line when retrieving a single statement.
(python-shell-send-region): Add a reference to
`python-shell-buffer-substring' in docstring.
* test/lisp/progmodes/python-tests.el (python-shell-buffer-substring-13)
(python-shell-buffer-substring-14, python-shell-buffer-substring-15)
(python-shell-buffer-substring-16, python-shell-buffer-substring-17):
New tests. (Bug#60142)
---
 lisp/progmodes/python.el            | 45 +++++++++++++-------
 test/lisp/progmodes/python-tests.el | 64 +++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index bdc9e6fa78..c808f57171 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3717,19 +3717,35 @@ python-shell-buffer-substring
      appending extra empty lines so tracebacks are correct.
   3. When the region sent is a substring of the current buffer, a
      coding cookie is added.
-  4. Wraps indented regions under an \"if True:\" block so the
-     interpreter evaluates them correctly."
-  (let* ((start (save-excursion
-                  ;; If we're at the start of the expression, and
-                  ;; there's just blank space ahead of it, then expand
-                  ;; the region to include the start of the line.
-                  ;; This makes things work better with the rest of
-                  ;; the data we're sending over.
+  4. When the region consists of a single statement, leading
+     whitespaces will be removed.  Otherwise, wraps indented
+     regions under an \"if True:\" block so the interpreter
+     evaluates them correctly."
+  (let* ((single-p (save-restriction
+                     (narrow-to-region start end)
+                     (= (progn
+                          (goto-char start)
+                          (python-nav-beginning-of-statement))
+                        (progn
+                          (goto-char end)
+                          (python-nav-beginning-of-statement)))))
+         (start (save-excursion
+                  ;; If we're at the start of the expression, and if
+                  ;; the region consists of a single statement, then
+                  ;; remove leading whitespaces, else if there's just
+                  ;; blank space ahead of it, then expand the region
+                  ;; to include the start of the line.  This makes
+                  ;; things work better with the rest of the data
+                  ;; we're sending over.
                   (goto-char start)
-                  (if (string-blank-p
-                       (buffer-substring (line-beginning-position) start))
-                      (line-beginning-position)
-                    start)))
+                  (if single-p
+                      (progn
+                        (skip-chars-forward "[:space:]" end)
+                        (point))
+                    (if (string-blank-p
+                         (buffer-substring (line-beginning-position) start))
+                        (line-beginning-position)
+                      start))))
          (substring (buffer-substring-no-properties start end))
          (starts-at-point-min-p (save-restriction
                                   (widen)
@@ -3753,7 +3769,7 @@ python-shell-buffer-substring
       (python-mode)
       (when fillstr
         (insert fillstr))
-      (when (not toplevel-p)
+      (when (and (not single-p) (not toplevel-p))
         (forward-line -1)
         (insert "if True:\n")
         (delete-region (point) (line-end-position)))
@@ -3797,7 +3813,8 @@ python-shell-send-region
 When called interactively SEND-MAIN defaults to nil, unless it's
 called with prefix argument.  When optional argument MSG is
 non-nil, forces display of a user-friendly message if there's no
-process running; defaults to t when called interactively."
+process running; defaults to t when called interactively.  The
+substring to be sent is retrieved using `python-shell-buffer-substring'."
   (interactive
    (list (region-beginning) (region-end) current-prefix-arg t))
   (let* ((string (python-shell-buffer-substring start end (not send-main)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 17d6d8aa70..930234a12f 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -4456,6 +4456,70 @@ python-shell-buffer-substring-12
                      (point-max))
                     "# -*- coding: utf-8 -*-\n\nif True:\n        # Whitespace\n\n    print ('a')\n\n"))))
 
+(ert-deftest python-shell-buffer-substring-13 ()
+  "Check substring from indented single statement."
+  (python-tests-with-temp-buffer
+   "
+def foo():
+    a = 1
+"
+   (should (string= (python-shell-buffer-substring
+                     (python-tests-look-at "a = 1")
+                     (pos-eol))
+                    "# -*- coding: utf-8 -*-\n\na = 1"))))
+
+(ert-deftest python-shell-buffer-substring-14 ()
+  "Check substring from indented single statement spanning multiple lines."
+  (python-tests-with-temp-buffer
+   "
+def foo():
+    a = \"\"\"Some
+    string\"\"\"
+"
+   (should (string= (python-shell-buffer-substring
+                     (python-tests-look-at "a = \"\"\"Some")
+                     (pos-eol 2))
+                    "# -*- coding: utf-8 -*-\n\na = \"\"\"Some\n    string\"\"\""))))
+
+(ert-deftest python-shell-buffer-substring-15 ()
+  "Check substring from partial statement."
+  (python-tests-with-temp-buffer
+   "
+def foo():
+    a = 1
+"
+   (should (string= (python-shell-buffer-substring
+                     (python-tests-look-at "    a = 1")
+                     (python-tests-look-at " = 1"))
+                    "# -*- coding: utf-8 -*-\n\na"))))
+
+(ert-deftest python-shell-buffer-substring-16 ()
+  "Check substring from partial statement."
+  (python-tests-with-temp-buffer
+   "
+def foo():
+    a = 1
+"
+   (should (string= (python-shell-buffer-substring
+                     (python-tests-look-at "1")
+                     (1+ (point)))
+                    "# -*- coding: utf-8 -*-\n\n1"))))
+
+(ert-deftest python-shell-buffer-substring-17 ()
+  "Check substring from multiline string."
+  (python-tests-with-temp-buffer
+   "
+def foo():
+    s = \"\"\"
+    a = 1
+    b = 2
+\"\"\"
+"
+   (should (string= (python-shell-buffer-substring
+                     (python-tests-look-at "a = 1")
+                     (python-tests-look-at "\"\"\""))
+                    "# -*- coding: utf-8 -*-\n\nif True:\n    a = 1\n    b = 2\n\n"))))
+
 
 \f
 ;;; Shell completion
-- 
2.34.1

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[Pierre Mercatoris]

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

Hello and sorry for the late reply..

Excuse my ignorance, but how would I need to go about to apply those
changes to test them?

Furthermore, I am not sure the main issue I raised has been diluted.
Basically I was wondering why a region, which does not include any
indentation (as it is a fragment of a line), is sent to the repl results in
indentation error if the life the fragment comes from was indented. In the
case Kobarty described above, he is mentioning sending the whole line to
the repl, in which case it can be debated how to deal with indentation.
However, my issue is that equivalent regions sent to the repl without any
indentation within them results in different behaviours depending on where
those regions (fragments of line) come from.

I hope I could explain myself better.

Thank you all for your interest in the matter.

On Thu, Dec 22, 2022 at 4:01 PM kobarity <kobarity@gmail.com> wrote:

> Augusto Stoffel wrote:
> > On Sun, 18 Dec 2022 at 12:39, Eli Zaretskii wrote:
> > >> If I select the `a` or `a = "test"` it will correctly send it to the
> > >> console, however it won't echo the evaluation of the statement.
> >
> > I can at least explain why this happens and is expected.
> >
> > An evaluation result is printed only if you send a bunch of statements,
> > the last of which is an expression.  OTOH, since whitespace is
> > significant in Python, if you evaluate anything that's not a "toplevel
> > form" it gets wrapper in a `if True:` statement, so the actually
> > evaluated code is not a simple expression anymore.
> >
> > It seems hard to work around this limitation.
>
> Sorry for the late reply.
>
> As Augusto explained, `if True:` is added to the indented region.
> This behavior is documented in `python-shell-buffer-substring'. Maybe
> it's better to add a reference to `python-shell-buffer-substring' in
> the docstring of `python-shell-send-region'.
>
> What I'm trying to do is to improve the behavior in some use cases.
> Specifically, there is no need to add `if True:` if the region
> consists of a single statement such as `a` or `a = "test"`.  Removing
> leading spaces should be enough to avoid an indentation error even if
> the single statement spans multiple lines.
>
> The corner case is the following use case.
>
> #+begin_src python
> if True:
>     s = """
>     a = 1
>     b = 2
> """
> #+end_src
>
> Let's assume we want to send the lines "a = 1" and "b = 1" only.
> Although they are part of a single statement (multiline string), `if
> True:` should be added to avoid an indentation error.  In other words,
> they should not be considered as a single statement.  To address such
> situation, the single statement check should be done after calling
> `narrow-to-region'.
>
> Attached is a patch to achieve this.  I appreciate your comments.
>
> Thanks,
>

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

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[kobarity]

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

Hi,

Pierre Mercatoris wrote:
> Excuse my ignorance, but how would I need to go about to apply those changes
> to test them?

The patch in my previous mail is a diff to master branch of Emacs.  I
attached a patch to python.el of Emacs 28.1.  It can be applied as
follows:

% patch python.el python.el.28.1.patch

After starting Emacs, I recommend to do 'M-x load-file' and specify
the patched python.el to avoid loading the old python.elc.

> Furthermore, I am not sure the main issue I raised has been diluted. Basically
> I was wondering why a region, which does not include any indentation (as it is
> a fragment of a line), is sent to the repl results in indentation error if the
> life the fragment comes from was indented. In the case Kobarty described
> above, he is mentioning sending the whole line to the repl, in which case it
> can be debated how to deal with indentation. However, my issue is that
> equivalent regions sent to the repl without any indentation within them
> results in different behaviours depending on where those regions (fragments of
> line) come from.

My explanation was probably misleading.  I'm not saying that the whole
line should be sent to the REPL.  My point was that if the content
sent to the REPL is one statement, there is no need to add `if True:`.
Please note that a "statement" in python.el is a logical unit based on
Python syntax and differs from physical lines.  A statement can be
part of a physical line, and it can also spans multiple physical
lines.

As Augusto pointed out, current Emacs python.el adds `if True:` if the
lines of the region was originally indented.  For example, let's
consider the following code.

#+begin_src python
def func():
    a = 1
    b = 2
#+end_src

If we set the region to "a = 1" and call `python-shell-send-region',
the following code will be sent to the REPL.

#+begin_src python
if True:
    a = 1
#+end_src

Even if the region is set to only "a", the following code will be
sent.

#+begin_src python
if True:
    a
#+end_src

Adding `if True:` is reasonable if the indented region spans multiple
lines.  For example, if the region is "a = 1\n    b = 2", the
following code will be sent.

#+begin_src python
if True:
    a = 1
    b = 2
#+end_src

If the region "a = 1\n    b = 2" or "    a = 1\n    b = 2" is sent as
is, an indentation error will occur.

I think current `python-shell-send-region' focuses on sending a region
of multiple lines and does not take into account sending part of a
line.

As I mentioned above, my patch is intended to improve this behavior.
If the region is "a = 1", or even "    a = 1", the following code will
be sent because this is a single statement.

#+begin_src python
a = 1
#+end_src

I think this behavior is what you expected.

Please note that the above explanation is a bit simplified.  The
actual contents `python-shell-send-region' sends includes a coding
cookie and empty lines.  Please see docstring of
`python-shell-buffer-substring' for more details.

I hope this clarifies things.

Regards,

[-- Attachment #2: python.el.28.1.patch --]
[-- Type: application/octet-stream, Size: 3447 bytes --]

--- python.el.orig	2022-12-30 23:17:47.490936027 +0900
+++ python.el	2022-12-30 23:19:25.555944407 +0900
@@ -3262,19 +3262,35 @@
      appending extra empty lines so tracebacks are correct.
   3. When the region sent is a substring of the current buffer, a
      coding cookie is added.
-  4. Wraps indented regions under an \"if True:\" block so the
-     interpreter evaluates them correctly."
-  (let* ((start (save-excursion
-                  ;; If we're at the start of the expression, and
-                  ;; there's just blank space ahead of it, then expand
-                  ;; the region to include the start of the line.
-                  ;; This makes things work better with the rest of
-                  ;; the data we're sending over.
+  4. When the region consists of a single statement, leading
+     whitespaces will be removed.  Otherwise, wraps indented
+     regions under an \"if True:\" block so the interpreter
+     evaluates them correctly."
+  (let* ((single-p (save-restriction
+                     (narrow-to-region start end)
+                     (= (progn
+                          (goto-char start)
+                          (python-nav-beginning-of-statement))
+                        (progn
+                          (goto-char end)
+                          (python-nav-beginning-of-statement)))))
+         (start (save-excursion
+                  ;; If we're at the start of the expression, and if
+                  ;; the region consists of a single statement, then
+                  ;; remove leading whitespaces, else if there's just
+                  ;; blank space ahead of it, then expand the region
+                  ;; to include the start of the line.  This makes
+                  ;; things work better with the rest of the data
+                  ;; we're sending over.
                   (goto-char start)
-                  (if (string-blank-p
-                       (buffer-substring (line-beginning-position) start))
-                      (line-beginning-position)
-                    start)))
+                  (if single-p
+                      (progn
+                        (skip-chars-forward "[:space:]" end)
+                        (point))
+                    (if (string-blank-p
+                         (buffer-substring (line-beginning-position) start))
+                        (line-beginning-position)
+                      start))))
          (substring (buffer-substring-no-properties start end))
          (starts-at-point-min-p (save-restriction
                                   (widen)
@@ -3297,7 +3313,7 @@
         (insert fillstr))
       (insert substring)
       (goto-char (point-min))
-      (when (not toplevel-p)
+      (when (and (not single-p) (not toplevel-p))
         (insert "if True:")
         (delete-region (point) (line-end-position)))
       (when nomain
@@ -3339,7 +3355,8 @@
 When called interactively SEND-MAIN defaults to nil, unless it's
 called with prefix argument.  When optional argument MSG is
 non-nil, forces display of a user-friendly message if there's no
-process running; defaults to t when called interactively."
+process running; defaults to t when called interactively.  The
+substring to be sent is retrieved using `python-shell-buffer-substring'."
   (interactive
    (list (region-beginning) (region-end) current-prefix-arg t))
   (let* ((string (python-shell-buffer-substring start end (not send-main)

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[Pierre Mercatoris]

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

Thank you so much for your detailed and simplified explanations, I highly
appreciate it.

I can confirm after applying the latest patch you provided that I am now
getting the expected behaviour
of being able to send statements regardless of their indentation. This will
fundamentally improve my daily
use of emacs. Thank you so much for that.

Regarding the patch, is there anything that should be done in order for the
patch to be applied in an incoming
Emacs release?

Kind regards,
Pierre

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

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[kobarity]

Pierre Mercatoris wrote:
> Regarding the patch, is there anything that should be done in order for the
> patch to be applied in an incoming 
> Emacs release?

Hi,

Thank you for testing my patch.  I think that if the committers decide
that it is worth incorporating, they will commit it.  Your report
should have been one of the bases for the decision.

bug#60142: 28.1; python.el Incorrect region when python-shell-send-region from indented code

[Eli Zaretskii]

> Date: Sat, 31 Dec 2022 16:23:32 +0900
> From: kobarity <kobarity@gmail.com>
> Cc: Augusto Stoffel <arstoffel@gmail.com>,
> 	Eli Zaretskii <eliz@gnu.org>,
> 	60142@debbugs.gnu.org
> 
> 
> Pierre Mercatoris wrote:
> > Regarding the patch, is there anything that should be done in order for the
> > patch to be applied in an incoming 
> > Emacs release?
> 
> Hi,
> 
> Thank you for testing my patch.  I think that if the committers decide
> that it is worth incorporating, they will commit it.  Your report
> should have been one of the bases for the decision.

Thanks, I installed this on the emacs-29 release branch, and I'm
therefore closing the bug.