client-server requst: onEnter

[matklad]

On Enter

Server Capability: { "onEnter": boolean }

This request is send from client to server to handle Enter keypress.

Method: experimental/onEnter

Request:: TextDocumentPositionParams

Response:: SnippetTextEdit[]

Example

fn main() {
    // Some /*cursor here*/ docs
    let x = 92;
}

experimental/onEnter returns the following snippet

fn main() {
    // Some
    // $0 docs
    let x = 92;
}

The primary goal of onEnter is to handle automatic indentation when opening a new line.
This is not yet implemented.
The secondary goal is to handle fixing up syntax, like continuing doc strings and comments, and escaping \n in string literals.

As proper cursor positioning is raison-d'etat for onEnter, it uses SnippetTextEdit.

Unresolved Question

• How to deal with synchronicity of the request?
  One option is to require the client to block until the server returns the response.
  Another option is to do a OT-style merging of edits from client and server.
  A third option is to do a record-replay: client applies heuristic on enter immediatelly, then applies all user's keypresses.
  When the server is ready with the response, the client rollbacks all the changes and applies the recorded actions on top of the correct response.
• How to deal with multiple carets?
• Should we extend this to arbitrary typed events and not just onEnter?
• The name seems wrong.

[matklad]

As a user, this thing is kind of a big deal for me, because always correct indentation on Enter is a pretty transformative experience. In IntelliJ, the caret is always where you want it to be, even if you don't think about this consciously.

[puremourning]

Isn’t this what format request and on type formatting is for?

This idea seems flawed to me because there are many other ways a new line could be inserted and many other ways that auto indent or format could or would be triggered. I feel it’s way too narrow and/or specific.

[matklad]

I feel it’s way too narrow and/or specific.

It is indeed pretty narrow implementation-wise, but, in terms of user experience, indenting the cursor right is very important (this opinion is informed by experience with IntelliJ, which handles this properly, using a code formatted to determine the target indentation).

We might be able to fold this into on type formatting request, if we allow it to specify cursor position/snippets and if we actually invoke it for \n (I think vscode didn’t last time I checked).

[puremourning]

Isn’t this just a weakness of your client/editor? I’ve never used an editor that didn’t behave the way you describe (or could be configured to).

The LSP should be about general concepts that are widely applicable, rather than specific quirks of one particular client.

[matklad]

Isn’t this just a weakness of your client/editor?

That's exactly my point, any editor which doesn't parse source code according to the language grammar is not powerful enough to correctly handle indent on enter. You can get xy% there using big enough pile of regular expressions, but this would be wrong conceptually (why heuristics when you can just parse code) and, subjectively, doesn't work well enough in practice (if I compare IntelliJ with any other editor).

To give some specific examples:

1 is Kotlin's syntax aware enter handler.
2 (commit that introduced most machinery).
3 another bit of machinery responsible for Kotlin onEnter. Not sure what's relationship between 1 and 3, on the first blush, looks that 3 is the general case + on type formatting changes, and 1 is special case for lambdas.

Significant parts of this logic depend on syntax tree (PsiElement and KtXXX stuff). And this logic is only additional custom stuff for Kotlin specifically, the base case (not entirely sure I am linking the right bit of code, not really an expert in IntelliJ typing machinery) "just" delegates to formatting model which, again, needs to build a syntax tree.

Now, of course the fact that one can use syntax tree for onEnter doesn't mean that one should. It's possible to approximate this stuff using much simpler text-based approach, and some users might prefer it. But I think IntelliJ provides a good evidence that a lot of users care about this behavior. I certainly do, as a user :)

[puremourning]

So should this feature be ‘semantic indent for line or range’ then ? Eg a request from client to server to get the indent for a specific line.

That is general, clearly identifies the purpose so as not to abuse the event, and can be implemented in any editor no matter what paradigm (modal, not modal, speech, whatever).

I can’t imagine why snippets are relevant here. Not least because snippets are a devisive UX and not all users want them.

[matklad]

Just indenting won't be enough to handle, for example, the example from the top of this issue.

fn main() {
    // Some /*cursor here*/ docs
    let x = 92;
}
->
fn main() {
    // Some
    // $0 docs
    let x = 92;
}

Maybe this is decomposable into on type formatting and reindent line, but my gut feeling that this would be an instance of midlayer mistake. I think it's better to give as much power as possible to the servers and see what patterns emerge, rather than to try to factor-out commonality. One request per UI-concern seems like a good design principle, and "what happens if you press Enter" is a specific UI-concern.

I can’t imagine why snippets are relevant here.

They are a convenient way to specify target cursor position ($0). I agree that full snippets are probably not relevant here, so restricting them to only contain $0 seems fin by me.

[puremourning]

give as much power as possible to the servers

I actually disagree because server vendors only test their servers with one client. The result is that you get wonky experience everywhere else. The protocol should not favour a specific user experience, use case or UI. It should relate semantic information in a way that allows editors to build features.

Midlayer or not, LSP is an abstraction layer. 'onEnter' is a leaky abstraction.

[dbaeumer]

I agree that this shouldn't be folded with on type formatting which intention is formatting of white spaces and not snippet insertion.

We also try to not query the server on basic user typing since it can result in very wired user experience if the server is for some reason lagging the response. This is why in VS Code all this needs to be contributed upfront to ensure it can be executed in the renderer.

[matklad]

We also try to not query the server on basic user typing since it can result in very wired user experience if the server is for some reason lagging the response. This is why in VS Code all this needs to be contributed upfront to ensure it can be executed in the renderer.

Yeah, I agree that performance (and more generally, synchronicity) requirements here are non-trivial. Note, however, that we already have one effectively synchronous request for a basic user action -- extend selection. Granted, selection model is not as core as the basic text model, but I would say that general situation is similar.

My gut feeling is that:

• this could be made fast enough for locally-running servers (it works in rust-analyzer quite well)
• we have to do this, as alternatives are not really compelling:
  • "regex-based" processing is limited
  • something like tree-sitter based processing should be powerful enough, but, at that stage, why not just embed a language server into the editor?
• finally, I am theoretically intrigued by record/replay approach, which look like it could both:
  • mask latency from slow servers
  • be reliable (ie, the end results does not depend on user's typing speed, only on the sequence of keystrokes)

EDIT: A bit of trivia: in rust-anlayzer, we specifically distinguish synchronous requests and we handle them directly on the main loop (usually, a request goes onto thread pool):

https://github.com/rust-analyzer/rust-analyzer/blob/dbb2c153ffad541474f2e5c7c7bd7ea93cf68f5f/crates/rust-analyzer/src/main_loop.rs#L507-L512

At the moment, out of 4 sync requests one is extend selection, and others are our extensions.