bug#20742: 24.5; python mode indentation fails pep8

bug#20742: 24.5; python mode indentation fails pep8

[Tommi Komulainen]

The automatic indentation and manual reindenting with
indent-according-to-mode puts the closing bracket in wrong place that
makes pep8/flake8 flycheck complain:

print("hello",
      "world",
)
E124 closing bracket does not match visual indentation

The bracket needs to align with the first thing after the opening
bracket if it's on the same line i.e.:

print("hello",
      "world",
      )

The automatic indentation doesn't cause any errors if there's a newline
after the opening bracket though:

print(
    "hello",
    "world",
)



In GNU Emacs 24.5.1 (x86_64-redhat-linux-gnu, GTK+ Version 3.16.2)
 of 2015-04-22 on buildhw-10.phx2.fedoraproject.org
Windowing system distributor `Fedora Project', version 11.0.11701000
System Description: Fedora release 22 (Twenty Two)

Configured using:
 `configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3
 --with-gpm=no build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu 'CFLAGS=-DMAIL_USE_LOCKF -O2 -g
 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4
 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
 -m64 -mtune=generic' LDFLAGS=-Wl,-z,relro'

Important settings:
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Python

Minor modes in effect:
  tooltip-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
(New file)
Can't guess python-indent-offset, using defaults: 4
python-indent--calculate-indentation: Wrong type argument:
number-or-marker-p, nil [2 times]
Making completion list...
python-indent--calculate-indentation: Wrong type argument:
number-or-marker-p, nil [2 times]
Making completion list...
user-error: End of history; no default available [2 times]
Making completion list...

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
help-fns mail-prsvr mail-utils help-mode python easymenu json comint
ring cl-loaddefs cl-lib ansi-color time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 81480 5832)
 (symbols 48 18574 0)
 (miscs 40 39 124)
 (strings 32 12163 4670)
 (string-bytes 1 364259)
 (vectors 16 10283)
 (vector-slots 8 394463 9319)
 (floats 8 66 386)
 (intervals 56 243 1)
 (buffers 960 13)
 (heap 1024 32872 1101))

bug#20742: 24.5; python mode indentation fails pep8

[Glenn Morris]

Tommi Komulainen wrote:

> python-indent--calculate-indentation: Wrong type argument:
> number-or-marker-p, nil [2 times]

Perhaps the behaviour is not intended.
(Related to bug#20560?)

bug#20742: 24.5; python mode indentation fails pep8

[Tommi Komulainen]

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

That's almost certainly me having the cursor after the closing bracket
rather than on top of it while double-checking the indentation on each
line. Best I can tell it made no difference to the indentation.

The other bug is closely related when the opening bracket is *not* followed
by newline, though I can't immediately say if either of the possible
indentations is preferred or an error according to pep8.

On Fri, 5 Jun 2015 at 22:16 Glenn Morris <rgm@gnu.org> wrote:

> Tommi Komulainen wrote:
>
> > python-indent--calculate-indentation: Wrong type argument:
> > number-or-marker-p, nil [2 times]
>
> Perhaps the behaviour is not intended.
> (Related to bug#20560?)
>

[-- Attachment #2: Type: text/html, Size: 969 bytes --]

bug#20742: 24.5; python mode indentation fails pep8

[Andreas Röhler]

Am 05.06.2015 um 09:58 schrieb Tommi Komulainen:

May confirm that bug

GNU Emacs 25.0.50.1 (i686-pc-linux-gnu, GTK+ Version 2.24.23) of 2015-06-07

bug#20742: Testcase for #20742

[Tommi Komulainen]

[-- Attachment #1.1: Type: text/plain, Size: 100 bytes --]

Patch against emacs-24.5 branch to update the tests to check the closing
paren indentation as well.

[-- Attachment #1.2: Type: text/html, Size: 121 bytes --]

[-- Attachment #2: 0001-update-python-tests.el-for-pep8-indentation.patch --]
[-- Type: application/octet-stream, Size: 2717 bytes --]

From 37f56dd48ba6c21455681085579a872aeb267036 Mon Sep 17 00:00:00 2001
From: Tommi Komulainen <tommi.komulainen@iki.fi>
Date: Fri, 19 Jun 2015 18:53:52 +0200
Subject: [PATCH] update python-tests.el for pep8 indentation

Add (failing) test for closing paren indentation when opening paren is not
followed by newline.  #20742
---
 test/automated/python-tests.el | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/test/automated/python-tests.el b/test/automated/python-tests.el
index 42c26fc..b2631c4 100644
--- a/test/automated/python-tests.el
+++ b/test/automated/python-tests.el
@@ -172,15 +172,19 @@ aliqua."
   (python-tests-with-temp-buffer
    "# Aligned with opening delimiter
 foo = long_function_name(var_one, var_two,
-                         var_three, var_four)
+                         var_three, var_four,
+                         )
 "
    (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at "foo = long_function_name(var_one, var_two,")
    (should (eq (car (python-indent-context)) :after-comment))
    (should (= (python-indent-calculate-indentation) 0))
-   (python-tests-look-at "var_three, var_four)")
+   (python-tests-look-at "var_three, var_four")
    (should (eq (car (python-indent-context)) :inside-paren))
+   (should (= (python-indent-calculate-indentation) 25))
+   (python-tests-look-at ")")
+   (should (eq (car (python-indent-context)) :inside-paren-at-closing-paren))
    (should (= (python-indent-calculate-indentation) 25))))
 
 (ert-deftest python-indent-pep8-2 ()
@@ -216,7 +220,8 @@ def long_function_name(
    "# Extra indentation is not necessary.
 foo = long_function_name(
   var_one, var_two,
-  var_three, var_four)
+  var_three, var_four,
+)
 "
    (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
@@ -226,9 +231,12 @@ foo = long_function_name(
    (python-tests-look-at "var_one, var_two,")
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 4))
-   (python-tests-look-at "var_three, var_four)")
+   (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 (= (python-indent-calculate-indentation) 4))
+   (python-tests-look-at ")")
+   (should (eq (car (python-indent-context)) :inside-paren-at-closing-paren))
+   (should (= (python-indent-calculate-indentation) 0))))
 
 (ert-deftest python-indent-base-case ()
   "Check base case does not trigger errors."
-- 
2.4.4

bug#20742: 24.5; [PATCH] python.el: fix close paren indentation to match pep8

[Tommi Komulainen]

From da684af7265bf40e4140328c36011fea6b040373 Mon Sep 17 00:00:00 2001
From: Tommi Komulainen <tommi.komulainen@iki.fi>
Date: Fri, 19 Jun 2015 18:53:52 +0200
Subject: [PATCH] python.el: fix close paren indentation to match pep8

When opening paren is followed by newline the closing paren should follow
the current indentation. Otherwise the closing paren should be aligned
with the opening paren. This fixes the latter case. #20742
---
 lisp/progmodes/python.el       | 12 +++++++++---
 test/automated/python-tests.el | 42 +++++++++++++++++++++++++++++++++---------
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index de2d2d1..013a565 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -952,12 +952,18 @@ possibilities can be narrowed to specific
indentation points."
         (`(,(or :after-line
                 :after-comment
                 :inside-string
-                :after-backslash
-                :inside-paren-at-closing-paren
-                :inside-paren-at-closing-nested-paren) . ,start)
+                :after-backslash) . ,start)
          ;; Copy previous indentation.
          (goto-char start)
          (current-indentation))
+        (`(,(or :inside-paren-at-closing-paren
+                :inside-paren-at-closing-nested-paren) . ,start)
+         (goto-char (+ 1 start))
+         (if (looking-at "[ \t]*\\(?:#\\|$\\)")
+             ;; Copy previous indentation.
+             (current-indentation)
+           ;; Align with opening paren.
+           (current-column)))
         (`(,(or :after-block-start
                 :after-backslash-first-line
                 :inside-paren-newline-start) . ,start)
diff --git a/test/automated/python-tests.el b/test/automated/python-tests.el
index 42c26fc..f35f188 100644
--- a/test/automated/python-tests.el
+++ b/test/automated/python-tests.el
@@ -171,16 +171,28 @@ aliqua."
   "First pep8 case."
   (python-tests-with-temp-buffer
    "# Aligned with opening delimiter
-foo = long_function_name(var_one, var_two,
-                         var_three, var_four)
+foo = long_function_name(var_one=[var_two,  # comment
+                                  var_three
+                                  ],
+                         var_four=0,
+                         )
 "
    (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
-   (python-tests-look-at "foo = long_function_name(var_one, var_two,")
+   (python-tests-look-at "foo = long_function_name(var_one")
    (should (eq (car (python-indent-context)) :after-comment))
    (should (= (python-indent-calculate-indentation) 0))
-   (python-tests-look-at "var_three, var_four)")
+   (python-tests-look-at "var_three")
    (should (eq (car (python-indent-context)) :inside-paren))
+   (should (= (python-indent-calculate-indentation) 34))
+   (python-tests-look-at "]")
+   (should (eq (car (python-indent-context))
:inside-paren-at-closing-nested-paren))
+   (should (= (python-indent-calculate-indentation) 34))
+   (python-tests-look-at "var_four")
+   (should (eq (car (python-indent-context)) :inside-paren))
+   (should (= (python-indent-calculate-indentation) 25))
+   (python-tests-look-at ")")
+   (should (eq (car (python-indent-context)) :inside-paren-at-closing-paren))
    (should (= (python-indent-calculate-indentation) 25))))

 (ert-deftest python-indent-pep8-2 ()
@@ -215,20 +227,32 @@ def long_function_name(
   (python-tests-with-temp-buffer
    "# Extra indentation is not necessary.
 foo = long_function_name(
-  var_one, var_two,
-  var_three, var_four)
+    var_one, var_two=[  # comment
+        var_three,
+    ],
+    var_four=0,
+)
 "
    (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at "foo = long_function_name(")
    (should (eq (car (python-indent-context)) :after-comment))
    (should (= (python-indent-calculate-indentation) 0))
-   (python-tests-look-at "var_one, var_two,")
+   (python-tests-look-at "var_one")
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 4))
-   (python-tests-look-at "var_three, var_four)")
+   (python-tests-look-at "var_three")
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
-   (should (= (python-indent-calculate-indentation) 4))))
+   (should (= (python-indent-calculate-indentation) 8))
+   (python-tests-look-at "]")
+   (should (eq (car (python-indent-context))
:inside-paren-at-closing-nested-paren))
+   (should (= (python-indent-calculate-indentation) 4))
+   (python-tests-look-at "var_four")
+   (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-at-closing-paren))
+   (should (= (python-indent-calculate-indentation) 0))))

 (ert-deftest python-indent-base-case ()
   "Check base case does not trigger errors."
-- 
2.4.4

bug#20742: 24.5; [PATCH] python.el: fix close paren indentation to match pep8

[npostavs]

Tommi Komulainen <tommi.komulainen@iki.fi> writes:

>>From da684af7265bf40e4140328c36011fea6b040373 Mon Sep 17 00:00:00 2001
> From: Tommi Komulainen <tommi.komulainen@iki.fi>
> Date: Fri, 19 Jun 2015 18:53:52 +0200
> Subject: [PATCH] python.el: fix close paren indentation to match pep8
>
> When opening paren is followed by newline the closing paren should follow
> the current indentation. Otherwise the closing paren should be aligned
> with the opening paren. This fixes the latter case. #20742

I'm having trouble applying this.

    error: corrupt patch at line 11
    Applying: python.el: fix close paren indentation to match pep8
    Patch failed at 0001 python.el: fix close paren indentation to match pep8

bug#20742: 24.5; [PATCH] python.el: fix close paren indentation to match pep8

[Noam Postavsky]

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

tags 20742 + patch
quit

>> When opening paren is followed by newline the closing paren should follow
>> the current indentation. Otherwise the closing paren should be aligned
>> with the opening paren. This fixes the latter case. #20742
>
> I'm having trouble applying this.
>
>     error: corrupt patch at line 11
>     Applying: python.el: fix close paren indentation to match pep8
>     Patch failed at 0001 python.el: fix close paren indentation to match pep8

I managed to get this applied; I've replaced the python-test.el
modifications, partly because they're written as updates to the existing
tests which makes them a bit harder to follow, and partly to make more
clear that the patch is below the 15 line copyright limit.

I'll push to master in a week or so if there are no further comments.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1760 bytes --]

From e0acfb7254f97f623286e65483535c1b6312f4ba Mon Sep 17 00:00:00 2001
From: Tommi Komulainen <tommi.komulainen@iki.fi>
Date: Fri, 19 Jun 2015 18:53:52 +0200
Subject: [PATCH 1/2] python.el: Fix close paren indentation to match pep8
 (Bug#20742)

* lisp/progmodes/python.el (python-indent--calculate-indentation):
When opening paren is followed by newline the closing paren should
follow the current indentation.  Otherwise the closing paren should be
aligned with the opening paren.  This fixes the latter case.

Copyright-paperwork-exempt: yes
---
 lisp/progmodes/python.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index eb3e31c4b7..188bc973d9 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1070,12 +1070,18 @@ (defun python-indent--calculate-indentation ()
         (`(,(or :after-line
                 :after-comment
                 :inside-string
-                :after-backslash
-                :inside-paren-at-closing-paren
-                :inside-paren-at-closing-nested-paren) . ,start)
+                :after-backslash) . ,start)
          ;; Copy previous indentation.
          (goto-char start)
          (current-indentation))
+        (`(,(or :inside-paren-at-closing-paren
+                :inside-paren-at-closing-nested-paren) . ,start)
+         (goto-char (+ 1 start))
+         (if (looking-at "[ \t]*\\(?:#\\|$\\)")
+             ;; Copy previous indentation.
+             (current-indentation)
+           ;; Align with opening paren.
+           (current-column)))
         (`(:inside-docstring . ,start)
          (let* ((line-indentation (current-indentation))
                 (base-indent (progn
-- 
2.11.0


[-- Attachment #3: patch --]
[-- Type: text/plain, Size: 1436 bytes --]

From 8649d874d5158d91b0b6056bc66b9a99f4c1c31a Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 20 Apr 2019 15:31:51 -0400
Subject: [PATCH 2/2] ; Add test for previous change

* test/lisp/progmodes/python-tests.el (python-indent-hanging-close-paren):
New test.
---
 test/lisp/progmodes/python-tests.el | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index b940f45bb9..a517909717 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -260,6 +260,19 @@ (ert-deftest python-indent-pep8-3 ()
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
    (should (= (python-indent-calculate-indentation) 4))))
 
+(ert-deftest python-indent-hanging-close-paren ()
+  "Like first pep8 case, but with hanging close paren." ;; See Bug#20742.
+  (python-tests-with-temp-buffer
+   "\
+foo = long_function_name(var_one, var_two,
+                         var_three, var_four
+                         )
+"
+   (should (= (python-indent-calculate-indentation) 0))
+   (python-tests-look-at ")")
+   (should (eq (car (python-indent-context)) :inside-paren-at-closing-paren))
+   (should (= (python-indent-calculate-indentation) 25))))
+
 (ert-deftest python-indent-base-case ()
   "Check base case does not trigger errors."
   (python-tests-with-temp-buffer
-- 
2.11.0

bug#20742: 24.5; [PATCH] python.el: fix close paren indentation to match pep8

[Noam Postavsky]

tags 20742 fixed
close 20742 27.1
quit

> I'll push to master in a week or so if there are no further comments.

Done.

3fc1d77a43 2019-04-28T16:45:13-04:00 "python.el: Fix close paren indentation to match pep8 (Bug#20742)"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3fc1d77a43818666542740fc34167c1faee30d5f

bug#20560: bug#20742: 24.5; [PATCH] python.el: fix close paren indentation to match pep8

[Lars Ingebrigtsen]

Noam Postavsky <npostavs@gmail.com> writes:

> I managed to get this applied; I've replaced the python-test.el
> modifications, partly because they're written as updates to the existing
> tests which makes them a bit harder to follow, and partly to make more
> clear that the patch is below the 15 line copyright limit.
>
> I'll push to master in a week or so if there are no further comments.

Looks like this was done, so I'm closing this bug report.

commit 3fc1d77a43818666542740fc34167c1faee30d5f
Author:     Tommi Komulainen <tommi.komulainen@iki.fi>
AuthorDate: Fri Jun 19 18:53:52 2015 +0200
Commit:     Noam Postavsky <npostavs@gmail.com>
CommitDate: Sun Apr 28 16:45:13 2019 -0400

    python.el: Fix close paren indentation to match pep8 (Bug#20742)
    
    * lisp/progmodes/python.el (python-indent--calculate-indentation):
    When opening paren is followed by newline the closing paren should
    follow the current indentation.  Otherwise the closing paren should be
    aligned with the opening paren.  This fixes the latter case.


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

bug#20560: bug#20742: 24.5; [PATCH] python.el: fix close paren indentation to match pep8

[Noam Postavsky]

On Thu, 17 Sep 2020 at 14:59, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Looks like this was done, so I'm closing this bug report.

>     python.el: Fix close paren indentation to match pep8 (Bug#20742)
>
>     * lisp/progmodes/python.el (python-indent--calculate-indentation):
>     When opening paren is followed by newline the closing paren should
>     follow the current indentation.  Otherwise the closing paren should be
>     aligned with the opening paren.  This fixes the latter case.
                                                      ^^^^^^

You're mixing up bugs, I think. As far as I know, 20560/33979 isn't
fixed. I had closed 20742/19906 already when pushing that commit.

bug#20560: bug#20742: 24.5; [PATCH] python.el: fix close paren indentation to match pep8

[Lars Ingebrigtsen]

Noam Postavsky <npostavs@gmail.com> writes:

> You're mixing up bugs, I think. As far as I know, 20560/33979 isn't
> fixed. I had closed 20742/19906 already when pushing that commit.

OK; reopening.

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