* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
@ 2016-02-14 17:09 Matthew Woodcraft
2016-12-13 2:01 ` Hong Xu
0 siblings, 1 reply; 9+ messages in thread
From: Matthew Woodcraft @ 2016-02-14 17:09 UTC (permalink / raw)
To: 22661
If I have the following code and add the colon after the 'else',
python.el's electric-indent moves the 'else' to align with the inner
'if'.
def foo()
if aaa:
if bbb:
x = 1
y = 1
else
I think it would be much better to leave the else alone in this
situation (ie, when its current position matches an outer 'if'), for two
reasons:
- it's generally better to leave things alone than to make a guess
which is quite likely to be wrong;
- if the user has typed the whole thing in one go, then to reach this
situation they must have explicitly dedented the 'else' already.
(Seen in emacs 25.1 pretest; 24.5 is the same; checked with 'emacs -Q'.)
In GNU Emacs 25.0.91.1 (i586-pc-linux-gnu, GTK+ Version 3.14.5)
of 2016-02-14, modified by Debian built on golux
Windowing system distributor 'The X.Org Foundation', version 11.0.11604000
System Description: Debian GNU/Linux 8.3 (jessie)
Configured using:
'configure --build i586-linux-gnu --prefix=/usr
--sharedstatedir=/var/lib --libexecdir=/usr/lib
--localstatedir=/var/lib --infodir=/usr/share/info
--mandir=/usr/share/man --with-pop=yes
--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/25.0.91/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.0.91/site-lisp:/usr/share/emacs/site-lisp
--build i586-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
--libexecdir=/usr/lib --localstatedir=/var/lib
--infodir=/usr/share/info --mandir=/usr/share/man --with-pop=yes
--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/25.0.91/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.0.91/site-lisp:/usr/share/emacs/site-lisp
--with-x=yes --with-x-toolkit=gtk3 --with-toolkit-scroll-bars
'CFLAGS=-g -O2 -fstack-protector-strong -Wformat
-Werror=format-security -Wall -fno-omit-frame-pointer'
CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-z,relro'
Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11
Important settings:
value of $LC_CTYPE: en_GB.UTF-8
locale-coding-system: utf-8-unix
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2016-02-14 17:09 bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:' Matthew Woodcraft
@ 2016-12-13 2:01 ` Hong Xu
2017-01-21 17:34 ` Matthew Woodcraft
0 siblings, 1 reply; 9+ messages in thread
From: Hong Xu @ 2016-12-13 2:01 UTC (permalink / raw)
To: 22661; +Cc: matthew
[-- Attachment #1.1: Type: text/plain, Size: 336 bytes --]
> If I have the following code and add the colon after the 'else',
> python.el's electric-indent moves the 'else' to align with the inner
> 'if'.
> def foo()
> if aaa:
> if bbb:
> x = 1
> y = 1
> else
I've attached a patch to fix this issue: It is caused by incorrect
detection of opening blocks.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-python-mode-Fix-detection-for-opening-blocks.patch --]
[-- Type: text/x-diff, Size: 2415 bytes --]
From: Hong Xu <hong@topbug.net>
Date: Mon, 12 Dec 2016 17:55:25 -0800
Subject: [PATCH] python-mode: Fix detection for opening blocks (bug# 22661).
* python.el (python-info-dedenter-opening-block-positions): There
can't be any back-indented lines between an opening block and the
current line.
---
lisp/progmodes/python.el | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d0d4a7f766ef..8d3bd3194151 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4660,7 +4660,8 @@ python-info-dedenter-opening-block-positions
(let ((dedenter-pos (python-info-dedenter-statement-p)))
(when dedenter-pos
(goto-char dedenter-pos)
- (let* ((pairs '(("elif" "elif" "if")
+ (let* ((cur-line (line-beginning-position))
+ (pairs '(("elif" "elif" "if")
("else" "if" "elif" "except" "for" "while")
("except" "except" "try")
("finally" "else" "except" "try")))
@@ -4676,7 +4677,18 @@ python-info-dedenter-opening-block-positions
(let ((indentation (current-indentation)))
(when (and (not (memq indentation collected-indentations))
(or (not collected-indentations)
- (< indentation (apply #'min collected-indentations))))
+ (< indentation (apply #'min collected-indentations)))
+ ;; There must be no line with indentation smaller than
+ ;; `indentation' between the found opening block and the
+ ;; current line, otherwise it is not an opening block.
+ (save-excursion
+ (forward-line)
+ (let ((no-back-indent t))
+ (while (and (< (point) cur-line)
+ (setq no-back-indent
+ (> (current-indentation) indentation)))
+ (forward-line))
+ no-back-indent)))
(setq collected-indentations
(cons indentation collected-indentations))
(when (member (match-string-no-properties 0)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply related [flat|nested] 9+ messages in thread* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2016-12-13 2:01 ` Hong Xu
@ 2017-01-21 17:34 ` Matthew Woodcraft
2017-01-22 2:08 ` Hong Xu
0 siblings, 1 reply; 9+ messages in thread
From: Matthew Woodcraft @ 2017-01-21 17:34 UTC (permalink / raw)
To: 22661, Hong Xu
Hong Xu <hong@topbug.net> writes:
>> If I have the following code and add the colon after the 'else',
>> python.el's electric-indent moves the 'else' to align with the inner
>> 'if'.
>> def foo()
>> if aaa:
>> if bbb:
>> x = 1
>> y = 1
>> else
> I've attached a patch to fix this issue: It is caused by incorrect
> detection of opening blocks.
>
> From: Hong Xu <hong@topbug.net>
> Date: Mon, 12 Dec 2016 17:55:25 -0800
> Subject: [PATCH] python-mode: Fix detection for opening blocks (bug# 22661).
>
> * python.el (python-info-dedenter-opening-block-positions): There
> can't be any back-indented lines between an opening block and the
> current line.
I have tested this patch and it does fix the case I posted.
But it misbehaves if I add a blank line before the 'else':
def foo()
if aaa:
if bbb:
x = 1
y = 1
else
Now when I add the colon after the 'else', electric-indent moves the
'else' to align with the 'def'.
I agree that python-info-dedenter-opening-block-positions is the right
thing to fix.
I think it's sufficient to look at the last nonblank line above the
current line, rather than all lines between the candidate opening block
and the current line.
-M-
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2017-01-21 17:34 ` Matthew Woodcraft
@ 2017-01-22 2:08 ` Hong Xu
2017-01-22 22:34 ` Matthew Woodcraft
2017-01-22 23:08 ` npostavs
0 siblings, 2 replies; 9+ messages in thread
From: Hong Xu @ 2017-01-22 2:08 UTC (permalink / raw)
To: Matthew Woodcraft; +Cc: 22661
[-- Attachment #1.1: Type: text/plain, Size: 774 bytes --]
On 2017-01-21 Sat 09:34 GMT-0800, Matthew Woodcraft <matthew@woodcraft.me.uk> wrote:
>
> I have tested this patch and it does fix the case I posted.
>
> But it misbehaves if I add a blank line before the 'else':
>
> def foo()
> if aaa:
> if bbb:
> x = 1
> y = 1
>
> else
>
> Now when I add the colon after the 'else', electric-indent moves the
> 'else' to align with the 'def'.
>
>
> I agree that python-info-dedenter-opening-block-positions is the right
> thing to fix.
>
> I think it's sufficient to look at the last nonblank line above the
> current line, rather than all lines between the candidate opening block
> and the current line.
>
> -M-
Thanks for your suggestion, Matthew. I've now fixed it in the new patch
as attached.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-python-mode-Fix-detection-for-opening-blocks.patch --]
[-- Type: text/x-diff, Size: 2795 bytes --]
From b9b742e15174de6abe5134427964fe4e0f852461 Mon Sep 17 00:00:00 2001
From: Hong Xu <hong@topbug.net>
Date: Mon, 12 Dec 2016 17:55:25 -0800
Subject: [PATCH] python-mode: Fix detection for opening blocks.
* python.el (python-info-dedenter-opening-block-positions): There
can't be any back-indented lines between an opening block and the
current line.
---
lisp/progmodes/python.el | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 306402d8e3b4..ae9b0890cfc2 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4659,7 +4659,8 @@ python-info-dedenter-opening-block-positions
(let ((dedenter-pos (python-info-dedenter-statement-p)))
(when dedenter-pos
(goto-char dedenter-pos)
- (let* ((pairs '(("elif" "elif" "if")
+ (let* ((cur-line (line-beginning-position))
+ (pairs '(("elif" "elif" "if")
("else" "if" "elif" "except" "for" "while")
("except" "except" "try")
("finally" "else" "except" "try")))
@@ -4675,7 +4676,22 @@ python-info-dedenter-opening-block-positions
(let ((indentation (current-indentation)))
(when (and (not (memq indentation collected-indentations))
(or (not collected-indentations)
- (< indentation (apply #'min collected-indentations))))
+ (< indentation (apply #'min collected-indentations)))
+ ;; There must be no line with indentation
+ ;; smaller than `indentation' (except for
+ ;; blank lines) between the found opening
+ ;; block and the current line, otherwise it
+ ;; is not an opening block.
+ (save-excursion
+ (forward-line)
+ (let ((no-back-indent t))
+ (while (and (< (point) cur-line)
+ (setq no-back-indent
+ (or (> (current-indentation) indentation)
+ (save-match-data
+ (python-info-current-line-empty-p)))))
+ (forward-line))
+ no-back-indent)))
(setq collected-indentations
(cons indentation collected-indentations))
(when (member (match-string-no-properties 0)
--
2.1.4
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply related [flat|nested] 9+ messages in thread* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2017-01-22 2:08 ` Hong Xu
@ 2017-01-22 22:34 ` Matthew Woodcraft
2017-01-22 23:08 ` npostavs
1 sibling, 0 replies; 9+ messages in thread
From: Matthew Woodcraft @ 2017-01-22 22:34 UTC (permalink / raw)
To: Hong Xu, 22661
Hong Xu <hong@topbug.net> writes:
> Thanks for your suggestion, Matthew. I've now fixed it in the new patch
> as attached.
>
> From b9b742e15174de6abe5134427964fe4e0f852461 Mon Sep 17 00:00:00 2001
> From: Hong Xu <hong@topbug.net>
> Date: Mon, 12 Dec 2016 17:55:25 -0800
> Subject: [PATCH] python-mode: Fix detection for opening blocks.
>
> * python.el (python-info-dedenter-opening-block-positions): There
> can't be any back-indented lines between an opening block and the
> current line.
I have tested this version of the patch and it fixes both cases I
posted.
I have read the new code and it looks correct to me, but I don't claim
to be very familiar with python.el .
-M-
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2017-01-22 2:08 ` Hong Xu
2017-01-22 22:34 ` Matthew Woodcraft
@ 2017-01-22 23:08 ` npostavs
2017-01-23 0:50 ` Hong Xu
1 sibling, 1 reply; 9+ messages in thread
From: npostavs @ 2017-01-22 23:08 UTC (permalink / raw)
To: Hong Xu; +Cc: Matthew Woodcraft, 22661
Hong Xu <hong@topbug.net> writes:
> Subject: [PATCH] python-mode: Fix detection for opening blocks.
>
> * python.el (python-info-dedenter-opening-block-positions): There
> can't be any back-indented lines between an opening block and the
> current line.
Looks good, with a minor adjustment I suggested below. Adding a test
would be nice too.
> ---
> lisp/progmodes/python.el | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
> index 306402d8e3b4..ae9b0890cfc2 100644
> --- a/lisp/progmodes/python.el
> +++ b/lisp/progmodes/python.el
> @@ -4659,7 +4659,8 @@ python-info-dedenter-opening-block-positions
> (let ((dedenter-pos (python-info-dedenter-statement-p)))
> (when dedenter-pos
> (goto-char dedenter-pos)
> - (let* ((pairs '(("elif" "elif" "if")
> + (let* ((cur-line (line-beginning-position))
> + (pairs '(("elif" "elif" "if")
> ("else" "if" "elif" "except" "for" "while")
> ("except" "except" "try")
> ("finally" "else" "except" "try")))
> @@ -4675,7 +4676,22 @@ python-info-dedenter-opening-block-positions
> (let ((indentation (current-indentation)))
> (when (and (not (memq indentation collected-indentations))
I think it's better to maximize the range of save-match-data, i.e., have
it cover as much code as practical between the place that sets the match
data and the place that uses the it, rather than minimizing the range so
that it only covers functions which modify match data (because the
default assumption in Emacs is that functions may modify match data
unless they say otherwise). In this case, I would put it around the
`and` in the line above.
> (or (not collected-indentations)
> - (< indentation (apply #'min collected-indentations))))
> + (< indentation (apply #'min collected-indentations)))
> + ;; There must be no line with indentation
> + ;; smaller than `indentation' (except for
> + ;; blank lines) between the found opening
> + ;; block and the current line, otherwise it
> + ;; is not an opening block.
> + (save-excursion
> + (forward-line)
> + (let ((no-back-indent t))
> + (while (and (< (point) cur-line)
> + (setq no-back-indent
> + (or (> (current-indentation) indentation)
> + (save-match-data
> + (python-info-current-line-empty-p)))))
> + (forward-line))
> + no-back-indent)))
> (setq collected-indentations
> (cons indentation collected-indentations))
> (when (member (match-string-no-properties 0)
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2017-01-22 23:08 ` npostavs
@ 2017-01-23 0:50 ` Hong Xu
2017-01-24 1:24 ` npostavs
0 siblings, 1 reply; 9+ messages in thread
From: Hong Xu @ 2017-01-23 0:50 UTC (permalink / raw)
To: npostavs; +Cc: matthew, 22661
[-- Attachment #1.1.1: Type: text/plain, Size: 623 bytes --]
On 01/22/2017 03:08 PM, npostavs@users.sourceforge.net wrote:
>
> I think it's better to maximize the range of save-match-data, i.e., have
> it cover as much code as practical between the place that sets the match
> data and the place that uses the it, rather than minimizing the range so
> that it only covers functions which modify match data (because the
> default assumption in Emacs is that functions may modify match data
> unless they say otherwise). In this case, I would put it around the
> `and` in the line above.
>
Thanks. I've attached a newer version with tests with all the issues fixed.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: 0001-python-mode-Fix-detection-for-opening-blocks.patch --]
[-- Type: text/x-patch; name="0001-python-mode-Fix-detection-for-opening-blocks.patch", Size: 3938 bytes --]
From 1f2108514c14339ff2db5fea2d1b35c3538b2efd Mon Sep 17 00:00:00 2001
From: Hong Xu <hong@topbug.net>
Date: Mon, 12 Dec 2016 17:55:25 -0800
Subject: [PATCH] python-mode: Fix detection for opening blocks.
* python.el (python-info-dedenter-opening-block-positions): There
can't be any back-indented lines between an opening block and the
current line.
* python-tests.el (python-indent-electric-colon-4): Add an indent
test case where there is one-more indented previous opening block.
---
lisp/progmodes/python.el | 20 ++++++++++++++++++--
test/lisp/progmodes/python-tests.el | 21 +++++++++++++++++++++
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d8262dd0a750..90b5e4e0dc67 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4693,7 +4693,8 @@ python-info-dedenter-opening-block-positions
(let ((dedenter-pos (python-info-dedenter-statement-p)))
(when dedenter-pos
(goto-char dedenter-pos)
- (let* ((pairs '(("elif" "elif" "if")
+ (let* ((cur-line (line-beginning-position))
+ (pairs '(("elif" "elif" "if")
("else" "if" "elif" "except" "for" "while")
("except" "except" "try")
("finally" "else" "except" "try")))
@@ -4709,7 +4710,22 @@ python-info-dedenter-opening-block-positions
(let ((indentation (current-indentation)))
(when (and (not (memq indentation collected-indentations))
(or (not collected-indentations)
- (< indentation (apply #'min collected-indentations))))
+ (< indentation (apply #'min collected-indentations)))
+ ;; There must be no line with indentation
+ ;; smaller than `indentation' (except for
+ ;; blank lines) between the found opening
+ ;; block and the current line, otherwise it
+ ;; is not an opening block.
+ (save-excursion
+ (forward-line)
+ (let ((no-back-indent t))
+ (save-match-data
+ (while (and (< (point) cur-line)
+ (setq no-back-indent
+ (or (> (current-indentation) indentation)
+ (python-info-current-line-empty-p))))
+ (forward-line)))
+ no-back-indent)))
(setq collected-indentations
(cons indentation collected-indentations))
(when (member (match-string-no-properties 0)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 2df1bbf50d81..158c52f080c4 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -1156,6 +1156,27 @@ python-tests-visible-string
(python-tests-look-at "that)")
(should (= (current-indentation) 6))))
+(ert-deftest python-indent-electric-colon-4 ()
+ "Test indentation case where there is one more-indented previous open block."
+ (python-tests-with-temp-buffer
+ "
+def f():
+ if True:
+ a = 5
+
+ if True:
+ a = 10
+
+ b = 3
+
+else
+"
+ (python-tests-look-at "else")
+ (goto-char (line-end-position))
+ (python-tests-self-insert ":")
+ (python-tests-look-at "else" -1)
+ (should (= (current-indentation) 4))))
+
(ert-deftest python-indent-region-1 ()
"Test indentation case from Bug#18843."
(let ((contents "
--
2.1.4
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related [flat|nested] 9+ messages in thread* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2017-01-23 0:50 ` Hong Xu
@ 2017-01-24 1:24 ` npostavs
2017-01-27 1:20 ` npostavs
0 siblings, 1 reply; 9+ messages in thread
From: npostavs @ 2017-01-24 1:24 UTC (permalink / raw)
To: Hong Xu; +Cc: matthew, 22661
tags 22661 patch
quit
Hong Xu <hong@topbug.net> writes:
>
> Thanks. I've attached a newer version with tests with all the issues fixed.
>
Excellent, I'll merge this to master in a few days.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:'
2017-01-24 1:24 ` npostavs
@ 2017-01-27 1:20 ` npostavs
0 siblings, 0 replies; 9+ messages in thread
From: npostavs @ 2017-01-27 1:20 UTC (permalink / raw)
To: Hong Xu; +Cc: matthew, 22661
tags 22661 fixed
close 22661 26.1
quit
npostavs@users.sourceforge.net writes:
> Hong Xu <hong@topbug.net> writes:
>>
>> Thanks. I've attached a newer version with tests with all the issues fixed.
>>
>
> Excellent, I'll merge this to master in a few days.
Pushed to master [1: 7cb7a58].
1: 2017-01-26 20:14:19 -0500 7cb7a582f44db94292709d35f4f5474f891f03b0
python-mode: Fix detection for opening blocks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-27 1:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-14 17:09 bug#22661: 25.0.91; python.el electric-indent misbehaviour with 'else:' and nested 'if:' Matthew Woodcraft
2016-12-13 2:01 ` Hong Xu
2017-01-21 17:34 ` Matthew Woodcraft
2017-01-22 2:08 ` Hong Xu
2017-01-22 22:34 ` Matthew Woodcraft
2017-01-22 23:08 ` npostavs
2017-01-23 0:50 ` Hong Xu
2017-01-24 1:24 ` npostavs
2017-01-27 1:20 ` npostavs
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).