ark: Reindent Lines action doesn't match as-you-type indentation behavior

[kevinushey]

It looks like VSCode has some separate code paths for handling indentation...

This is used for the "Reindent Lines" command:

https://github.com/microsoft/vscode/blob/06bd7ae776e2706a4aba2690d95788327dd16c43/src/vs/editor/contrib/indentation/browser/indentation.ts#L29-L139

This is used for regular as-you-type indentation:

https://github.com/microsoft/vscode/blob/06bd7ae776e2706a4aba2690d95788327dd16c43/src/vs/editor/common/languages/autoIndent.ts#L278-L364

Unfortunately, the indentation rules as currently constructed seem to behave well for as-you-type indentation, but not for the explicit Reindent Lines command.

[kevinushey]

More likely, the issue here is that we use some fairly complex onEnter rules to handle indentation, but because the indentation engine doesn't know about these, it doesn't handle re-indenting of code properly.

https://github.com/rstudio/myriac/blob/74aed836106c300524aae8f8b7feed9c8bab8212/amalthea/crates/ark/extension/language-configuration.json#L46-L114

[kevinushey]

There's a couple things we could consider here:

1. Patch the existing getReindentEditOperations function, to support onEnterRules and also (presumedly) electric character behaviors,
2. Allow languages to register an indentation command, which would then let the extension handle reindent themselves via the VSCode API,
3. A version of the above, but using the LSP.

I think the cleanest way forward would likely be (2), and it would also give us an extension point separate from VSCode (and so reduce the risk of upstream conflicts)

[kevinushey]

Not necessary for MVP.

[jennybc]

@kevinushey Do you have any concrete examples that really highlight the problem you have in mind? As in, how do we suffer from this?

I played around a bit with breakpoints in places suggested by the above links, but am having trouble forming a call to action without knowing exactly what we're trying to fix.

[jennybc]

Adding more notes about the complexity of indentation. Complexity in the sense that indentation comes about in several different ways.

microsoft/vscode#111088: Summary: the Reindent Lines command seems to operate purely from indentationRules in the language configuration. If you don't have such rules, Reindent Lines won't do anything. In contrast, there is default indentation behaviour when the user is typing.

If there is no indentation rule set for the programming language, the editor will indent when the line ends with an open bracket and outdent when you type a closing bracket. The bracket here is defined by brackets. from https://code.visualstudio.com/api/language-extensions/language-configuration-guide#indentation-rules

We have taken control of user-is-typing indentation, because we have onEnterRules.

microsoft/vscode#111089: This is a very concrete demo of what @kevinushey's getting at, i.e. indentation-as-you-type != indentation-via-reindent-lines. Official VS Code response:

Currently the auto indent feature in VS Code is regex based, which doesn't really understand the scope (function, statement) etc and it's a challenge to tweak the indentation rules and the engine to work for all common cases. Formatting works more consistently for the scenarios listed above, so I would recommend to enable formatting on type or save. Closing this as we don't expect to get around it in the near future.

Which highlights the fact that we need a formatter (#1407).

Finally, I think the indentation (and formatting more generally) when you paste is yet another, third thing and I think also related to having registered a formatter.

Notice that editor.formatOnPaste setting is controlled by the DocumentRangeFormattingEditProvider and not affected by auto indentation. from https://code.visualstudio.com/api/language-extensions/language-configuration-guide#indentation-rules

---

The Editor: Auto Indent setting is relevant.

none: The editor will not insert indentation automatically.
keep: The editor will keep the current line's indentation.
brackets: The editor will keep the current line's indentation and honor language defined brackets.
advanced: The editor will keep the current line's indentation, honor language defined brackets and invoke special onEnterRules defined by languages.
full: The editor will keep the current line's indentation, honor language defined brackets, invoke special onEnterRules defined by languages, and honor indentationRules defined by languages.

https://github.com/microsoft/vscode/blob/f42c55740dc3dea712ad25d30691fb09efedd25b/src/vs/editor/common/config/editorOptions.ts#L5287-L5293

[kevinushey]

Interesting; it seems like this might "just work" now? I wish I had included an example in my initial report, but IIRC the Reindent Lines command wasn't handling the indentation of closing parens / brackets as I would have expected.

[jennybc]

Ah, no, I should have updated here. I have now experienced how different the indentation can be from "indent-as-you-go" vs. "Reindent lines". Although concrete examples are always welcome! But I get it now.

[kevinushey]

Either way, it seems like "Reindent Lines" is mostly useless once you have language support for proper formatters...

[jennybc]

I guess "Reindent lines" is for the case when you want help re-indenting but you explicitly don't want other formatting opinions enforced on your file? But I agree it's a gesture we might decide we don't care deeply about.

I think if we can make our onEnterRules as good as they can be, people won't perceive much need to run "Reindent Lines". And our answer to "multiple lines are wonky" is to offer a formatter that can act on a whole document or selection.

[jthomasmock]

Just noting that I am running into "as-you-type" indentation issues as well. Should that go into a separate issue?

Note the indent increasing on every new line.

library(dplyr)

mtcars |> 
  group_by(cyl) |> 
    mutate(mean = mean(mpg)) |> 
      arrange(desc(mean))

Screen.Cast.2023-10-10.at.10.50.56.AM.mp4

[jennybc]

I think our existing indentation issues probably capture the problem 😬

#1316, #1319, #1327

[lionel-]

Jenny nicely summed up the problem, we really need a formatter to fix this. We have RC plans for a rudimentary indent-only formatter in #2251 (comment) so I'm bumping this issue to RC.