More indentation bugs

Bug #883905 reported by Georg Brandl
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
python-mode.el
Fix Released
Medium
Andreas Roehler

Bug Description

Given this file:

def itn(n, digits=8, format=DEFAULT_FORMAT):
    """Convert a python number to a number field.
    """
    # number.
    if 0 <= n < 8 ** (digits - 1):
        s = bytes("%0*o" % (digits - 1, n), "ascii") + NUL
    elif format == GNU_FORMAT and -256 ** (digits - 1) <= n < 256 ** (digits - 1):
        if n >= 0:
            s = bytearray([0o200])
        else:
            s = bytearray([0o377])
            n = 256 ** digits + n

        for i in range(digits - 1):
            s.insert(1, n & 0o377)
            n >>= 8
    else:
        raise ValueError("overflow in number field")

    return s

open it, put point on any of the lines below the "if n >= 0" and press TAB. Line will be outdented to column 0 or 4.

Changed in python-mode:
assignee: nobody → Andreas Roehler (a-roehler)
milestone: none → 6.0.4
importance: Undecided → Medium
Revision history for this message
Andreas Roehler (a-roehler) wrote :

Hi,

can't reproduce.

Maybe fix of lp:883902 made it gone?

Revision history for this message
Georg Brandl (gbrandl) wrote :

No, I had already fixed that before. Maybe it's because of my customization? The py- variables are set like this:

 '(py-closing-list-dedents-bos t)
 '(py-encoding-string "# -*- coding: utf-8 -*-")
 '(py-honor-comment-indentation t)
 '(py-imenu-show-method-args-p t)
 '(py-load-python-mode-pymacs-p nil)
 '(py-shebang-startstring "#!/usr/bin/env")
 '(py-shell-name "ipython")
 '(py-shell-switch-buffers-on-execute nil)

It kind of seems to me like python-mode does find the correct indentation, but the logic cycling through the different possible indentation levels is flawed; maybe some cycle state needs to be reset?

Revision history for this message
Andreas Roehler (a-roehler) wrote : Re: [Bug 883905] Re: More indentation bugs

Am 31.10.2011 07:21, schrieb Georg Brandl:
> No, I had already fixed that before. Maybe it's because of my
> customization? The py- variables are set like this:
>
> '(py-closing-list-dedents-bos t)
> '(py-encoding-string "# -*- coding: utf-8 -*-")
> '(py-honor-comment-indentation t)
> '(py-imenu-show-method-args-p t)
> '(py-load-python-mode-pymacs-p nil)
> '(py-shebang-startstring "#!/usr/bin/env")
> '(py-shell-name "ipython")
> '(py-shell-switch-buffers-on-execute nil)
>

there should be no effect so far

> It kind of seems to me like python-mode does find the correct
> indentation, but the logic cycling through the different possible
> indentation levels is flawed; maybe some cycle state needs to be reset?
>

Don't see that.

Cycling has a single dependency basically, the max reasonable indent.

Could you indicate a line/buffer position wherefrom to start a test?

Also output from M-x report-emacs-bugs might be useful.

Thanks,

Andreas

Changed in python-mode:
status: New → Incomplete
Revision history for this message
Georg Brandl (gbrandl) wrote :
Download full text (10.9 KiB)

The line
            s = bytearray([0o200])
would be a good point to start.

Pasting relevant output from "report-emacs-bug":

In GNU Emacs 23.3.3 (x86_64-pc-linux-gnu, GTK+ Version 2.24.6)
 of 2011-09-28 on ndl
Windowing system distributor `The X.Org Foundation', version 11.0.11004000
configured using `configure '--prefix=/usr' '--build=x86_64-pc-linux-gnu' '--host=x86_64-pc-linux-gnu' '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--datadir=/usr/share' '--sysconfdir=/etc' '--localstatedir=/var/lib' '--libdir=/usr/lib64' '--program-suffix=-emacs-23' '--infodir=/usr/share/info/emacs-23' '--with-crt-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64' '--with-gameuser=games' '--without-hesiod' '--without-kerberos' '--without-kerberos5' '--with-gpm' '--with-dbus' '--with-sound' '--with-x' '--without-ns' '--without-gconf' '--with-toolkit-scroll-bars' '--with-gif' '--with-jpeg' '--with-png' '--with-rsvg' '--with-tiff' '--with-xpm' '--with-xft' '--with-libotf' '--with-m17n-flt' '--with-x-toolkit=gtk' 'build_alias=x86_64-pc-linux-gnu' 'host_alias=x86_64-pc-linux-gnu' 'CFLAGS=-O2 -pipe -march=core2 -fno-strict-aliasing' 'LDFLAGS=-Wl,-O1 -Wl,--as-needed' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: C
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Python

Minor modes in effect:
  reveal-mode: t
  shell-dirtrack-mode: t
  yas/global-mode: t
  yas/minor-mode: t
  semantic-tag-folding-mode: t
  semantic-decoration-mode: t
  semantic-mru-bookmark-mode: t
  semantic-idle-summary-mode: t
  semantic-idle-scheduler-mode: t
  senator-minor-mode: t
  test-case-mode: t
  test-case-global-mode: t
  highlight-symbol-mode: t
  hi-lock-mode: t
  nxhtml-menu-mode: t
  nxhtml-tag-do-also: t
  popcmp-group-alternatives: t
  popcmp-short-help-beside-alts: t
  mlinks-active-links: t
  rngalt-minimal-validation-header: t
  rngalt-display-validation-header: t
  global-auto-complete-mode: t
  auto-complete-mode: t
  eproject-mode: t
  winpoint-mode: t
  flymake-mode: t
  autopair-mode: t
  show-paren-mode: t
  recentf-mode: t
  minibuffer-electric-default-mode: t
  gud-tooltip-mode: t
  global-hl-line-mode: t
  display-time-mode: t
  desktop-save-mode: t
  cua-mode: t
  auto-insert-mode: t
  semantic-highlight-func-mode: t
  semantic-stickyfunc-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/gbr/.emacs.d/nxhtml/util/web-vcs hides /home/gbr/.emacs.d/nxhtml/web-vcs
/home/gbr/.emacs.d/nxhtml/util/vline hides ~/.emacs.d/vline
~/.emacs.d/emacs-goodies-el/browse-kill-ring hides ~/.emacs.d/browse-kill-ring
~/.emacs.d/emacs-goodies-el/browse-kill-ring hides /usr/share/emacs/site-lisp/browse-kill-ring/browse-kill-ring
~/.emacs.d...

Changed in python-mode:
status: Incomplete → New
Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 08:35, schrieb Georg Brandl:
> The line
> s = bytearray([0o200])
> would be a good point to start.

If you go there, what does

M-x py-compute-indentation

message?

Revision history for this message
Georg Brandl (gbrandl) wrote :

Aha! It has the correct value (and for all other lines too).

And indeed, when I bind TAB to "py-indent-line" (instead of "indent-for-tab-command"), I get the correct behavior, but it doesn't cycle anymore.

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 09:30, schrieb Georg Brandl:
> Aha! It has the correct value (and for all other lines too).
>
> And indeed, when I bind TAB to "py-indent-line" (instead of "indent-for-
> tab-command"), I get the correct behavior, but it doesn't cycle anymore.
>

unloading python.el might solve it

I do the following before loading my several python-modes:

   (when (featurep 'ipython) (unload-feature 'ipython t))
   (when (featurep 'python-mode) (unload-feature 'python-mode t))
   (when (featurep 'python) (unload-feature 'python t))

maybe we should deliver this?

Changed in python-mode:
status: New → In Progress
Revision history for this message
Georg Brandl (gbrandl) wrote :

With python.el unloaded, I get

cond: Symbol's function definition is void: python-continuation-line-p

And indeed this function seems to be defined in python.el only.

Revision history for this message
Georg Brandl (gbrandl) wrote :

Looking a bit more closely, there seem to be *tons* of functions and vars used in current python-mode.el that come from python.el.

May I suggest that we keep to using the prefix "py-" even when copying stuff from python.el, so that this confusion can't happen again?

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 14:20, schrieb Georg Brandl:
> With python.el unloaded, I get
>
> cond: Symbol's function definition is void: python-continuation-line-p
>
> And indeed this function seems to be defined in python.el only.
>

OK, thats another bug.

please replace with

py-continuation-line-p

will fix it here.

thanks

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 14:37, schrieb Georg Brandl:
> Looking a bit more closely, there seem to be *tons* of functions and
> vars used in current python-mode.el that come from python.el.

Ah, yes.

>
> May I suggest that we keep to using the prefix "py-" even when copying
> stuff from python.el, so that this confusion can't happen again?
>

In this case it was simple a typo, as the related function exists.

With the other stuff there are pros and cons.
Basically you are right.

OTOH if we do things exactly the way it's done at python.el, we can keep
tracking fixes in the future if we don't make more changes.

Some more aspects: code came in only recently. Still playing with the
function itself. Changing prefixes also is error-prone etc.

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 14:37, schrieb Georg Brandl:

When this typo is fixed, what's with the TAB?

Revision history for this message
Georg Brandl (gbrandl) wrote :

I still get the same behavior: py-indent-line works, indent-for-tab-command does strange things.

BTW, we have py-indent-line and python-indent-line functions defined in python-mode.el --- I'm now totally confused which indentation function is used in which case.

Revision history for this message
Georg Brandl (gbrandl) wrote :

OK, I think got it now.

When I call "M-x py-indent-line", I jump into the True branch of the "(if (interactive-p)" (line 3421 in current trunk). Then, it works fine.

When I press TAB, I jump into the False branch, which has several different behaviors: when point is on the first non-space or before, the line isn't indented properly but outdented to column 0 or the nearest 4-space multiple.

Is this really intended behavior? If yes, what is the point? I would like TAB to indent the line properly, no matter where the point is on the line, and this is also how TAB behaved previously in python-mode.

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 15:56, schrieb Georg Brandl:
> OK, I think got it now.
>
> When I call "M-x py-indent-line", I jump into the True branch of the
> "(if (interactive-p)" (line 3421 in current trunk). Then, it works fine.
>
> When I press TAB, I jump into the False branch,

what does this mean? is TAB still bound by python.el ?

What says M-x describe-key TAB?

Revision history for this message
Georg Brandl (gbrandl) wrote :

No, it's not bound by python.el. TAB invokes 'indent-for-tab-command, which invokes the function given by 'indent-line-function, which is set by 'python-mode to 'py-indent-line. But apparently it is called in such a way that (interactive-p) is nil.

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 18:31, schrieb Georg Brandl:
> No, it's not bound by python.el. TAB invokes 'indent-for-tab-command,
> which invokes the function given by 'indent-line-function, which is set
> by 'python-mode to 'py-indent-line. But apparently it is called in such
> a way that (interactive-p) is nil.
>

that's correct.

it's called from indent-for-tab-command, which means from another
function, not interactive here.

In that case, TAB cycles possible indents.
So you should get cycling when repeating TAB

So this bug is done?

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 15:30, schrieb Georg Brandl:
> I still get the same behavior: py-indent-line works, indent-for-tab-
> command does strange things.
>
> BTW, we have py-indent-line and python-indent-line functions defined in
> python-mode.el

no harm from here with resp. to function.
Nonetheless, will make some cleanup here.
Thanks pointing at.

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 15:56, schrieb Georg Brandl:
  I would
> like TAB to indent the line properly, no matter where the point is on
> the line,

TAB was bound to cycling previously and now again.

If it's just to jump to outmost reasonable indent, call

py-indent-line directly.

Or bind it to TAB to avoid cycling.

use BACKSPACE from outmost.

and this is also how TAB behaved previously in python-mode.
>

Revision history for this message
Georg Brandl (gbrandl) wrote :

> TAB was bound to cycling previously and now again.

But what it does now is *not* cycling as it did before. When I enter a line that is correctly indented, pressing TAB once should do nothing.

> If it's just to jump to outmost reasonable indent, call
> py-indent-line directly.

BTW, "outmost indent" is ambiguous to me. I assume you mean the largest reasonable indentation?

> Or bind it to TAB to avoid cycling.

But I like cycling, just not the way it's done right now. Even if this behavior is kept, I question its usefulness as a default behavior.

Revision history for this message
Andreas Roehler (a-roehler) wrote :

Am 01.11.2011 20:03, schrieb Georg Brandl:
>> TAB was bound to cycling previously and now again.
>
> But what it does now is *not* cycling as it did before. When I enter a
> line that is correctly indented, pressing TAB once should do nothing.

OK, that's a difference.

>
>> If it's just to jump to outmost reasonable indent, call
>> py-indent-line directly.
>
> BTW, "outmost indent" is ambiguous to me. I assume you mean the largest
> reasonable indentation?

yes

>
>> Or bind it to TAB to avoid cycling.
>
> But I like cycling, just not the way it's done right now. Even if this
> behavior is kept, I question its usefulness as a default behavior.
>

Will re-introduce that do-nothing-first-when-on-largest-indent
and close the bug when done.

Revision history for this message
Georg Brandl (gbrandl) wrote :

Thanks!

Changed in python-mode:
status: In Progress → Fix Committed
Changed in python-mode:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.