microbit-foundation / python-editor-v3

Micro:bit Educational Foundation Python Editor V3
https://python.microbit.org
MIT License
57 stars 36 forks source link

The `else` in `try/except/else/finally` automatically removes indentation #798

Open microbit-carlos opened 2 years ago

microbit-carlos commented 2 years ago

Bug Description

The else statement in a try/except/else/finally is automatically nested incorrectly.

How To Reproduce

Steps to reproduce the behaviour:

In the editor, copy and paste this:

def foo():
    try:
        print("try")
    except:
        print("exception")
    else:
        print("else")
    finally:
        print("finally")

Now remove the else: print("else") and manually try to type it.

After typing else when pressing the : key, the indentation goes to the left one level more than it should. Works fine for finally:.

Expected behavior

To be nested to the same level as try, except, and finally.

Screenshots

Video showing this issue:

https://user-images.githubusercontent.com/29712657/177339962-395ceef3-6f73-4d14-9a59-edc6e17f03c8.mp4

Environment

Desktop (please complete the following information):

Additional context

N/A

microbit-matt-hillsdon commented 2 years ago

I learned something about Python today! We should raise a CM bug for this.

microbit-matt-hillsdon commented 2 years ago

Just in post-live to raise an upstream issue.

microbit-matt-hillsdon commented 2 years ago

Raised https://github.com/codemirror/dev/issues/976. Removing milestone.

microbit-carlos commented 2 years ago

Thanks Matt!

Quick thing, the example linked in the issue wraps the try/except/else in a for loop, which can also have an else statement, so technically it could mistaken as being correctly aligned with the loop. Wrapping it in a function (or anything else that doesn't have an else) should make that a bit more clear.

microbit-matt-hillsdon commented 2 years ago

... so technically it could mistaken as being correctly aligned with the loop. Wrapping it in a function (or anything else that doesn't have an else) should make that a bit more clear.

I suppose it could, though surely that's not what you'd expect to happen. It's been fixed upstream (untried by me, but patch looks totally as expected) and I don't think anyone was confused by this. I just went looking for an example that made some sort of sense as it's an unusual language feature!