bug#62092: 30.0.50; "case" does not work as dedenter in Python

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[kobarity]

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

Hi,

New keywords "match" and "case" were added to block-start in
`python-rx', which enables basic indentation support for these
keywords.  However, "case" does not work as a dedenter.  For example,
try the following code.

#+begin_src python
match a:
    case 1:
        print(1)
        case 2:
#+end_src

At the "case 2:" line, inputting ":" does not dedent the line.
Several TAB keys are needed for correct indentation.

Attached is a patch to make "case" a dedenter with an ERT.

--
In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw scroll bars) of 2023-03-10 built on ubuntu
Repository revision: 8ee205d232574e12921b052c7e93b7e16d6f1187
Repository branch: master
System Description: Ubuntu 22.04.2 LTS

Configured features:
ACL CAIRO FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG SECCOMP SOUND
SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER X11 XDBE XIM XPM
LUCID ZLIB

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

Major mode: Python

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-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
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils python json
subr-x map rx project pcase treesit cl-seq comint ansi-osc ring
cl-loaddefs cl-lib ansi-color term/screen term/xterm xterm byte-opt gv
bytecomp byte-compile rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine 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
theme-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 dynamic-setting
system-font-setting font-render-setting cairo x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 60054 7620)
 (symbols 48 7362 0)
 (strings 32 20143 1278)
 (string-bytes 1 593570)
 (vectors 16 11583)
 (vector-slots 8 139242 9455)
 (floats 8 30 11543)
 (intervals 56 239 0)
 (buffers 984 12))

[-- Attachment #2: 0001-Make-case-keyword-a-dedenter-in-Python.patch --]
[-- Type: application/octet-stream, Size: 2736 bytes --]

From 93801015034ee6e5072051641acab227de74082a Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Fri, 10 Mar 2023 14:26:22 +0900
Subject: [PATCH] Make "case" keyword a dedenter in Python

* lisp/progmodes/python.el (python-rx): Add "case" to dedenter.
(python-info-dedenter-opening-block-positions): Add "case" to pairs.
* test/lisp/progmodes/python-tests.el (python-indent-dedenters-9):
New test.
---
 lisp/progmodes/python.el            |  5 +++--
 test/lisp/progmodes/python-tests.el | 15 +++++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 630250c15c3..2d85d059d97 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -426,7 +426,7 @@ python-rx
                                              (or "def" "for" "with")))
                                     symbol-end))
             (dedenter          (seq symbol-start
-                                    (or "elif" "else" "except" "finally")
+                                    (or "elif" "else" "except" "finally" "case")
                                     symbol-end))
             (block-ender       (seq symbol-start
                                     (or
@@ -5784,7 +5784,8 @@ python-info-dedenter-opening-block-positions
                (pairs '(("elif" "elif" "if")
                         ("else" "if" "elif" "except" "for" "while")
                         ("except" "except" "try")
-                        ("finally" "else" "except" "try")))
+                        ("finally" "else" "except" "try")
+                        ("case" "case")))
                (dedenter (match-string-no-properties 0))
                (possible-opening-blocks (cdr (assoc-string dedenter pairs)))
                (collected-indentations)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 6928e313dc4..7bb6f8617e7 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -1658,6 +1658,21 @@ python-indent-dedenters-8
    (python-indent-line t)
    (should (= (python-indent-calculate-indentation t) 6))))
 
+(ert-deftest python-indent-dedenters-9 ()
+  "Test de-indentation for the case keyword."
+  (python-tests-with-temp-buffer
+   "
+match a:
+    case 1:
+        print(1)
+        case 2
+"
+   (python-tests-look-at "case 2")
+   (should (eq (car (python-indent-context)) :at-dedenter-block-start))
+   (should (= (python-indent-calculate-indentation) 4))
+   (python-indent-line t)
+   (should (= (python-indent-calculate-indentation t) 4))))
+
 (ert-deftest python-indent-inside-string-1 ()
   "Test indentation for strings."
   (python-tests-with-temp-buffer
-- 
2.34.1

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[Eli Zaretskii]

> Cc: Lele Gaifax <lele@metapensiero.it>
> Date: Fri, 10 Mar 2023 14:33:14 +0900
> From: kobarity <kobarity@gmail.com>
> 
> New keywords "match" and "case" were added to block-start in
> `python-rx', which enables basic indentation support for these
> keywords.  However, "case" does not work as a dedenter.  For example,
> try the following code.

Could you please tell in which version of Emacs did we add those new
keywords?  It's important for deciding on which branch to fix the
problem.

Thanks.

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[kobarity]

Eli Zaretskii wrote:
> Could you please tell in which version of Emacs did we add those new
> keywords?  It's important for deciding on which branch to fix the
> problem.

Sorry for the lack of information.  Those keywords were added in
Bug#55572.  The commit ID is 35d0190b0b91c085c73bbe6c2b8e93ea8288b589.
So I think it's only for Emacs 29.

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[Dmitry Gutov]

On 10/03/2023 07:33, kobarity wrote:
> Attached is a patch to make "case" a dedenter with an ERT.

Here's a minor issue with it, probably not critical:

I type the colon after 'case 2', see it indented correctly, but also a 
message in the echo area:

   Closes case 1:

I type the next case with colon and see:

   Closes case 2:

and so on. They don't do much harm, but they still seem semantically 
incorrect.

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[kobarity]

Dmitry Gutov wrote:
> I type the colon after 'case 2', see it indented correctly, but also a
> message in the echo area:
> 
>   Closes case 1:
> 
> I type the next case with colon and see:
> 
>   Closes case 2:
> 
> and so on. They don't do much harm, but they still seem semantically
> incorrect.

Thank you for testing my patch.  I don't think this is a incorrect
message.  Let's discuss with the following code:

#+begin_src python
match a:
    case 1:
        print("a = 1")
        match b:
            case 2:
                print("b = 2")
                case 3
#+end_src

When I type ":" at the last line after "case 3", it will be indented
as follows:

#+begin_src python
match a:
    case 1:
        print("a = 1")
        match b:
            case 2:
                print("b = 2")
            case 3:
#+end_src

The message "Closes case 2:" is shown.  It means that the "case 3:"
corresponds to the "case 2:" in the inner "match" statement, not the
"case 1:" in the outer "match" statement.

When I type TAB twice, the line will be indented as follows:

#+begin_src python
match a:
    case 1:
        print("a = 1")
        match b:
            case 2:
                print("b = 2")
    case 3:
#+end_src

The message "Closes case 1:" is shown.  It is shown to help
distinguishing nested blocks, although it will not be useful if the
case statements are same.

This is the same behavior as "if" blocks, etc.  The term "Closes" may
not be the most appropriate for "case," but I don't think it is wrong.

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[Dmitry Gutov]

On 11/03/2023 04:08, kobarity wrote:

> The message "Closes case 1:" is shown.  It is shown to help
> distinguishing nested blocks, although it will not be useful if the
> case statements are same.
> 
> This is the same behavior as "if" blocks, etc.  The term "Closes" may
> not be the most appropriate for "case," but I don't think it is wrong.

Fair enough. I do think it's not ideal ("case" is not a "closer"), but 
neither is "else", for that matter. So the issue is not new.

And it can be useful, like you say, to distinguish nested blocks.

Pushed your change to emacs-29, and closing. Thanks!

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[kobarity]

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


Dmitry Gutov wrote:
> Pushed your change to emacs-29, and closing. Thanks!

Thank you for applying the patch.  However, I overlooked one problem.

#+begin_src python
match a:
    case 1:
        match b:
            case 2
#+end_src

When I type ":" at the last line after "case 2", the message "Closes
case 1:" is shown.  This is wrong because it cannot be the "case"
block of the outer "match" block (match a:).  I'm sorry if you were
mentioning this case.

Similar message can be shown with "elif":

#+begin_src python
for c in (1, 2):
    if a == 1:
        for d in (3, 4):
            elif b == 1:
#+end_src

However, this is not a correct Python code because "elif" cannot be
the first sentence in a block.

Dedenters other than "case" can never be the first sentence of a
block, whereas "case" can be the first sentence of a block.

So I think it is appropriate that the first "case" in the block should
not be considered a dedenter.  Attached is a patch to add a condition
in `python-info-dedenter-statement-p'.

[-- Attachment #2: 0001-Improve-indenting-case-in-Python.patch --]
[-- Type: application/octet-stream, Size: 3517 bytes --]

From f0dafe85cb78b0510e70fe7b1d093d013aea9008 Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Sat, 25 Mar 2023 22:59:05 +0900
Subject: [PATCH] Improve indenting "case" in Python

* lisp/progmodes/python.el (python-info-dedenter-statement-p): Do not
consider the first "case" in the block as dedenter.
* test/lisp/progmodes/python-tests.el
(python-info-dedenter-opening-block-positions-7)
(python-info-dedenter-statement-p-6): New tests. (Bug#62092)
---
 lisp/progmodes/python.el            |  9 ++++++-
 test/lisp/progmodes/python-tests.el | 38 +++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 2fe88323c35..bbabce80b4d 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -5854,7 +5854,14 @@ python-info-dedenter-statement-p
   (save-excursion
     (python-nav-beginning-of-statement)
     (when (and (not (python-syntax-context-type))
-               (looking-at (python-rx dedenter)))
+               (looking-at (python-rx dedenter))
+               ;; Exclude the first "case" in the block.
+               (not (and (string= (match-string-no-properties 0)
+                                  "case")
+                         (save-excursion
+                           (back-to-indentation)
+                           (python-util-forward-comment -1)
+                           (equal (char-before) ?:)))))
       (point))))
 
 (defun python-info-line-ends-backslash-p (&optional line-number)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index ed4a08da6ab..50153e66da5 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5940,6 +5940,26 @@ python-info-dedenter-opening-block-positions-6
      (equal (list (python-tests-look-at "if (" -1 t))
             (python-info-dedenter-opening-block-positions)))))
 
+(ert-deftest python-info-dedenter-opening-block-positions-7 ()
+  "Test case blocks."
+  (python-tests-with-temp-buffer
+   "
+match a:
+    case 1:
+        match b:
+            case 2:
+                something()
+            case 3:
+"
+   (python-tests-look-at "case 1:")
+   (should-not (python-info-dedenter-opening-block-positions))
+   (python-tests-look-at "case 2:")
+   (should-not (python-info-dedenter-opening-block-positions))
+   (python-tests-look-at "case 3:")
+   (equal (list (python-tests-look-at "case 2:" -1)
+                (python-tests-look-at "case 1:" -1 t))
+            (python-info-dedenter-opening-block-positions))))
+
 (ert-deftest python-info-dedenter-opening-block-message-1 ()
   "Test dedenters inside strings are ignored."
   (python-tests-with-temp-buffer
@@ -6125,6 +6145,24 @@ python-info-dedenter-statement-p-5
                  (point))
                (python-info-dedenter-statement-p)))))
 
+(ert-deftest python-info-dedenter-statement-p-6 ()
+  "Test case keyword."
+  (python-tests-with-temp-buffer
+      "
+match a:  # Comment
+    case 1:
+        match b:
+            case 2:
+                something()
+            case 3:
+"
+    (python-tests-look-at "case 1:")
+    (should-not (python-info-dedenter-statement-p))
+    (python-tests-look-at "case 2:")
+    (should-not (python-info-dedenter-statement-p))
+    (python-tests-look-at "case 3:")
+    (should (= (point) (python-info-dedenter-statement-p)))))
+
 (ert-deftest python-info-line-ends-backslash-p-1 ()
   (python-tests-with-temp-buffer
    "
-- 
2.34.1

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[Dmitry Gutov]

On 25/03/2023 16:13, kobarity wrote:
> 
> Dmitry Gutov wrote:
>> Pushed your change to emacs-29, and closing. Thanks!
> 
> Thank you for applying the patch.  However, I overlooked one problem.
> 
> #+begin_src python
> match a:
>      case 1:
>          match b:
>              case 2
> #+end_src
> 
> When I type ":" at the last line after "case 2", the message "Closes
> case 1:" is shown.  This is wrong because it cannot be the "case"
> block of the outer "match" block (match a:).  I'm sorry if you were
> mentioning this case.

Not really, I wasn't thinking about this one in particular.

> Similar message can be shown with "elif":
> 
> #+begin_src python
> for c in (1, 2):
>      if a == 1:
>          for d in (3, 4):
>              elif b == 1:
> #+end_src
> 
> However, this is not a correct Python code because "elif" cannot be
> the first sentence in a block.
> 
> Dedenters other than "case" can never be the first sentence of a
> block, whereas "case" can be the first sentence of a block.
> 
> So I think it is appropriate that the first "case" in the block should
> not be considered a dedenter.  Attached is a patch to add a condition
> in `python-info-dedenter-statement-p'.

Thanks! I've pushed it to emacs-29 as well.

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[Mattias Engdegård]

The test python-info-dedenter-opening-block-positions-7 added as part of this bug in 8f42db010d ends with

+   (equal (list (python-tests-look-at "case 2:" -1)
+                (python-tests-look-at "case 1:" -1 t))
+            (python-info-dedenter-opening-block-positions))))

which, as you can see, isn't actually a check; the `equal` result is thrown away.
(Emacs master was just changed to detect this, so there is a byte-compiler warning.)

Unfortunately, wrapping the call in `should` fails, so there is more to it.
Would you please take a look?

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[kobarity]

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


Mattias Engdegård wrote:
> 
> The test python-info-dedenter-opening-block-positions-7 added as part of this bug in 8f42db010d ends with
> 
> +   (equal (list (python-tests-look-at "case 2:" -1)
> +                (python-tests-look-at "case 1:" -1 t))
> +            (python-info-dedenter-opening-block-positions))))
> 
> which, as you can see, isn't actually a check; the `equal` result is thrown away.
> (Emacs master was just changed to detect this, so there is a byte-compiler warning.)
> 
> Unfortunately, wrapping the call in `should` fails, so there is more to it.
> Would you please take a look?

Sorry, I meant to use `should'.  Another mistake was forgetting to use
RESTORE-POINT argument when calling `python-tests-look-at'.  Attached
is the patch to fix this.  It also includes the indentation fix for 
`python-info-dedenter-opening-block-positions-6'.

[-- Attachment #2: 0001-Fix-test-in-python-tests.el-added-by-bug-62092.patch --]
[-- Type: application/octet-stream, Size: 1943 bytes --]

From a39bc229ae258b04f4463c4cabfbe9f1c48ee7df Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Tue, 15 Aug 2023 22:27:45 +0900
Subject: [PATCH] Fix test in python-tests.el added by bug #62092

* test/lisp/progmodes/python-tests.el
(python-info-dedenter-opening-block-positions-6): Fix indentation.
(python-info-dedenter-opening-block-positions-7): Add calling
`should'.  Add RESTORE-POINT argument when calling
`python-tests-look-at'.
---
 test/lisp/progmodes/python-tests.el | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 54e32cbf07b..9f935f2748c 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -5973,9 +5973,9 @@ python-info-dedenter-opening-block-positions-6
         else
 "
    (python-tests-look-at "else\n")
-    (should
-     (equal (list (python-tests-look-at "if (" -1 t))
-            (python-info-dedenter-opening-block-positions)))))
+   (should
+    (equal (list (python-tests-look-at "if (" -1 t))
+           (python-info-dedenter-opening-block-positions)))))
 
 (ert-deftest python-info-dedenter-opening-block-positions-7 ()
   "Test case blocks."
@@ -5993,9 +5993,9 @@ python-info-dedenter-opening-block-positions-7
    (python-tests-look-at "case 2:")
    (should-not (python-info-dedenter-opening-block-positions))
    (python-tests-look-at "case 3:")
-   (equal (list (python-tests-look-at "case 2:" -1)
-                (python-tests-look-at "case 1:" -1 t))
-            (python-info-dedenter-opening-block-positions))))
+   (should (equal (list (python-tests-look-at "case 2:" -1 t)
+                        (python-tests-look-at "case 1:" -1 t))
+                  (python-info-dedenter-opening-block-positions)))))
 
 (ert-deftest python-info-dedenter-opening-block-message-1 ()
   "Test dedenters inside strings are ignored."
-- 
2.34.1

bug#62092: 30.0.50; "case" does not work as dedenter in Python

[Mattias Engdegård]

15 aug. 2023 kl. 15.43 skrev kobarity <kobarity@gmail.com>:

> Sorry, I meant to use `should'.  Another mistake was forgetting to use
> RESTORE-POINT argument when calling `python-tests-look-at'.  Attached
> is the patch to fix this.  It also includes the indentation fix for 
> `python-info-dedenter-opening-block-positions-6'.

Excellent, thank you! Applied to master.