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