unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: kobarity <kobarity@gmail.com>
To: Augusto Stoffel <arstoffel@gmail.com>
Cc: 61463@debbugs.gnu.org
Subject: bug#61463: 29.0.60; python-shell-send-region moves point
Date: Tue, 14 Feb 2023 00:43:39 +0900	[thread overview]
Message-ID: <eke7cz6dvadw.wl-kobarity@gmail.com> (raw)
In-Reply-To: <87pmaeo8g4.fsf@gmail.com>

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

Augusto Stoffel wrote:
> On Emacs -Q:
> 
> 1) M-x run-python
> 2) In the scratch buffer, type something, say 1 + 2 and mark it, leaving
>    point at the end.
> 3) M-x python-shell-send-string

Probably a typo of python-shell-send-region.

> Now the point is at the beginning of the region.  It was supposed not to
> move.
> 
> I suspect this might be due to a missing `save-excursion' in the
> definition of the local variable `single-p' in python.el.

Thank you for pointing that out.  You are right, `single-p' needs
`save-excursion'.  The same bug exists in `starts-at-first-line-p'.
I'm sorry for those bugs.

Atatched is a fix for this problem with improved ERTs to detect this
issue.

[-- Attachment #2: 0001-Fix-point-moving-when-calling-python-shell-send-regi.patch --]
[-- Type: application/octet-stream, Size: 11035 bytes --]

From e8714171a974961946771f23f2e26ec27b5344fc Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Tue, 14 Feb 2023 00:30:15 +0900
Subject: [PATCH] Fix point moving when calling python-shell-send-region

* lisp/progmodes/python.el (python-shell-buffer-substring): Add
`save-excursion' to prevent the point from moving.
* test/lisp/progmodes/python-tests.el (python-tests-should-not-move):
New helper function to assert that point does not move while calling a
function.
(python-shell-buffer-substring-*): Use
`python-tests-should-not-move'. (Bug#61463)
---
 lisp/progmodes/python.el            | 26 ++++++------
 test/lisp/progmodes/python-tests.el | 62 ++++++++++++++++++++---------
 2 files changed, 58 insertions(+), 30 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index df0d1c96965..0d714c31e9e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3759,14 +3759,15 @@ python-shell-buffer-substring
      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)))))
+  (let* ((single-p (save-excursion
+                     (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
@@ -3785,10 +3786,11 @@ python-shell-buffer-substring
                         (line-beginning-position)
                       start))))
          (substring (buffer-substring-no-properties start end))
-         (starts-at-first-line-p (save-restriction
-                                   (widen)
-                                   (goto-char start)
-                                   (= (line-number-at-pos) 1)))
+         (starts-at-first-line-p (save-excursion
+                                   (save-restriction
+                                     (widen)
+                                     (goto-char start)
+                                     (= (line-number-at-pos) 1))))
          (encoding (python-info-encoding))
          (toplevel-p (zerop (save-excursion
                               (goto-char start)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index df71990278e..4f24c042c6a 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -189,6 +189,14 @@ python-tests-visible-string
                            (overlay-end overlay))))
       (buffer-substring-no-properties (point-min) (point-max)))))
 
+(defun python-tests-should-not-move (func &rest args)
+  "Assert that point does not move while calling FUNC with ARGS.
+Returns the value returned by FUNC."
+  (let ((pos (point))
+        (ret (apply func args)))
+    (should (= pos (point)))
+    ret))
+
 (defun python-virt-bin (&optional virt-root)
   "Return the virtualenv bin dir, starting from VIRT-ROOT.
 If nil, VIRT-ROOT defaults to `python-shell-virtualenv-root'.
@@ -4213,7 +4221,8 @@ python-shell-buffer-substring-1
     pass
 "
    (should (string= (buffer-string)
-                    (python-shell-buffer-substring (point-min) (point-max))))))
+                    (python-tests-should-not-move
+                     #'python-shell-buffer-substring (point-min) (point-max))))))
 
 (ert-deftest python-shell-buffer-substring-2 ()
   "Main block should be removed if NOMAIN is non-nil."
@@ -4229,7 +4238,8 @@ python-shell-buffer-substring-2
     foo = Foo()
     print (foo)
 "
-   (should (string= (python-shell-buffer-substring (point-min) (point-max) t)
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring (point-min) (point-max) t)
                     "
 class Foo(models.Model):
     pass
@@ -4256,7 +4266,8 @@ python-shell-buffer-substring-3
 class Bar(models.Model):
     pass
 "
-   (should (string= (python-shell-buffer-substring (point-min) (point-max) t)
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring (point-min) (point-max) t)
                     "
 class Foo(models.Model):
     pass
@@ -4284,7 +4295,8 @@ python-shell-buffer-substring-4
 class Bar(models.Model):
     pass
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "class Foo(models.Model):")
                      (progn (python-nav-forward-sexp) (point)))
                     "# -*- coding: latin-1 -*-
@@ -4307,7 +4319,8 @@ python-shell-buffer-substring-5
 class Bar(models.Model):
     pass
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "class Bar(models.Model):")
                      (progn (python-nav-forward-sexp) (point)))
                     "# -*- coding: latin-1 -*-
@@ -4338,7 +4351,8 @@ python-shell-buffer-substring-6
 class Bar(models.Model):
     pass
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "# coding: latin-1")
                      (python-tests-look-at "if __name__ == \"__main__\":"))
                     "# -*- coding: latin-1 -*-
@@ -4365,7 +4379,8 @@ python-shell-buffer-substring-7
 class Bar(models.Model):
     pass
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "# coding: latin-1")
                      (python-tests-look-at "if __name__ == \"__main__\":"))
                     "# -*- coding: utf-8 -*-
@@ -4385,7 +4400,8 @@ python-shell-buffer-substring-8
 class Foo(models.Model):
     pass
 "
-   (should (string= (python-shell-buffer-substring (point-min) (point-max))
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring (point-min) (point-max))
                     "# coding: utf-8
 
 
@@ -4404,7 +4420,8 @@ python-shell-buffer-substring-9
 class Bar(models.Model):
     pass
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (point-min)
                      (python-tests-look-at "class Bar(models.Model):"))
                     "# coding: utf-8
@@ -4421,7 +4438,8 @@ python-shell-buffer-substring-10
 def foo():
     print ('a')
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "print ('a')")
                      (point-max))
                     "# -*- coding: utf-8 -*-\nif True:\n    print ('a')\n\n"))))
@@ -4433,7 +4451,8 @@ python-shell-buffer-substring-11
 def foo():
     print ('a')
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (progn
                        (python-tests-look-at "print ('a')")
                        (backward-char 1)
@@ -4451,7 +4470,8 @@ python-shell-buffer-substring-12
 
     print ('a')
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "# Whitespace")
                      (point-max))
                     "# -*- coding: utf-8 -*-\n\nif True:\n        # Whitespace\n\n    print ('a')\n\n"))))
@@ -4463,7 +4483,8 @@ python-shell-buffer-substring-13
 def foo():
     a = 1
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "a = 1")
                      (pos-eol))
                     "# -*- coding: utf-8 -*-\n\na = 1"))))
@@ -4476,7 +4497,8 @@ python-shell-buffer-substring-14
     a = \"\"\"Some
     string\"\"\"
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "a = \"\"\"Some")
                      (pos-eol 2))
                     "# -*- coding: utf-8 -*-\n\na = \"\"\"Some\n    string\"\"\""))))
@@ -4488,7 +4510,8 @@ python-shell-buffer-substring-15
 def foo():
     a = 1
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "    a = 1")
                      (python-tests-look-at " = 1"))
                     "# -*- coding: utf-8 -*-\n\na"))))
@@ -4500,7 +4523,8 @@ python-shell-buffer-substring-16
 def foo():
     a = 1
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "1")
                      (1+ (point)))
                     "# -*- coding: utf-8 -*-\n\n1"))))
@@ -4515,7 +4539,8 @@ python-shell-buffer-substring-17
     b = 2
 \"\"\"
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'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"))))
@@ -4525,7 +4550,8 @@ python-shell-buffer-substring-18
   (python-tests-with-temp-buffer
    "s = 'test'
 "
-   (should (string= (python-shell-buffer-substring
+   (should (string= (python-tests-should-not-move
+                     #'python-shell-buffer-substring
                      (python-tests-look-at "'test'")
                      (pos-eol))
                     "'test'"))))
-- 
2.34.1


  reply	other threads:[~2023-02-13 15:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-12 21:55 bug#61463: 29.0.60; python-shell-send-region moves point Augusto Stoffel
2023-02-13 15:43 ` kobarity [this message]
2023-02-18 16:45   ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eke7cz6dvadw.wl-kobarity@gmail.com \
    --to=kobarity@gmail.com \
    --cc=61463@debbugs.gnu.org \
    --cc=arstoffel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).