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

Making auto indentation work #8996

Closed
karrtikr opened this issue Dec 10, 2019 · 7 comments
Closed

Making auto indentation work #8996

karrtikr opened this issue Dec 10, 2019 · 7 comments
Labels
area-editor-* User-facing catch-all area-formatting feature-request Request for new features or functionality needs PR Ready to be worked on

Comments

@karrtikr
Copy link

karrtikr commented Dec 10, 2019

As noted in spike #8669

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

Note we also do dedenting when user presses semi-colon : https://github.com/Microsoft/vscode-python/blob/3c7bb774519ebf5aed91f90e10c7b8dd0fc567ec/src/client/typeFormatters/blockFormatProvider.ts#L25
Make sure these does not conflict with what Kbrose's extension is doing.

@ghost ghost added the triage-needed Needs assignment to the proper sub-team label Dec 10, 2019
@karrtikr karrtikr added area-editor-* User-facing catch-all needs PR labels Dec 10, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Dec 10, 2019
@karrtikr karrtikr added triage-needed Needs assignment to the proper sub-team feature-request Request for new features or functionality area-formatting and removed triage-needed Needs assignment to the proper sub-team labels Dec 10, 2019
@kbrose
Copy link

kbrose commented Dec 14, 2019

If you have any questions about my code let me know.

Be sure to also check out https://github.com/DSpeckhals/python-indent-parser/blob/master/src/index.ts for the heavy lifting on the parsing side (library written by @DSpeckhals with contributions from me). It should be pretty thoroughly documented.

@karrtikr
Copy link
Author

@kbrose Yes, thank you.

The conditions when the shortcut is triggered have been heavily restricted, but there may still be times this extension is unexpectedly overriding Enter behavior.

Can you recall about cases where extension unexpectedly is overriding what your extension is doing? We would want to correct that.

@kbrose
Copy link

kbrose commented Dec 18, 2019

I have not personally noticed any misbehaving on that front. The only issue I've received about unexpected behavior related to overriding Enter was related to the vim plugin, which I resolved here.

@J-Fields
Copy link

Hey there! VSCodeVim relies on the editor.action.reindentselectedlines command to place your cursor in the right place after a command like cc (change current line). I believe this relies on the given language's indentationRules (this is an educated guess - please correct me if I'm wrong).

Is there any chance you could define indentationRules, even if you override their effect with more complicated logic with formatOnType?

Happy to discuss and/or help out with this. Thanks for the great extension!

@karrtikr
Copy link
Author

@J-Fields If you browse through the mentioned issues, we used to have indentationRules, and it caused a few bugs, so we decided to remove it.
Also I am not sure if we can override their effect with formatOnType, i.e I am not sure what happens when rules conflict.

As indentationRules don't support every indentation scenario, we can try adding indentationRules after all the other work is done.

@karrtikr
Copy link
Author

karrtikr commented Sep 8, 2021

It seems "Line being moved is automatically indented according to the indentation level of its context (i.e. the line(s) above it)": #6886 is not supported with @kbrose 's extension.

@github-actions github-actions bot removed the needs PR label Aug 9, 2022
@karrtikr karrtikr added the needs PR Ready to be worked on label Aug 9, 2022
@karrtikr
Copy link
Author

This is now supported by Pylance so closing this. If anyone still faces an issue with this, feel free to open up an issue on https://github.com/microsoft/pylance-release.

@karrtikr karrtikr closed this as not planned Won't fix, can't repro, duplicate, stale Dec 20, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-editor-* User-facing catch-all area-formatting feature-request Request for new features or functionality needs PR Ready to be worked on
Projects
None yet
Development

No branches or pull requests

3 participants