bug#63959: python-mode does not keep indentation in square brackets []

bug#63959: python-mode does not keep indentation in square brackets []

[Konstantin Kharlamov]

Usually in programming modes, when previous indentation is kind of "special",
the new lines should keep the indentation from the previous line. However, it
doesn't work in this case.


# Steps to reproduce

1. Create file `test.py` with following content:

    for infix in [ # some description
                  "_cdata", "_cmeta", "_corig", "_cpool", "_cvol", "_wcorig",
                  "indentation is broken here", "bar"]:
        print(infix)

2. Open it as `emacs -Q test.py`
3. Put a caret on the 3rd line (which says "indentation is broken"
4. Press TAB


## Expected

Indentation won't change

## Actual

The line goes back by 4 spaces or so

# Additional information

emacs version: compiled from latest git a week ago, commit 5cace109d2b

bug#63959: python-mode does not keep indentation in square brackets []

[Eli Zaretskii]

> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Date: Thu, 08 Jun 2023 12:39:23 +0300
> 
> Usually in programming modes, when previous indentation is kind of "special",
> the new lines should keep the indentation from the previous line. However, it
> doesn't work in this case.
> 
> 
> # Steps to reproduce
> 
> 1. Create file `test.py` with following content:
> 
>     for infix in [ # some description
>                   "_cdata", "_cmeta", "_corig", "_cpool", "_cvol", "_wcorig",
>                   "indentation is broken here", "bar"]:
>         print(infix)
> 
> 2. Open it as `emacs -Q test.py`
> 3. Put a caret on the 3rd line (which says "indentation is broken"
> 4. Press TAB
> 
> 
> ## Expected
> 
> Indentation won't change
> 
> ## Actual
> 
> The line goes back by 4 spaces or so
> 
> # Additional information
> 
> emacs version: compiled from latest git a week ago, commit 5cace109d2b

kobarity, any comments?

bug#63959: python-mode does not keep indentation in square brackets []

[kobarity]

Eli Zaretskii wrote:
> > From: Konstantin Kharlamov <hi-angel@yandex.ru>
> > Date: Thu, 08 Jun 2023 12:39:23 +0300
> > 
> > Usually in programming modes, when previous indentation is kind of "special",
> > the new lines should keep the indentation from the previous line. However, it
> > doesn't work in this case.
> > 
> > 
> > # Steps to reproduce
> > 
> > 1. Create file `test.py` with following content:
> > 
> >     for infix in [ # some description
> >                   "_cdata", "_cmeta", "_corig", "_cpool", "_cvol", "_wcorig",
> >                   "indentation is broken here", "bar"]:
> >         print(infix)
> > 
> > 2. Open it as `emacs -Q test.py`
> > 3. Put a caret on the 3rd line (which says "indentation is broken"
> > 4. Press TAB
> > 
> > 
> > ## Expected
> > 
> > Indentation won't change
> > 
> > ## Actual
> > 
> > The line goes back by 4 spaces or so
> > 
> > # Additional information
> > 
> > emacs version: compiled from latest git a week ago, commit 5cace109d2b
> 
> kobarity, any comments?

I think the current Python mode tries to indent based on parens,
regardless of the indentation of the previous line.  However, it would
also be reasonable to maintain the indentation of the previous line.
I will see if I can implement it.

bug#63959: python-mode does not keep indentation in square brackets []

[kobarity]

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


I wrote:
> Eli Zaretskii wrote:
> > > From: Konstantin Kharlamov <hi-angel@yandex.ru>
> > > Date: Thu, 08 Jun 2023 12:39:23 +0300
> > > 
> > > Usually in programming modes, when previous indentation is kind of "special",
> > > the new lines should keep the indentation from the previous line. However, it
> > > doesn't work in this case.
> > > 
> > > 
> > > # Steps to reproduce
> > > 
> > > 1. Create file `test.py` with following content:
> > > 
> > >     for infix in [ # some description
> > >                   "_cdata", "_cmeta", "_corig", "_cpool", "_cvol", "_wcorig",
> > >                   "indentation is broken here", "bar"]:
> > >         print(infix)
> > > 
> > > 2. Open it as `emacs -Q test.py`
> > > 3. Put a caret on the 3rd line (which says "indentation is broken"
> > > 4. Press TAB
> > > 
> > > 
> > > ## Expected
> > > 
> > > Indentation won't change
> > > 
> > > ## Actual
> > > 
> > > The line goes back by 4 spaces or so
> > > 
> > > # Additional information
> > > 
> > > emacs version: compiled from latest git a week ago, commit 5cace109d2b
> > 
> > kobarity, any comments?
> 
> I think the current Python mode tries to indent based on parens,
> regardless of the indentation of the previous line.  However, it would
> also be reasonable to maintain the indentation of the previous line.
> I will see if I can implement it.

Attached is a patch to implement it.  I introduced a new indent
context `:inside-paren-continuation-line' for the continuation lines
within paren.  The indent context respects the indentation of the
previous line.

However, it may happen that the previous line is indented for the
inner paren as in the ERT `python-indent-inside-paren-2':

data = {'key': {
    'objlist': [
        {'pk': 1,
         'name': 'first'},
        {'pk': 2,
         'name': 'second'}
    ]
}}

The line "{'pk': 2," is considered as the continuation line, but it
should not respect the indentation of the PREVIOUS line "'name':
'first'},".  So skipping such lines with inner parens were needed.
It searches backward for the line which starts with the item of the
same opening paren as the target line.

In the case of the above example, if the target line is "{'pk': 2,",
its opening paren is "[" in the line "'objlist': [".  It first checks
the previous line "'name': 'first'},", but its opening paren is "{" in
"{'pk': 1,".  So this line is skipped.  Next, it checks the line
"{'pk': 1," and its opening paren is "[" in the line "'objlist': [",
which is same as the target line.  So the target line's indentation
will be as same as the line "{'pk': 1,".

It would be helpful if you could try this patch.

Does anyone think we should have a customize variable that switches
between the traditional behavior of ignoring the indentation of the
previous line and this new behavior?

[-- Attachment #2: 0001-Fix-Python-indentation-of-continuation-lines-within-.patch --]
[-- Type: application/octet-stream, Size: 12125 bytes --]

From c3aac2626eb346b19f5128208dc6ca4b7ca7b6c3 Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Sun, 18 Jun 2023 23:47:25 +0900
Subject: [PATCH] Fix Python indentation of continuation lines within parens

* lisp/progmodes/python.el (python-indent-context): Add a new indent
context `:inside-paren-continuation-line'.
(python-indent--calculate-indentation): Use the new indent context.
* test/lisp/progmodes/python-tests.el (python-indent-pep8-2)
(python-indent-pep8-3)
(python-indent-inside-paren-1)
(python-indent-inside-paren-2)
(python-indent-inside-paren-3)
(python-indent-inside-paren-6)
(python-indent-after-backslash-2): Change to use the new indent
context.
(python-indent-inside-paren-8)
(python-indent-inside-paren-9): New tests. (Bug#63959)
---
 lisp/progmodes/python.el            | 26 ++++++++-
 test/lisp/progmodes/python-tests.el | 89 +++++++++++++++++++++++++----
 2 files changed, 101 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 26dafde7591..50d712ebb0c 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1406,6 +1406,10 @@ python-indent-context
  - Point is inside a paren from a block start followed by some
    items on the same line.
  - START is the first non space char position *after* the open paren.
+:inside-paren-continuation-line
+ - Point is on a continuation line inside a paren.
+ - START is the position where the previous line (excluding lines
+   for inner parens) starts.
 
 :after-backslash
  - Fallback case when point is after backslash.
@@ -1460,7 +1464,21 @@ python-indent-context
                      (= (line-number-at-pos)
                         (progn
                           (python-util-forward-comment)
-                          (line-number-at-pos))))))))
+                          (line-number-at-pos)))))))
+               (continuation-start
+                (when start
+                  (save-excursion
+                    (forward-line -1)
+                    (back-to-indentation)
+                    ;; Skip inner parens.
+                    (cl-loop with prev-start = (python-syntax-context 'paren)
+                             while (and prev-start (>= prev-start start))
+                             if (= prev-start start)
+                             return (point)
+                             else do (goto-char prev-start)
+                                     (back-to-indentation)
+                                     (setq prev-start
+                                           (python-syntax-context 'paren)))))))
           (when start
             (cond
              ;; Current line only holds the closing paren.
@@ -1476,6 +1494,9 @@ python-indent-context
                 (back-to-indentation)
                 (python-syntax-closing-paren-p))
               (cons :inside-paren-at-closing-nested-paren start))
+             ;; This line is a continuation of the previous line.
+             (continuation-start
+              (cons :inside-paren-continuation-line continuation-start))
              ;; This line starts from an opening block in its own line.
              ((save-excursion
                 (goto-char start)
@@ -1591,7 +1612,8 @@ python-indent--calculate-indentation
         (`(,(or :after-line
                 :after-comment
                 :inside-string
-                :after-backslash) . ,start)
+                :after-backslash
+                :inside-paren-continuation-line) . ,start)
          ;; Copy previous indentation.
          (goto-char start)
          (current-indentation))
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 9323f72f384..54e32cbf07b 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -683,7 +683,7 @@ python-indent-pep8-2
    (should (= (python-indent-calculate-indentation) 8))
    (python-tests-look-at "var_four):")
    (should (eq (car (python-indent-context))
-               :inside-paren-newline-start-from-block))
+               :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 8))
    (python-tests-look-at "print (var_one)")
    (should (eq (car (python-indent-context))
@@ -707,8 +707,8 @@ python-indent-pep8-3
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 4))
    (python-tests-look-at "var_three, var_four)")
-   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
-   (should (= (python-indent-calculate-indentation) 4))))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
+   (should (= (python-indent-calculate-indentation) 2))))
 
 (ert-deftest python-indent-hanging-close-paren ()
   "Like first pep8 case, but with hanging close paren." ;; See Bug#20742.
@@ -864,7 +864,7 @@ python-indent-inside-paren-1
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 4))
    (python-tests-look-at "{")
-   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 4))
    (python-tests-look-at "'objlist': [")
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
@@ -876,20 +876,20 @@ python-indent-inside-paren-1
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 16))
    (python-tests-look-at "'name': 'first',")
-   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 16))
    (python-tests-look-at "},")
    (should (eq (car (python-indent-context))
                :inside-paren-at-closing-nested-paren))
    (should (= (python-indent-calculate-indentation) 12))
    (python-tests-look-at "{")
-   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 12))
    (python-tests-look-at "'pk': 2,")
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 16))
    (python-tests-look-at "'name': 'second',")
-   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 16))
    (python-tests-look-at "}")
    (should (eq (car (python-indent-context))
@@ -933,7 +933,7 @@ python-indent-inside-paren-2
    (should (eq (car (python-indent-context)) :inside-paren))
    (should (= (python-indent-calculate-indentation) 9))
    (python-tests-look-at "{'pk': 2,")
-   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 8))
    (python-tests-look-at "'name': 'second'}")
    (should (eq (car (python-indent-context)) :inside-paren))
@@ -966,10 +966,10 @@ python-indent-inside-paren-3
    (should (eq (car (python-indent-context)) :inside-paren))
    (should (= (python-indent-calculate-indentation) 8))
    (forward-line 1)
-   (should (eq (car (python-indent-context)) :inside-paren))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 8))
    (forward-line 1)
-   (should (eq (car (python-indent-context)) :inside-paren))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 8))))
 
 (ert-deftest python-indent-inside-paren-4 ()
@@ -1023,7 +1023,7 @@ python-indent-inside-paren-6
    (should (eq (car (python-indent-context)) :inside-paren))
    (should (= (python-indent-calculate-indentation) 11))
    (forward-line 1)
-   (should (eq (car (python-indent-context)) :inside-paren))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 11))))
 
 (ert-deftest python-indent-inside-paren-7 ()
@@ -1034,6 +1034,71 @@ python-indent-inside-paren-7
    ;; This signals an error if the test fails
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))))
 
+(ert-deftest python-indent-inside-paren-8 ()
+  "Test for Bug#63959."
+  (python-tests-with-temp-buffer
+   "
+for a in [  # comment
+          'some',  # Manually indented.
+          'thing']:  # Respect indentation of the previous line.
+"
+   (python-tests-look-at "for a in [  # comment")
+   (should (eq (car (python-indent-context)) :no-indent))
+   (should (= (python-indent-calculate-indentation) 0))
+   (forward-line 1)
+   (should (eq (car (python-indent-context))
+               :inside-paren-newline-start-from-block))
+   (should (= (python-indent-calculate-indentation) 8))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
+   (should (= (python-indent-calculate-indentation) 10))))
+
+(ert-deftest python-indent-inside-paren-9 ()
+  "Test `:inside-paren-continuation-line'."
+  (python-tests-with-temp-buffer
+   "
+a = (((
+    1, 2),
+      3),  # Do not respect the indentation of the previous line
+     4)  # Do not respect the indentation of the previous line
+b = ((
+        1, 2),  # Manually indented
+     3,  # Do not respect the indentation of the previous line
+     4,  # Respect the indentation of the previous line
+        5,  # Manually indented
+        6)  # Respect the indentation of the previous line
+"
+   (python-tests-look-at "a = (((")
+   (should (eq (car (python-indent-context)) :no-indent))
+   (should (= (python-indent-calculate-indentation) 0))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (= (python-indent-calculate-indentation) 4))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren))
+   (should (= (python-indent-calculate-indentation) 6))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren))
+   (should (= (python-indent-calculate-indentation) 5))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :after-line))
+   (should (= (python-indent-calculate-indentation) 0))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (= (python-indent-calculate-indentation) 4))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren))
+   (should (= (python-indent-calculate-indentation) 5))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
+   (should (= (python-indent-calculate-indentation) 5))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
+   (should (= (python-indent-calculate-indentation) 5))
+   (forward-line 1)
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
+   (should (= (python-indent-calculate-indentation) 8))))
+
 (ert-deftest python-indent-inside-paren-block-1 ()
   "`python-indent-block-paren-deeper' set to nil (default).
 See Bug#62696."
@@ -1271,7 +1336,7 @@ python-indent-after-backslash-2
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 27))
    (python-tests-look-at "status='bought'")
-   (should (eq (car (python-indent-context)) :inside-paren-newline-start))
+   (should (eq (car (python-indent-context)) :inside-paren-continuation-line))
    (should (= (python-indent-calculate-indentation) 27))
    (python-tests-look-at ") \\")
    (should (eq (car (python-indent-context)) :inside-paren-at-closing-paren))
-- 
2.34.1

bug#63959: python-mode does not keep indentation in square brackets []

[Konstantin Kharlamov]

On Sun, 2023-06-18 at 23:56 +0900, kobarity wrote:
> 
> I wrote:
> > Eli Zaretskii wrote:
> > > > From: Konstantin Kharlamov <hi-angel@yandex.ru>
> > > > Date: Thu, 08 Jun 2023 12:39:23 +0300
> > > > 
> > > > Usually in programming modes, when previous indentation is kind of
> > > > "special",
> > > > the new lines should keep the indentation from the previous line.
> > > > However, it
> > > > doesn't work in this case.
> > > > 
> > > > 
> > > > # Steps to reproduce
> > > > 
> > > > 1. Create file `test.py` with following content:
> > > > 
> > > >     for infix in [ # some description
> > > >                   "_cdata", "_cmeta", "_corig", "_cpool", "_cvol",
> > > > "_wcorig",
> > > >                   "indentation is broken here", "bar"]:
> > > >         print(infix)
> > > > 
> > > > 2. Open it as `emacs -Q test.py`
> > > > 3. Put a caret on the 3rd line (which says "indentation is broken"
> > > > 4. Press TAB
> > > > 
> > > > 
> > > > ## Expected
> > > > 
> > > > Indentation won't change
> > > > 
> > > > ## Actual
> > > > 
> > > > The line goes back by 4 spaces or so
> > > > 
> > > > # Additional information
> > > > 
> > > > emacs version: compiled from latest git a week ago, commit 5cace109d2b
> > > 
> > > kobarity, any comments?
> > 
> > I think the current Python mode tries to indent based on parens,
> > regardless of the indentation of the previous line.  However, it would
> > also be reasonable to maintain the indentation of the previous line.
> > I will see if I can implement it.
> 
> Attached is a patch to implement it.  I introduced a new indent
> context `:inside-paren-continuation-line' for the continuation lines
> within paren.  The indent context respects the indentation of the
> previous line.
> 
> However, it may happen that the previous line is indented for the
> inner paren as in the ERT `python-indent-inside-paren-2':
> 
> data = {'key': {
>     'objlist': [
>         {'pk': 1,
>          'name': 'first'},
>         {'pk': 2,
>          'name': 'second'}
>     ]
> }}
> 
> The line "{'pk': 2," is considered as the continuation line, but it
> should not respect the indentation of the PREVIOUS line "'name':
> 'first'},".  So skipping such lines with inner parens were needed.
> It searches backward for the line which starts with the item of the
> same opening paren as the target line.
> 
> In the case of the above example, if the target line is "{'pk': 2,",
> its opening paren is "[" in the line "'objlist': [".  It first checks
> the previous line "'name': 'first'},", but its opening paren is "{" in
> "{'pk': 1,".  So this line is skipped.  Next, it checks the line
> "{'pk': 1," and its opening paren is "[" in the line "'objlist': [",
> which is same as the target line.  So the target line's indentation
> will be as same as the line "{'pk': 1,".
> 
> It would be helpful if you could try this patch.

Thank you, tested, works for me!

> Does anyone think we should have a customize variable that switches
> between the traditional behavior of ignoring the indentation of the
> previous line and this new behavior?

I doubt it's useful. It is in general how indentation works in many modes, and I
don't think there ever been a variable to disable that. Stefan Monnier in
particular has a paper called "SMIE: weakness is power", where such indentation
is shortly discussed.

bug#63959: python-mode does not keep indentation in square brackets []

[Andreas Röhler]

Am 18.06.23 um 17:20 schrieb Konstantin Kharlamov:
> On Sun, 2023-06-18 at 23:56 +0900, kobarity wrote:
>> I wrote:
>>> Eli Zaretskii wrote:
>>>>> From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>>>> Date: Thu, 08 Jun 2023 12:39:23 +0300
>>>>>
>>>>> Usually in programming modes, when previous indentation is kind of
>>>>> "special",
>>>>> the new lines should keep the indentation from the previous line.
>>>>> However, it
>>>>> doesn't work in this case.
>>>>>
>>>>>
>>>>> # Steps to reproduce
>>>>>
>>>>> 1. Create file `test.py` with following content:
>>>>>
>>>>>      for infix in [ # some description
>>>>>                    "_cdata", "_cmeta", "_corig", "_cpool", "_cvol",
>>>>> "_wcorig",
>>>>>                    "indentation is broken here", "bar"]:
>>>>>          print(infix)
>>>>>
>>>>> 2. Open it as `emacs -Q test.py`
>>>>> 3. Put a caret on the 3rd line (which says "indentation is broken"
>>>>> 4. Press TAB
>>>>>
>>>>>
>>>>> ## Expected
>>>>>
>>>>> Indentation won't change
>>>>>
>>>>> ## Actual
>>>>>
>>>>> The line goes back by 4 spaces or so
>>>>>
>>>>> # Additional information
>>>>>
>>>>> emacs version: compiled from latest git a week ago, commit 5cace109d2b
>>>> kobarity, any comments?
>>> I think the current Python mode tries to indent based on parens,
>>> regardless of the indentation of the previous line.  However, it would
>>> also be reasonable to maintain the indentation of the previous line.
>>> I will see if I can implement it.
>> Attached is a patch to implement it.  I introduced a new indent
>> context `:inside-paren-continuation-line' for the continuation lines
>> within paren.  The indent context respects the indentation of the
>> previous line.
>>
>> However, it may happen that the previous line is indented for the
>> inner paren as in the ERT `python-indent-inside-paren-2':
>>
>> data = {'key': {
>>      'objlist': [
>>          {'pk': 1,
>>           'name': 'first'},
>>          {'pk': 2,
>>           'name': 'second'}
>>      ]
>> }}
>>
>> The line "{'pk': 2," is considered as the continuation line, but it
>> should not respect the indentation of the PREVIOUS line "'name':
>> 'first'},".  So skipping such lines with inner parens were needed.
>> It searches backward for the line which starts with the item of the
>> same opening paren as the target line.
>>
>> In the case of the above example, if the target line is "{'pk': 2,",
>> its opening paren is "[" in the line "'objlist': [".  It first checks
>> the previous line "'name': 'first'},", but its opening paren is "{" in
>> "{'pk': 1,".  So this line is skipped.  Next, it checks the line
>> "{'pk': 1," and its opening paren is "[" in the line "'objlist': [",
>> which is same as the target line.  So the target line's indentation
>> will be as same as the line "{'pk': 1,".
>>
>> It would be helpful if you could try this patch.
> Thank you, tested, works for me!
>
>> Does anyone think we should have a customize variable that switches
>> between the traditional behavior of ignoring the indentation of the
>> previous line and this new behavior?
> I doubt it's useful. It is in general how indentation works in many modes, and I
> don't think there ever been a variable to disable that. Stefan Monnier in
> particular has a paper called "SMIE: weakness is power", where such indentation
> is shortly discussed.
>
>
> IMO the question of previous lines indentation isn't raised here, as a more specific rule applies: inside a non-empty list
> indent with its first element. Whereas if list starts empty, as with "[" here, next indents as that line plus offset.

bug#63959: python-mode does not keep indentation in square brackets []

[kobarity]

Andreas Röhler wrote:
> >> However, it may happen that the previous line is indented for the
> >> inner paren as in the ERT `python-indent-inside-paren-2':
> >> 
> >> data = {'key': {
> >>      'objlist': [
> >>          {'pk': 1,
> >>           'name': 'first'},
> >>          {'pk': 2,
> >>           'name': 'second'}
> >>      ]
> >> }}
> >> 
> >> The line "{'pk': 2," is considered as the continuation line, but it
> >> should not respect the indentation of the PREVIOUS line "'name':
> >> 'first'},".  So skipping such lines with inner parens were needed.
> >> It searches backward for the line which starts with the item of the
> >> same opening paren as the target line.
> >> 
> >> In the case of the above example, if the target line is "{'pk': 2,",
> >> its opening paren is "[" in the line "'objlist': [".  It first checks
> >> the previous line "'name': 'first'},", but its opening paren is "{" in
> >> "{'pk': 1,".  So this line is skipped.  Next, it checks the line
> >> "{'pk': 1," and its opening paren is "[" in the line "'objlist': [",
> >> which is same as the target line.  So the target line's indentation
> >> will be as same as the line "{'pk': 1,".
> >> 
> >> It would be helpful if you could try this patch.
> > Thank you, tested, works for me!
> > 
> >> Does anyone think we should have a customize variable that switches
> >> between the traditional behavior of ignoring the indentation of the
> >> previous line and this new behavior?
> > I doubt it's useful. It is in general how indentation works in many modes, and I
> > don't think there ever been a variable to disable that. Stefan Monnier in
> > particular has a paper called "SMIE: weakness is power", where such indentation
> > is shortly discussed.
> > 
> > 
> > IMO the question of previous lines indentation isn't raised here, as a more specific rule applies: inside a non-empty list
> > indent with its first element. Whereas if list starts empty, as with "[" here, next indents as that line plus offset.

Let me explain it again.  I added the line numbers.

1: data = {'key': {
2:     'objlist': [
3:         {'pk': 1,
4:          'name': 'first'},
5:         {'pk': 2,
6:          'name': 'second'}
7:     ]
8: }}

I think you are saying that the third line will be indented with an
additional offset to the second line, am I correct?  If so, I totally
agree with you.  This behavior has not changed with my patch.

What I wanted to say in the previous mail is that the fifth line
should align with the third line, not the fourth line.

As for such standard indentation, my patch does not change the
behavior.

However, if the user intentionally changed the indentation of the
third line, the current Python mode would indent the fourth and
subsequent lines as follows:

1: data = {'key': {
2:     'objlist': [
3:             {'pk': 1,  # Intentionally changed
4:              'name': 'first'},
5:         {'pk': 2,
6:          'name': 'second'}
7:     ]
8: }}

After applying my patch, it is indented as follows:

1: data = {'key': {
2:     'objlist': [
3:             {'pk': 1,  # Intentionally changed
4:              'name': 'first'},
5:             {'pk': 2,
6:              'name': 'second'}
7:     ]
8: }}

bug#63959: python-mode does not keep indentation in square brackets []

[Andreas Röhler]

Il 24.06.23 14:37, kobarity ha scritto:
> Andreas Röhler wrote:
>>>> However, it may happen that the previous line is indented for the
>>>> inner paren as in the ERT `python-indent-inside-paren-2':
>>>>
>>>> data = {'key': {
>>>>       'objlist': [
>>>>           {'pk': 1,
>>>>            'name': 'first'},
>>>>           {'pk': 2,
>>>>            'name': 'second'}
>>>>       ]
>>>> }}
>>>>
>>>> The line "{'pk': 2," is considered as the continuation line, but it
>>>> should not respect the indentation of the PREVIOUS line "'name':
>>>> 'first'},".  So skipping such lines with inner parens were needed.
>>>> It searches backward for the line which starts with the item of the
>>>> same opening paren as the target line.
>>>>
>>>> In the case of the above example, if the target line is "{'pk': 2,",
>>>> its opening paren is "[" in the line "'objlist': [".  It first checks
>>>> the previous line "'name': 'first'},", but its opening paren is "{" in
>>>> "{'pk': 1,".  So this line is skipped.  Next, it checks the line
>>>> "{'pk': 1," and its opening paren is "[" in the line "'objlist': [",
>>>> which is same as the target line.  So the target line's indentation
>>>> will be as same as the line "{'pk': 1,".
>>>>
>>>> It would be helpful if you could try this patch.
>>> Thank you, tested, works for me!
>>>
>>>> Does anyone think we should have a customize variable that switches
>>>> between the traditional behavior of ignoring the indentation of the
>>>> previous line and this new behavior?
>>> I doubt it's useful. It is in general how indentation works in many modes, and I
>>> don't think there ever been a variable to disable that. Stefan Monnier in
>>> particular has a paper called "SMIE: weakness is power", where such indentation
>>> is shortly discussed.
>>>
>>>
>>> IMO the question of previous lines indentation isn't raised here, as a more specific rule applies: inside a non-empty list
>>> indent with its first element. Whereas if list starts empty, as with "[" here, next indents as that line plus offset.
> Let me explain it again.  I added the line numbers.
>
> 1: data = {'key': {
> 2:     'objlist': [
> 3:         {'pk': 1,
> 4:          'name': 'first'},
> 5:         {'pk': 2,
> 6:          'name': 'second'}
> 7:     ]
> 8: }}
>
> I think you are saying that the third line will be indented with an
> additional offset to the second line, am I correct?  If so, I totally
> agree with you.  This behavior has not changed with my patch.
>
> What I wanted to say in the previous mail is that the fifth line
> should align with the third line, not the fourth line.
>
> As for such standard indentation, my patch does not change the
> behavior.
>
> However, if the user intentionally changed the indentation of the
> third line, the current Python mode would indent the fourth and
> subsequent lines as follows:
>
> 1: data = {'key': {
> 2:     'objlist': [
> 3:             {'pk': 1,  # Intentionally changed
> 4:              'name': 'first'},
> 5:         {'pk': 2,
> 6:          'name': 'second'}
> 7:     ]
> 8: }}
>
> After applying my patch, it is indented as follows:
>
> 1: data = {'key': {
> 2:     'objlist': [
> 3:             {'pk': 1,  # Intentionally changed
> 4:              'name': 'first'},
> 5:             {'pk': 2,
> 6:              'name': 'second'}
> 7:     ]
> 8: }}


Thanks for your explanation, which makes me better understand your 
endeavour. A question remains: is this new feature worth that possibly 
raise of complexity? Your patch provides a higher degree of freedom 
while keeping regularity  - which is a pro.

bug#63959: python-mode does not keep indentation in square brackets []

[kobarity]

Andreas Röhler wrote:
> Thanks for your explanation, which makes me better understand your
> endeavour. A question remains: is this new feature worth that possibly
> raise of complexity? Your patch provides a higher degree of freedom
> while keeping regularity  - which is a pro.

I think it is natural for many people to indent the same as the
previous line.  In fact, it is standard behavior outside the parens.
For example, if you intentionally change the indentation of the first
line of a block, the following lines will have the same indentation:

def func():
        a = 1  # Intentionally changed.
        b = 2  # Same indent as previous line

You can even do the following, although it will result in an
IndentationError when executed:

def func():
    a = 1
        b = 2  # Intentionally changed.
        c = 3  # Same indent as previous line

If inside the parens, the following will not result in an
IndentationError:

a = (
    1,
        2,  # Intentionally changed.
        3)  # Same indent as previous line

So I rather think the rule of indenting the same as the previous line
is more useful inside parens.

I would consider making the indentation the same as the previous line
inside the parens a fix rather than a new feature.  This is why I
named the commit "Fix Python indentation of continuation lines within
parens."

bug#63959: python-mode does not keep indentation in square brackets []

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I would consider making the indentation the same as the previous line
> inside the parens a fix rather than a new feature.

Definitely.  It might also fix a performance bug (when re-indenting
line by line a long parenthesized list of things, always going back to
the beginning can make the overall indentation O(N²) rather than O(N),
where N is the number of lines).


        Stefan

bug#63959: python-mode does not keep indentation in square brackets []

[Eli Zaretskii]

> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: 63959@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 18 Jun 2023 18:20:25 +0300
> 
> > It would be helpful if you could try this patch.
> 
> Thank you, tested, works for me!

Thanks for testing.  I've now installed this on the master branch, and
I'm marking the bug done.