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

[kobarity <kobarity@gmail.com>]

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