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

Unspecified client behavior: textDocument/onTypeFormatting #1430

Closed
siegel opened this issue Mar 23, 2022 · 5 comments
Closed

Unspecified client behavior: textDocument/onTypeFormatting #1430

siegel opened this issue Mar 23, 2022 · 5 comments

Comments

@siegel
Copy link
Contributor

siegel commented Mar 23, 2022

In the description for textDocument/onTypeFormatting, the specification says only:

The document on type formatting request is sent from the client to the server to format parts of the document during typing.

In the parameters, the comment for the ch parameter says only:

	/**
	 * The character that has been typed.
	 */
	ch: string;

What is not clear from this (and seems very important for a client developer to know) is whether the typed character should be entered into the document's contents (which to me also means that a textDocument/didChange notification is sent to the server) before the textDocument/onTypeFormatting request is sent.

Or should none of that happen, and the client should send textDocument/onTypeFormatting before entering the character into the document's contents, and subsequently only do so if the server returns no edits?

I'd be grateful if this could be explicitly specified (and I wouldn't mind knowing for my own edification, since the answer is immediately relevant to my interests :-) ).

@rwols
Copy link
Contributor

rwols commented Mar 23, 2022

See #1053

@siegel
Copy link
Contributor Author

siegel commented Mar 23, 2022

@rwols Thank you very much - that is quite helpful. (And I'm glad I'm not the only one who had that question. :-) )

I have one remaining question: is the location specified in the position parameter the location at which the character gets inserted? Or is it the location of where the insertion point ends up after the text (including the typed character and/or any automatically generated text) have been inserted?

I think that a statement in the spec along the lines of @dbaeumer's advice might prevent future confusion; specifically (with minor copyediting):

onTypeFormatting should always act on the synced server state; thus, clients must always flush any pending content changes to the server before issuing textDocument/onTypeFormatting in order to ensure a correct response.

@dbaeumer in #1053 I see your point about the specification not requiring a specific order of operations by the client other than ensuring that the server is up to date.

On the other hand, not saying something explicit like…

the client must insert the typed character and nothing else

or

the client must not insert the typed character

or

the client must insert the typed character and any automatically generated text it thinks is appropriate

or even (not recommended)

the client may insert whatever it wants, or nothing at all, and the server is required to act on the current synced contents of the document, using the inserted character as a hint

…leaves (IMO) too much room for variance in server behavior.

This in turn, leaves client developers at the mercy of the "it works in VS Code, therefore it's correct" trap and in the unpleasant position of needing to support all kinds of server-specific special cases.

So, if as @rwols describes in #1053 that the client can (and should) insert any text that it feels appropriate before syncing the changes to the server, I think that should be strengthened to "must" and then documented as such in the specification.

And I think the specification would also do well to clarify whether the position parameter indicates the location of the typed character vs. the location of the insertion point after any edits the client has made to the backing store.

Thanks for reading. :-)

@dbaeumer
Copy link
Member

The position doesn't denote any concrete insertion. Concrete insertion positions always come with document change events.

The position denotes the position in the file around which the format on type should happen. The character denotes the character that has triggered the format on type request. They usually are side by side to each other, but they don't necessarily have to.

@HighCommander4
Copy link

The position denotes the position in the file around which the format on type should happen.

Does this mean it's the server's responsibility to be flexible, and e.g. in the case of formatting after a newline, perform the same formatting whether the position in the request is just before or just after the newline?

@dbaeumer
Copy link
Member

dbaeumer commented Apr 1, 2022

The idea is actually that the position indicates the cursor position (but since LSP allows sending arbitrary request, it can't be speced that way). So, the answer to your questions is yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants