bug#18796: 24.4; python indent-for-tab-command fails to recognize if-elif-else

bug#18796: 24.4; python indent-for-tab-command fails to recognize if-elif-else

[Carlos Pita]

While in python-mode select the following code and press tab:

def f():
    if True:
        pass
    else:
        pass

You will get:

def f():
    if True:
        pass
else:
    pass


Which is clearly wrong. I understand that auto-indenting python could be
a difficult task. Maybe there's a rationale for the above behavior I
can't grasp. But the result is somewhat surprising for the unaware.

-------------------------

In GNU Emacs 24.4.1 (i686-pc-linux-gnu, GTK+ Version 3.12.2)
 of 2014-10-22 on carlos
Windowing system distributor `The X.Org Foundation', version 11.0.11502000
Configured using:
 `configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft
 'CFLAGS=-march=i686 -mtune=generic -O2 -pipe -fstack-protector-strong
 --param=ssp-buffer-size=4' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

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

Major mode: Python

Minor modes in effect:
  show-paren-mode: t
  shell-dirtrack-mode: t
  eldoc-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  ido-ubiquitous-mode: t
  ido-everywhere: t
  winner-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<up> <up> C-e <return> <backspace> e l s e : <return> 
0 <return> <return> <up> <up> <up> <up> <up> C-a <down> 
<up> C-SPC <down> <down> <down> <down> <tab> <up> <up> 
C-SPC <down> <down> <down> <down> C-w <down> C-x C-s 
<down> <down> C-h f i n d e n t - f o r <right> <return> 
<down> <down> <down> <down> <up> <up> <up> <down> <down> 
q <down> <down> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <down> <down> <down> <down> SPC SPC SPC <down> 
SPC SPC <up> <up> <up> <down> <down> <delete> <up> 
<up> <down> <down> <left> <delete> <delete> <down> 
<tab> <up> <up> <up> C-a C-SPC <down> <down> <down> 
<down> <tab> C-/ <up> <up> <up> <up> <up> <up> <return> 
<return> <up> <up> i f SPC T r u e : C-a C-d C-k i 
f SPC T r u e : <tab> <tab> <tab> <tab> <tab> <tab> 
<down> <down> <down> <down> C-a C-SPC <down> <down> 
<down> <down> <tab> <tab> C-/ C-/ <up> <up> <up> <up> 
<up> <up> <up> <tab> p a s s <return> e l s e <return> 
SPC SPC SPC SPC p a s s <up> <up> <up> <tab> C-/ C-a 
C-SPC <down> <down> <down> <down> <tab> <up> <up> C-e 
: <down> <tab> <up> <up> <up> C-a C-SPC <down> <down> 
<down> <down> <tab> <up> <up> <up> <up> <up> <return> 
d e <backspace> <backspace> <backspace> <backspace> 
d e f SPC x x x x ( ) : <return> <up> M-d d <down> 
<up> <backspace> f <down> <backspace> <backspace> <down> 
C-a C-SPC <down> <down> <down> <down> <tab> <up> <up> 
<tab> <down> <tab> <up> <up> <up> <up> C-a C-SPC <down> 
<down> <down> <down> <down> <tab> C-/ <up> <up> <up> 
<up> <up> C-SPC <down> <down> <down> <down> <down> 
M-w <down> M-x r e p o r t - e m <tab> <return>

Recent messages:
Mark activated
Closes elif argv[1] == "send":
Undo! [3 times]
Mark set
Closes if True:
Mark activated [2 times]
Closes if True:
Mark set
Undo!
Mark activated

Load-path shadows:
~/.emacs.d/lisp/rmail hides /usr/share/emacs/24.4/lisp/mail/rmail

Features:
(nnir qp gnus-dup mule-util debug misearch multi-isearch jka-compr
oauth2 warnings plstore cus-edit cus-start cus-load eieio-opt speedbar
sb-image ezimage dframe python json tramp-cache tramp tramp-compat
tramp-loaddefs trampver mailalias smtpmail pp shadow emacsbug sendmail
rect url-queue timezone shr-color color url-http url-gw url-auth eww
mm-url shr sort smiley gnus-cite mail-extr gnus-async gnus-bcklg gnus-ml
disp-table gnus-topic nndraft nnmh nnfolder utf-7 gnutls network-stream
starttls nnimap parse-time tls utf7 netrc gnus-agent gnus-srvr
gnus-score score-mode nnvirtual gnus-msg nntp gnus-cache
google-contacts-gnus gnus-art mm-uu mml2015 mm-view mml-smime smime dig
gnus-sum nnoo gnus-group gnus-undo nnmail mail-source
google-contacts-message google-contacts xml url-cache url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
mailcap url-util url-parse auth-source eieio byte-opt bytecomp
byte-compile cconv eieio-core password-cache url-vars google-oauth
gnus-start gnus-spec gnus-int gnus-range message idna rfc822 mml mml-sec
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums mailabbrev gmm-utils mailheader gnus-win gnus gnus-ems
nnheader gnus-util mail-utils mm-util mail-prsvr wid-edit vc-git
epa-file epa derived epg server paredit edmacro kmacro paren cl-macs
ob-python ob-R org org-macro org-footnote org-pcomplete org-list
org-faces org-entities noutline outline easy-mmode org-version
ob-emacs-lisp ob ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys
ob-comint ob-core ob-eval org-compat org-macs org-loaddefs find-func
cal-menu calendar cal-loaddefs ess-toolbar ess-mouse mouseme thingatpt
browse-url ess-menu ess-swv ess-noweb ess-noweb-font-lock-mode
ess-bugs-l essd-els ess-sas-d ess-sas-l ess-sas-a shell pcomplete
ess-sta-d ess-sta-l cc-vars cc-defs make-regexp ess-sp6-d ess-sp3-d
ess-julia ess-r-d compile ess-tracebug format-spec ess-roxy hideshow
ess-help ess-developer ess-r-args eldoc ess-s-l ess ess-inf comint
ansi-color ess-mode ess-noweb-mode ess-utils time-date ess-custom
executable ess-compat ess-site yasnippet help-mode cl gv ido-ubiquitous
cl-loaddefs cl-lib advice help-fns imenu-anywhere imenu ido windmove
winner ring info easymenu package epg-config wombat-theme 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 8 557759 103077)
 (symbols 24 46775 0)
 (miscs 20 5488 3171)
 (strings 16 166668 3684)
 (string-bytes 1 4655761)
 (vectors 8 48281)
 (vector-slots 4 1634972 78634)
 (floats 8 1011 1185)
 (intervals 28 21468 2483)
 (buffers 512 63)
 (heap 1024 81411 3306))

bug#18796:

[Carlos Pita]

Besides, pressing tab while the point is over "else:" and no region is
active correctly reports "Closes if..." in the echo area and keeps the
indentation level right. So somehow the indentation logic is there,
just failing for the region.

bug#18796:

[Carlos Pita]

I've doing some debugging. I found out why the bug happens, but I
still have to think of a way to fix it:

Say you are in a dedenter line like "else:". In python-indent-region
delete-horizontal-space gets executed, so the dedenter "else:" moves
to the first column and the current indentation becomes 0. Now
python-info-dedenter-opening-block-position initializes indentation as
the value returned by (current-indentation), that is 0. Then the
opening block position is calculated and the following code gets
executed:

        (if (<= (current-indentation) indentation)
            (setq position (car positions))
          (setq positions (cdr positions)))))

In this code (current-indentation) returns the indentation of the
opening block, which is fine. But it gets compared to indentation,
which is always 0. So the else branch is taken every time and
positions is set to nil (cdr positions).

Obviously the problem is that at the point
python-info-dedenter-opening-block-position is called the original
dedenter indentation was lost.

Maybe the original indentation should be saved as a dynamic variable
before deleting the horizontal space. Or the horizontal space
shouldn't be deleted at all and indentation should happen as a
displacement from the original position instead of a displacement from
the left margin.

bug#18796:

[Carlos Pita]

This fix seems to work:

In python-indent-region replace:

                (delete-horizontal-space)
                (indent-to (python-indent-calculate-indentation)))))

for

            (let ((indentation (python-indent-calculate-indentation)))
                  (delete-horizontal-space)
                  (indent-to indentation)))))

So the new indentation is calculated on the basis of the original one
(that is, before removing horizontal space).

On Wed, Oct 22, 2014 at 3:11 PM, Carlos Pita <carlosjosepita@gmail.com> wrote:
> I've doing some debugging. I found out why the bug happens, but I
> still have to think of a way to fix it:
>
> Say you are in a dedenter line like "else:". In python-indent-region
> delete-horizontal-space gets executed, so the dedenter "else:" moves
> to the first column and the current indentation becomes 0. Now
> python-info-dedenter-opening-block-position initializes indentation as
> the value returned by (current-indentation), that is 0. Then the
> opening block position is calculated and the following code gets
> executed:
>
>         (if (<= (current-indentation) indentation)
>             (setq position (car positions))
>           (setq positions (cdr positions)))))
>
> In this code (current-indentation) returns the indentation of the
> opening block, which is fine. But it gets compared to indentation,
> which is always 0. So the else branch is taken every time and
> positions is set to nil (cdr positions).
>
> Obviously the problem is that at the point
> python-info-dedenter-opening-block-position is called the original
> dedenter indentation was lost.
>
> Maybe the original indentation should be saved as a dynamic variable
> before deleting the horizontal space. Or the horizontal space
> shouldn't be deleted at all and indentation should happen as a
> displacement from the original position instead of a displacement from
> the left margin.