Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spike - auto indentation #8669

Closed
luabud opened this issue Nov 19, 2019 · 4 comments
Closed

Spike - auto indentation #8669

luabud opened this issue Nov 19, 2019 · 4 comments
Assignees
Labels
area-editor-* User-facing catch-all feature-request Request for new features or functionality

Comments

@luabud
Copy link
Member

luabud commented Nov 19, 2019

To understand where do we start

@luabud luabud added area-editor-* User-facing catch-all needs proposal Need to make some design decisions feature-request Request for new features or functionality labels Nov 19, 2019
@luabud luabud changed the title Spike - Understand where do we start for getting auto indentation Spike - auto indentation Nov 19, 2019
@luabud luabud added this to the FY20Q2 milestone Nov 19, 2019
@karthiknadig
Copy link
Member

Figure out what work needs to be done to use formatOnType, what limitations are there, and how we can get it to work appropriately. LS is not going to do format on type.

@kimadeline
Copy link

  • look at the code we have that implements format on type
  • see if it can be reworked to only do indentation, or will we have to drop it

@karrtikr
Copy link

karrtikr commented Dec 10, 2019

There are three approaches to solve this:

1. VSCode Language Configuration

VSCode provides OnEnterRules and indentationRules which dedents/indents the block given the previous line matches the regex. But regexes can't be used to cover every case, and the current rules are not powerful enough. microsoft/vscode#66235
Also sometimes just knowledge of previous line is not sufficient to make a decision on the indent.

2. On-type indenting

When editor.formatOnType is set to true, one can intervene in between when a user types and indent accordingly. We currently use this in onEnterFormatter.ts and other places. We can use this to cover all the cases one by one. But on-type formatting is not enabled by default nor is very popular (and enables a slew of formatting rules other than cursor alignment), so forcing users to enable on-type formatting as a whole would be good to avoid.

3. Add a key binding for enter

We can add something like

"keybindings": [
    {
        "command": "<Command key>",
        "key": "enter",
        "when": "editorTextFocus && editorLangId == python"
    }
],

(may need more modifying)

and use that to intervene. This way we don't need to have editor.formatOnType enabled. However we need to keep in mind that in this case we're responsible for cursor alignment after the user presses enter, otherwise cursor would stay where it is.

As there is no progress on vscode upstream issues related to 1., going with 3. makes the most sense at this point.

https://github.com/kbrose/vsc-python-indent/blob/master/src/indent.ts seems like a wonderful place to take help from. It covers most cases (#481) we currently have open issues for and is well documented. We can use npm package python-indent-parser similarly to the way @kbrose does.

Also another thing to note here is that having both language configuration and key bindings can override each other sometimes. So we should be moving away from regexes when we solve work items for this.

EDIT: We've decided to go with 2.

Work items

@karrtikr
Copy link

Closed in favor of #8996 (comment)

@ghost ghost removed the needs proposal Need to make some design decisions label Dec 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-editor-* User-facing catch-all feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants