WIP: Revise Python indent behaviour

microbit-matt-hillsdon commented

2021-07-27 15:49:36 +02:00

(Migrated from github.com)

I've been testing the Python authoring experience and comparing to VS Code. I think the VS Code indentation generally feels more usual for Python. There are a few cases where CM feels like it is getting in the way and fighting a user's indentation changes and a few where it's not helpful in what feel to me to be clear-cut scenarios.

I've attempted to align the behaviour as I explored this. Details and exceptions noted below. Just a draft PR for now for discussion. Happy to move discussion wherever you'd prefer.

I think it captures the behaviours that I'm interested in. I'm not so confident in the implementation yet. I'll test it some more over the coming days but would love some feedback. Hopefully the tests provide some cases to try out that illustrate differences. I appreciate that indentation preferences can be subjective but I'm interested in exploring whether these changes make sense for lang-python.

Demos:

I've focussed on the automatic indent when typing code and I'm hazy on other uses of indent within CodeMirror. Will my approach here cause issues with other indent-related features?

CC @mattpauldavies based on recent interest in Python indentation.

Details of the change:

This aims to follow VS Code's behaviour and biases towards not changing
the indent as the user may be in the middle of rearranging code.

As an exception, we implement dedent behaviour for return and raise.
VS Code doesn't do this as they don't have a way to tell if you're in
the middle of the expression after the return or raise.

Significant changes:

Adds dedent behaviour for constructs that must end a body.
Adds delimited indents for argument lists, parameter lists and
parathesized expressions. The latter is necessary for tuple indents to
work properly as a tuple isn't parsed as such when first being written.
Doesn't fight manual dedents. Previously trying to dedent then leave a
blank line or two before the next statement would result in a reindent.
Removes dedent for else and similar to match VS Code. VS Code has
attempted to add these in the past but has had to revert because of
users finding them more annoying than helpful. The implementation here
was double-dedenting if the user dedented manually and wrote a valid
if/else statement.

I've been testing the Python authoring experience and comparing to VS Code. I think the VS Code indentation generally feels more usual for Python. There are a few cases where CM feels like it is getting in the way and fighting a user's indentation changes and a few where it's not helpful in what feel to me to be clear-cut scenarios. I've attempted to align the behaviour as I explored this. Details and exceptions noted below. Just a draft PR for now for discussion. Happy to move discussion wherever you'd prefer. I think it captures the behaviours that I'm interested in. I'm not so confident in the implementation yet. I'll test it some more over the coming days but would love some feedback. Hopefully the tests provide some cases to try out that illustrate differences. I appreciate that indentation preferences can be subjective but I'm interested in exploring whether these changes make sense for lang-python. Demos: - [Revised indent behaviour sandbox](https://codesandbox.io/s/revised-indent-behaviour-61jb8?file=/src/python.ts) - [Current indent behaviour for easy comparison](https://codesandbox.io/s/current-indent-behaviour-sj04j?file=/src/index.js) I've focussed on the automatic indent when typing code and I'm hazy on other uses of indent within CodeMirror. Will my approach here cause issues with other indent-related features? CC @mattpauldavies based on recent interest in Python indentation. ---- Details of the change: This aims to follow VS Code's behaviour and biases towards not changing the indent as the user may be in the middle of rearranging code. As an exception, we implement dedent behaviour for return and raise. VS Code doesn't do this as they don't have a way to tell if you're in the middle of the expression after the return or raise. Significant changes: - Adds dedent behaviour for constructs that must end a body. - Adds delimited indents for argument lists, parameter lists and parathesized expressions. The latter is necessary for tuple indents to work properly as a tuple isn't parsed as such when first being written. - Doesn't fight manual dedents. Previously trying to dedent then leave a blank line or two before the next statement would result in a reindent. - Removes dedent for else and similar to match VS Code. VS Code has attempted to add these in the past but has had to revert because of users finding them more annoying than helpful. The implementation here was double-dedenting if the user dedented manually and wrote a valid if/else statement.

👍 1

marijnh commented

2021-07-27 17:59:39 +02:00

(Migrated from github.com)

I'll spend more time looking into this later, but at a glance, I don't think the use of currentIndent is solid—try pressing enter after something like...

def foo():
  foo(bar,
      baz)

That will align the cursor with baz. Using the indentation of the line that context.pos is on is generally problematic—in insertNewlineAndIndent the indent context's simulateBreak feature is used, which is ignored when you go straight through the doc, and more generally, there is no indentation for the new line yet, so trying to compute that to use as a baseline isn't generally meaningful.

I'll spend more time looking into this later, but at a glance, I don't think the use of `currentIndent` is solid—try pressing enter after something like... ``` def foo(): foo(bar, baz) ``` That will align the cursor with `baz`. Using the indentation of the line that `context.pos` is on is generally problematic—in `insertNewlineAndIndent` the indent context's [`simulateBreak`](https://codemirror.net/6/docs/ref/#language.IndentContext.constructor^options.simulateBreak) feature is used, which is ignored when you go straight through the doc, and more generally, there is no indentation for the new line yet, so trying to compute that to use as a baseline isn't generally meaningful.

microbit-matt-hillsdon commented

2021-07-27 19:07:09 +02:00

(Migrated from github.com)

Thanks @marijnh. I'll give that some thought tomorrow. For the example you give, it indents under baz which does match VS Code. That doesn't feel unexpected to me, but doubtless some of this is the behaviours you're used to.

I'm interested in using current indent because I'm worried we otherwise end up fighting the user's intentional indent changes. I think this is the worst example, where the user intends to end the block, leave a blank line and start a new block of code but the indentation is forced into alignment with the body:

Absent a clear indication of the end of the block I think you probably need to consider the current indent.

Thanks @marijnh. I'll give that some thought tomorrow. For the example you give, it indents under baz which does match VS Code. That doesn't feel unexpected to me, but doubtless some of this is the behaviours you're used to. I'm interested in using current indent because I'm worried we otherwise end up fighting the user's intentional indent changes. I think this is the worst example, where the user intends to end the block, leave a blank line and start a new block of code but the indentation is forced into alignment with the body: ![reindent](https://user-images.githubusercontent.com/44397098/127196961-088d70f7-3745-4768-a2aa-8122cb14eae5.gif) Absent a clear indication of the end of the block I think you probably need to consider the current indent.

marijnh commented

2021-08-14 12:52:20 +02:00

(Migrated from github.com)

Does the behavior in @codemirror/lang-python 0.19.2 address your concerns?

Does the behavior in \@codemirror/lang-python 0.19.2 address your concerns?

microbit-matt-hillsdon commented

2021-08-14 13:45:26 +02:00

(Migrated from github.com)

Does the behavior in @codemirror/lang-python 0.19.2 address your concerns?

That looks interesting, thank you. I'm away for the next week, but I'll take a proper look when I'm back and follow up here.

> Does the behavior in @codemirror/lang-python 0.19.2 address your concerns? That looks interesting, thank you. I'm away for the next week, but I'll take a proper look when I'm back and follow up here.

mattpauldavies commented

2021-08-16 18:47:02 +02:00

(Migrated from github.com)

For what it's worth, I've been testing @codemirror/lang-python 0.19.2 today and it looks to be pretty much bang on! Amazing work as ever, @marijnh 🙌

For what it's worth, I've been testing `@codemirror/lang-python 0.19.2` today and it looks to be pretty much bang on! Amazing work as ever, @marijnh 🙌

marijnh commented

2021-08-16 21:41:03 +02:00

(Migrated from github.com)

Cool. Going to close this, feel free to report any further mess-ups that you find.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

WIP: Revise Python indent behaviour #4

Pull request closed