Fourth level blocks indent incorrectly

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

Bug Description

This

for x in y:
    for z in l:
        for r in t:
                pass <--- indents here. Pressing <backspace> dedents eight spaces (i.e. you can go to column 0 in two presess)

It seems to happen with all blocks:

while 1:
    while 1:
        while 1:
                pass

and when you mix them:

try:
    if True:
        while True:
                pass

Changed in python-mode:
assignee: nobody → Andreas Roehler (a-roehler)
milestone: none → 6.0.5
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
Andreas Roehler (a-roehler) wrote : Re: [Bug 939577] [NEW] Fourth level blocks indent incorrectly

Am 23.02.2012 16:20, schrieb Ryan Kaskel:
> Public bug reported:
>
> This
>
> for x in y:
> for z in l:
> for r in t:
> pass<--- indents here. Pressing<backspace> dedents eight spaces (i.e. you can go to column 0 in two presess)

Hi,

AFAIU indent of 16 is a legal value here, as different blocks may have
different indent levels.

Not sure it that's true when nested, CC to Barry for this question.

So if smart-indentation is on, it sets py-indent-offset at 8 at this point.

Setting py-smart-indentation nil should solve that for the moment.

[ ... ]

Andreas

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

I've also encountered this bug. The point is that there is no reason to indent that block to 16, not 12!

Revision history for this message
Andreas Roehler (a-roehler) wrote : Re: [Bug 939577] Re: Fourth level blocks indent incorrectly

Am 23.02.2012 22:15, schrieb Georg Brandl:
> I've also encountered this bug. The point is that there is no reason to
> indent that block to 16, not 12!
>

Okay, will fix smart-indentation so it doesn't take action in nested blocks.

Remains the question if 16 would be a permitted indent - reasonable or not.

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

Of course it's permitted; just like

try:
 if True:
                             while True:
                               pass

would also be permitted. That doesn't mean that you want to write (or see) code like that :)

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

Am 24.02.2012 08:57, schrieb Georg Brandl:
> Of course it's permitted; just like
>
> try:
> if True:
> while True:
> pass
>
> would also be permitted. That doesn't mean that you want to write (or
> see) code like that :)
>

thanks, Georg, that helps.

Think we agree the example given looks like bad coding style.

OTOH Python-mode is not designed to impose a certain style - beside the
indent range between 2-8 `py-guessed-sanity-check' already does.

If we prevent certain styles, we run into a mess at other places with
some probability.
Python-mode got some complexity already...

In the precise case: how could that happen?

Python-mode by themselves would not propose that final indent.

So it was inserted deliberately. Why?

Think before doing any change here, we should learn something about the
workflow.

Andreas

Changed in python-mode:
status: Triaged → Incomplete
assignee: Andreas Roehler (a-roehler) → nobody
Changed in python-mode:
milestone: 6.0.5 → none
importance: Medium → Undecided
Revision history for this message
Georg Brandl (gbrandl) wrote :

I believe the issue is not that python-mode tries to keep consistent for a code file that already has that strange indent.

At least in my case (and from the sound of the OP, also in theirs), the file was perfectly consistently formatted with 4-space indents, and still python-mode insisted to put an 8 space indent at some level.

Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

I'm not sure why anyone would want multiple indentation lengths in a Python file. While it is allowed, I've never seen code written like that.

I think python-mode should default to the style defined in PEP 8 (http://www.python.org/dev/peps/pep-0008/). One of the first lines on that doc says "Use 4 spaces per indentation level."

The user should surely be allowed to change that but IMO it should be really difficult to use a mix of indentation lengths and it definitely shouldn't be the default.

Maybe I'm wrong, but wouldn't changing this make the code less complex because multiple indentation lengths wouldn't be an issue?

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

Am 24.02.2012 17:42, schrieb Georg Brandl:
> I believe the issue is not that python-mode tries to keep consistent for
> a code file that already has that strange indent.
>
> At least in my case (and from the sound of the OP, also in theirs), the
> file was perfectly consistently formatted with 4-space indents, and
> still python-mode insisted to put an 8 space indent at some level.
>

if you can send a recipe how to reproduce that, I'll fix the bug.
When such an 8-space indent did occur?

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

Am 24.02.2012 18:35, schrieb Ryan Kaskel:
> I'm not sure why anyone would want multiple indentation lengths in a
> Python file. While it is allowed, I've never seen code written like
> that.
>
> I think python-mode should default to the style defined in PEP 8
> (http://www.python.org/dev/peps/pep-0008/).

it does

One of the first lines on
> that doc says "Use 4 spaces per indentation level."
>

just the Python-mode default

> The user should surely be allowed to change that but IMO it should be
> really difficult to use a mix of indentation lengths

Why? IMO it should be up to the user. We assist the Python syntax as
smoothly and complete as possible.

  and it definitely
> shouldn't be the default.

it is

>
> Maybe I'm wrong, but wouldn't changing this

Relying on a hard-coded four-space indent would be less complex indeed.
But it would break Python rules. No need so far as indentation works BTW.

Rather tell: how did you produce that 8-space indent?
If it came out from common proceeding, we have a bug.

Need a description of these steps still,

Andreas

  make the code less complex
> because multiple indentation lengths wouldn't be an issue?
>

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

closing this considered as a feature request,
revno: 1355 delivers a way how to deal with non-default coding style

####

  Go to the example given by lp:939577, line 4,
  beginning of word "pass" at column 16.

  M-x py-smart-indentation-on
  makes TAB circle between 0, 8 und 16

  M-x py-smart-indentation-off
  makes TAB circle between 0, 4, 8 und 12

  Resp. using `py-toggle-smart-indentation'

  Alternatively:

  `py-indent-line' called with an optional
  \\[universal-argument] followed by a numeric argument
  neither 1 nor 4 will switch off `py-smart-indentation'
  for this execution and introduce default-indent.

#######

should a recipe be available, Python-mode causes such a 8-space indent, please re-open.

Changed in python-mode:
status: Incomplete → Fix Committed
Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

Hi Andreas,

I'm reopening this issue because I'm not sure if I communicated the original bug report correctly.

If I type in emacs with python-mode on:

if True:
    x = 1
    if x:
        y = 2
        if y:
                z = 3
                ^
                 |------------ python-mode indents 8 spaces instead of four.

Each type I pressed RET after a colon (":"), the cursor indented four spaces.

The third time I pressed RET after an if statement (although this happens after all blocks), the cursor moved 8 spaces.

if True: # pressing RET here will move the cursor 4 spaces from column 0
    if True: # pressing RET here will move the cursor 8 spaces from column 0
        if True: # pressing RET here will move the cursor *16* spaces from column 0; this doesn't make sense
                if True: # pressing RET here will move the cursor 20 spaces from column 0
                    pass

Notice the indentation pattern is 4 - 8 - 16 - 20; it should be 4 - 8 - 12 -16.

I completely agree that people should be able to code however they like, but there is a bug in the current behavior.

Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

Please see the previous comment. I'm not sure I communicated the bug properly.

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

Am 25.02.2012 10:22, schrieb Ryan Kaskel:
> Hi Andreas,
>
> I'm reopening this issue because I'm not sure if I communicated the
> original bug report correctly.
>

ahh, no, it's my bad. After re-reading I understand your recipe.
Sorry being that stupid :)

Also seeing the bug now - on it.

Cheers,

Andreas

> If I type in emacs with python-mode on:
>
> if True:
> x = 1
> if x:
> y = 2
> if y:
> z = 3
> ^
> |------------ python-mode indents 8 spaces instead of four.
>
> Each type I pressed RET after a colon (":"), the cursor indented four
> spaces.
>
> The third time I pressed RET after an if statement (although this
> happens after all blocks), the cursor moved 8 spaces.
>
> if True: # pressing RET here will move the cursor 4 spaces from column 0
> if True: # pressing RET here will move the cursor 8 spaces from column 0
> if True: # pressing RET here will move the cursor *16* spaces from column 0; this doesn't make sense
> if True: # pressing RET here will move the cursor 20 spaces from column 0
> pass
>
> Notice the indentation pattern is 4 - 8 - 16 - 20; it should be 4 - 8 -
> 12 -16.
>
> I completely agree that people should be able to code however they like,
> but there is a bug in the current behavior.
>

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

Hi Ryan, hi Georg,

please try again - and re-open if needed.

thanks,

Andreas

Changed in python-mode:
status: In Progress → Fix Committed
Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

I've pulled in revision 853 but I still see them same behavior. I've changed the status to "confirmed."

def test():
    if x:
        try:
                pass <--- indents here

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

Am 28.02.2012 10:43, schrieb Ryan Kaskel:
> I've pulled in revision 853 but I still see them same behavior. I've
> changed the status to "confirmed."
>
> def test():
> if x:
> try:
> pass<--- indents here
>
> ** Changed in: python-mode
> Status: Fix Committed => Confirmed
>

hmm, can't reproduce.

Attached a png how it looks here.
Can you start from emacs -Q and send M-x report-emacs-bug?

Thanks,

Andreas

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

okay, detected a bug meanwhile.

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

introduced restrictions:

Indent might be guessed savely only from beginning of a block.
Also current line must be ignored.

HTH, please re-open if needed.

Changed in python-mode:
status: In Progress → Fix Committed
Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

Please see next comment.

Changed in python-mode:
status: Fix Committed → Confirmed
Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

Hi Andreas,

I really appreciate all your hard work on this bug.

I took a look at your screenshot but unfortunatley I'm not seeing the same thing.

I started up emacs23 with -q.

Please see my screenshots. I've attached a picture of the value py-guess-indent-offset as I was typing.

Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

It seems I can only attach one image.

Here is the final step.

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

hmm, really strange.

8 would be correct at next line, not behind pass.

So what to do?

in

py-bug-numbered-tests.el there is a test for this.

Can you run M-x fourth-level-blocks-indent-incorrectly-lp-939577-test?
Putting py-guess-... under edebug might reveal something too.

Finally we should get it, the matter has no miracles so far.

Thanks,

Andreas

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

BTW a comment to the meaning of flags - as I understand that.

"confirm" means the asignee sees it, not just the reporter :)

cheers

Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

Okay, I've run the test and it fails for me (with both "emacs 23 -q" and "emacs-snapshot" with my normal config on two different computers).

I've attached a screenshot which is hopefully helpful.

I will try to debug the elisp further as you suggested.

I wonder if anyone else can duplicate this issue.

Also, I'm not sure if you wrote that comment in the test ("indents here. Pressing <backspace> dedents eight spaces (i.e. you can go to column 0 in two presess)") before or after you realized the bug I was trying to report (which deals with indentation rather than dedenting).

I've drawn what I meant on the screenshot which I know you don't like but hopefully it helps.

Best,
Ryan

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

Am 29.02.2012 21:10, schrieb Ryan Kaskel:
> Okay, I've run the test and it fails for me (with both "emacs 23 -q" and
> "emacs-snapshot" with my normal config on two different computers).
>
> I've attached a screenshot which is hopefully helpful.
>
> I will try to debug the elisp further as you suggested.
>
> I wonder if anyone else can duplicate this issue.
>
> Also, I'm not sure if you wrote that comment in the test ("indents here.
> Pressing<backspace> dedents eight spaces (i.e. you can go to column 0
> in two presess)") before or after you realized the bug I was trying to
> report (which deals with indentation rather than dedenting).
>
> I've drawn what I meant on the screenshot which I know you don't like

oh, very nice pictures :)

here are some tunes for it..

http://www.youtube.com/watch?v=ppLKAzyYLQU&feature=mr_meh&list=AVGxdCwVVULXfgQBNhVFIiTSqxhrOsiceM&lf=list_related&playnext=0

well, understood your report this time.
unfortunately don't get it for now.

let's see if heavens can help :)

> but hopefully it helps.
>
> Best,
> Ryan
>
> ** Attachment added: "emacs-lp-939577.png"
> https://bugs.launchpad.net/python-mode/+bug/939577/+attachment/2795040/+files/emacs-lp-939577.png
>

Changed in python-mode:
milestone: 6.0.5 → 6.0.6
Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

I'm an idiot. I was using an older byte-compiled version of python-mode.el. DOH!

The issue is fixed -- closing this bug.

Changed in python-mode:
status: New → Fix Released
Revision history for this message
Ryan Kaskel (ryankaskel) wrote :

I'm marking this as "new" again for a brief fix.

Could line 2866 check for py-verbose-p?

(http://bazaar.launchpad.net/~python-mode-devs/python-mode/python-mode/view/head:/python-mode.el#L2866)

(message "py-indent-line-indent: %s" py-indent-line-indent) -->
(when (and (interactive-p) py-verbose-p) (message "py-indent-line-indent: %s" py-indent-line-indent))

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

Am 09.03.2012 10:05, schrieb Ryan Kaskel:
> I'm marking this as "new" again for a brief fix.
>
> Could line 2866 check for py-verbose-p?
>
> (http://bazaar.launchpad.net/~python-mode-devs/python-mode/python-
> mode/view/head:/python-mode.el#L2866)
>
> (message "py-indent-line-indent: %s" py-indent-line-indent) -->
> (when (and (interactive-p) py-verbose-p) (message "py-indent-line-indent: %s" py-indent-line-indent))
>
>
> ** Changed in: python-mode
> Status: Fix Released => New
>

thanks pointing at. This "message" deserves to be dropped completely, as it's an intern function.

Changed in python-mode:
status: New → 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.