bug#55702: 29.0.50; python-mark-defun does not handle backslash escaped newline

bug#55702: 29.0.50; python-mark-defun does not handle backslash escaped newline

[kobarity]

When the following steps are performed, the first line "def \" will
not be included in the region.

1. emacs -Q
2. Load the following Python file using M-x find-file

#+begin_src python
def \
        foo(x):
    return x
#+end_src

3. M-x forward-line (Move point to the line "foo(x):".)
4. M-x python-mark-defun

In GNU Emacs 29.0.50 (build 4, x86_64-pc-linux-gnu)
 of 2022-05-21 built on ubuntu
Repository revision: 208102fa470e3417320062cdb48a9967d80bf092
Repository branch: master
System Description: Ubuntu 22.04 LTS

Configured using:
 'configure --without-x --with-gnutls=ifavailable'

Configured features:
ACL LIBXML2 MODULES NOTIFY INOTIFY PDUMPER SECCOMP SOUND SQLITE3 THREADS
XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow regexp-opt sort mail-extr face-remap help-mode tool-bar emacsbug
message mailcap yank-media rmc puny dired dired-loaddefs rfc822 mml
mml-sec password-cache epa derived epg rfc6068 epg-config gnus-util
text-property-search time-date seq mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs cl-lib
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
term/screen term/xterm xterm gv subr-x byte-opt bytecomp byte-compile
cconv iso-transl tooltip eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice simple 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 abbrev obarray oclosure
cl-preloaded button loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads inotify
multi-tty make-network-process emacs)

Memory information:
((conses 16 48401 6401)
 (symbols 48 5803 1)
 (strings 32 16090 1890)
 (string-bytes 1 510153)
 (vectors 16 9643)
 (vector-slots 8 112266 9121)
 (floats 8 25 264)
 (intervals 56 385 66)
 (buffers 992 12))

bug#55702: 29.0.50; python-mark-defun does not handle backslash escaped newline

[kobarity]

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

python-mark-defun and python-info-looking-at-beginning-of-defun do not
consider the presence of backslash escaped newline in defuns.
Attached is a patch to fix this issue.

[-- Attachment #2: fix-55702.patch --]
[-- Type: application/octet-stream, Size: 2529 bytes --]

commit 28aeb41bc05ecf2d887a8d9290f860e52ee4e73e
Author: kobarity <kobarity@gmail.com>
Date:   Sun May 29 21:23:35 2022 +0900

    Make `python-mark-defun' handle backslash escaped newline
    
    * lisp/progmodes/python.el (python-mark-defun)
    (python-info-looking-at-beginning-of-defun): Allow backslash
    escaped newline in defuns (bug#55702).

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index c2483436fe..7ab063daa9 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1426,7 +1426,7 @@ python-mark-defun
 marks the next defun after the ones already marked."
   (interactive "p")
   (when (python-info-looking-at-beginning-of-defun)
-    (end-of-line 1))
+    (python-nav-end-of-statement))
   (mark-defun allow-extend))
 
 \f
@@ -5290,6 +5290,7 @@ python-info-looking-at-beginning-of-defun
   "Check if point is at `beginning-of-defun' using SYNTAX-PPSS."
   (and (not (python-syntax-context-type (or syntax-ppss (syntax-ppss))))
        (save-excursion
+         (python-nav-beginning-of-statement)
          (beginning-of-line 1)
          (looking-at python-nav-beginning-of-defun-regexp))))
 
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 8db0a07170..c1b71d15fd 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -1736,6 +1736,36 @@ python-mark-defun-3
      (should (= (marker-position (mark-marker))
                 expected-mark-end-position)))))
 
+(ert-deftest python-mark-defun-4 ()
+  "Test `python-mark-defun' with point inside backslash escaped defun."
+  (python-tests-with-temp-buffer
+   "
+def \\
+        foo(x):
+    return x
+"
+   (let ((transient-mark-mode t)
+         (expected-mark-beginning-position
+          (progn
+            (python-tests-look-at "def ")
+            (1- (line-beginning-position))))
+         (expected-mark-end-position
+          (save-excursion
+            (python-tests-look-at "return x")
+            (forward-line)
+            (point))))
+     (python-tests-look-at "def ")
+     (python-mark-defun 1)
+     (should (= (point) expected-mark-beginning-position))
+     (should (= (marker-position (mark-marker))
+                expected-mark-end-position))
+     (deactivate-mark)
+     (python-tests-look-at "foo(x)")
+     (python-mark-defun 1)
+     (should (= (point) expected-mark-beginning-position))
+     (should (= (marker-position (mark-marker))
+                expected-mark-end-position)))))
+
 \f
 ;;; Navigation
 

bug#55702: 29.0.50; python-mark-defun does not handle backslash escaped newline

[kobarity]

Sorry, it may not be good to change the behavior of
python-info-looking-at-beginning-of-defun.  I will reconsider the fix.

bug#55702: 29.0.50; python-mark-defun does not handle backslash escaped newline

[kobarity]

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

Hello,

I wrote:
> When the following steps are performed, the first line "def \" will
> not be included in the region.
>
> 1. emacs -Q
> 2. Load the following Python file using M-x find-file
>
> #+begin_src python
> def \
>         foo(x):
>     return x
> #+end_src
>
> 3. M-x forward-line (Move point to the line "foo(x):".)
> 4. M-x python-mark-defun

In fact, this is not only the issue in python-mark-defun, but also the
issue in python-nav-beginning-of-defun. In the example above,
python-nav-beginning-of-defun does not move the point if the point is
located at the beginning of the line "foo(x):". My previous patch was
not sufficient to solve the issue in python-nav-beginning-of-defun. So
I attach the revised patch.

On the other hand, I'm wondering if it is worth supporting such rarely
used forms, as there seems to be some more issues regarding the line
continuation using backslash. Is it better to leave such rarely used
forms unsupported and keep the code simple?

Below is the explanation of my patch:

> Sorry, it may not be good to change the behavior of
> python-info-looking-at-beginning-of-defun.  I will reconsider the fix.

Instead of changing the default behavior of
python-info-looking-at-beginning-of-defun, I added an argument to
change the behavior. When CHECK-STATEMENT is non-nil, the current
statement is checked instead of the current physical line.

This argument is used in python-nav--beginning-of-defun to be able
to recognize the defun with line continuation using backslash. When
ARG is positive, the point is moved to the end of the statement if the
point is located at the continuation line. This is necessary for
re-search-backward to find the defun regexp.

If the defun is found when ARG is negative, it is necessary to move
point to the beginning of the statement, because the point is at the
end of the defun regexp which may be in the different line from the
beginning of the statement.

Best Regards,

[-- Attachment #2: fix-55702.patch --]
[-- Type: application/octet-stream, Size: 7284 bytes --]

commit de745b4a395d79e0cba915593c228345d7862d59
Author: kobarity <kobarity@gmail.com>
Date:   Sun Jul 3 17:14:48 2022 +0900

    Make `python-nav-beginning-of-defun' handle line continuation using backslash
    
    * lisp/progmodes/python.el (python-nav--beginning-of-defun): Allow
    line continuation using backslash in defuns (bug#55702).
    (python-info-looking-at-beginning-of-defun): Add CHECK-STATEMENT
    argument.

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 16cdf58611..7a626ae35e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1472,15 +1472,17 @@ python-nav--beginning-of-defun
                      0))))
          (found
           (progn
-            (when (and (python-info-looking-at-beginning-of-defun)
+            (when (and (python-info-looking-at-beginning-of-defun nil t)
                        (or (< arg 0)
                            ;; If looking at beginning of defun, and if
                            ;; pos is > line-content-start, ensure a
                            ;; backward re search match this defun by
                            ;; going to end of line before calling
                            ;; re-search-fn bug#40563
-                           (and (> arg 0) (> pos line-content-start))))
-              (end-of-line 1))
+                           (and (> arg 0)
+                                (or (python-info-continuation-line-p)
+                                    (> pos line-content-start)))))
+              (python-nav-end-of-statement))
 
             (while (and (funcall re-search-fn
                                  python-nav-beginning-of-defun-regexp nil t)
@@ -1490,14 +1492,18 @@ python-nav--beginning-of-defun
                             (and (> arg 0)
                                  (not (= (current-indentation) 0))
                                  (>= (current-indentation) body-indentation)))))
-            (and (python-info-looking-at-beginning-of-defun)
+            (and (python-info-looking-at-beginning-of-defun nil t)
                  (or (not (= (line-number-at-pos pos)
                              (line-number-at-pos)))
                      (and (>= (point) line-beg-pos)
                           (<= (point) line-content-start)
                           (> pos line-content-start)))))))
     (if found
-        (or (beginning-of-line 1) t)
+        (progn
+          (when (< arg 0)
+            (python-nav-beginning-of-statement))
+          (beginning-of-line 1)
+          t)
       (and (goto-char pos) nil))))
 
 (defun python-nav-beginning-of-defun (&optional arg)
@@ -5299,10 +5305,15 @@ python-info-assignment-continuation-line-p
       (forward-line -1)
       (python-info-assignment-statement-p t))))
 
-(defun python-info-looking-at-beginning-of-defun (&optional syntax-ppss)
-  "Check if point is at `beginning-of-defun' using SYNTAX-PPSS."
+(defun python-info-looking-at-beginning-of-defun (&optional syntax-ppss
+                                                            check-statement)
+  "Check if point is at `beginning-of-defun' using SYNTAX-PPSS.
+When CHECK-STATEMENT is non-nil, the current statement is checked
+instead of the current physical line."
   (and (not (python-syntax-context-type (or syntax-ppss (syntax-ppss))))
        (save-excursion
+         (when check-statement
+           (python-nav-beginning-of-statement))
          (beginning-of-line 1)
          (looking-at python-nav-beginning-of-defun-regexp))))
 
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index c59a2e7953..d7b3c102f2 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -1757,6 +1757,36 @@ python-mark-defun-4
      (should (= (marker-position (mark-marker))
                 expected-mark-end-position)))))
 
+(ert-deftest python-mark-defun-5 ()
+  "Test `python-mark-defun' with point inside backslash escaped defun."
+  (python-tests-with-temp-buffer
+   "
+def \\
+        foo(x):
+    return x
+"
+   (let ((transient-mark-mode t)
+         (expected-mark-beginning-position
+          (progn
+            (python-tests-look-at "def ")
+            (1- (line-beginning-position))))
+         (expected-mark-end-position
+          (save-excursion
+            (python-tests-look-at "return x")
+            (forward-line)
+            (point))))
+     (python-tests-look-at "def ")
+     (python-mark-defun 1)
+     (should (= (point) expected-mark-beginning-position))
+     (should (= (marker-position (mark-marker))
+                expected-mark-end-position))
+     (deactivate-mark)
+     (python-tests-look-at "foo(x)")
+     (python-mark-defun 1)
+     (should (= (point) expected-mark-beginning-position))
+     (should (= (marker-position (mark-marker))
+                expected-mark-end-position)))))
+
 \f
 ;;; Navigation
 
@@ -1905,17 +1935,47 @@ python-nav-beginning-of-defun-3
 (ert-deftest python-nav-beginning-of-defun-4 ()
   (python-tests-with-temp-buffer
    "
+def a():
+    pass
+
 def \\
-        a():
+        b():
     return 0
+
+def c():
+    pass
 "
-   (python-tests-look-at "return 0")
+   (python-tests-look-at "def c():")
+   (should (= (save-excursion
+                (python-nav-beginning-of-defun)
+                (point))
+              (save-excursion
+                (python-tests-look-at "def \\" -1)
+                (beginning-of-line)
+                (point))))
+   (python-tests-look-at "return 0" -1)
    (should (= (save-excursion
                 (python-nav-beginning-of-defun)
                 (point))
               (save-excursion
                 (python-tests-look-at "def \\" -1)
                 (beginning-of-line)
+                (point))))
+   (python-tests-look-at "b():" -1)
+   (should (= (save-excursion
+                (python-nav-beginning-of-defun)
+                (point))
+              (save-excursion
+                (python-tests-look-at "def \\" -1)
+                (beginning-of-line)
+                (point))))
+   (python-tests-look-at "def a():" -1)
+   (should (= (save-excursion
+                (python-nav-beginning-of-defun -1)
+                (point))
+              (save-excursion
+                (python-tests-look-at "def \\")
+                (beginning-of-line)
                 (point))))))
 
 (ert-deftest python-nav-end-of-defun-1 ()
@@ -5242,6 +5302,23 @@ python-info-looking-at-beginning-of-defun-1
    (python-tests-look-at "deff()")
    (should (not (python-info-looking-at-beginning-of-defun)))))
 
+(ert-deftest python-info-looking-at-beginning-of-defun-2 ()
+  (python-tests-with-temp-buffer
+   "
+def \\
+        foo(arg):
+    pass
+"
+   (python-tests-look-at "def \\")
+   (should (python-info-looking-at-beginning-of-defun))
+   (should (python-info-looking-at-beginning-of-defun nil t))
+   (python-tests-look-at "foo(arg):")
+   (should (not (python-info-looking-at-beginning-of-defun)))
+   (should (python-info-looking-at-beginning-of-defun nil t))
+   (python-tests-look-at "pass")
+   (should (not (python-info-looking-at-beginning-of-defun)))
+   (should (not (python-info-looking-at-beginning-of-defun nil t)))))
+
 (ert-deftest python-info-current-line-comment-p-1 ()
   (python-tests-with-temp-buffer
    "

bug#55702: 29.0.50; python-mark-defun does not handle backslash escaped newline

[Lars Ingebrigtsen]

kobarity <kobarity@gmail.com> writes:

> On the other hand, I'm wondering if it is worth supporting such rarely
> used forms, as there seems to be some more issues regarding the line
> continuation using backslash. Is it better to leave such rarely used
> forms unsupported and keep the code simple?

It does seem slightly obscure, but the patch seems to make sense to me,
so I've pushed it to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no