* bug#61463: 29.0.60; python-shell-send-region moves point
2023-02-12 21:55 bug#61463: 29.0.60; python-shell-send-region moves point Augusto Stoffel
@ 2023-02-13 15:43 ` kobarity
2023-02-18 16:45 ` Eli Zaretskii
0 siblings, 1 reply; 3+ messages in thread
From: kobarity @ 2023-02-13 15:43 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 61463
[-- 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
^ permalink raw reply related [flat|nested] 3+ messages in thread