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

Support onTypeFormatting to format comment blocks #180

Open
p-bakker opened this issue Jan 17, 2021 · 13 comments
Open

Support onTypeFormatting to format comment blocks #180

p-bakker opened this issue Jan 17, 2021 · 13 comments

Comments

@p-bakker
Copy link

Hi,

Originally raised this case with Wild Web Developer (eclipse-wildwebdeveloper/wildwebdeveloper#599), but apparently the behavior comes from here.

When I'm in a multiline comment and hit enter, I expect the new line to automatically get an indent + space + asterisk + space. However, I just get an empty line

Example

before hitting enter:

    /**
     * %%cursorPosition%%
     * 
     */

currently after hitting enter:

    /**
     * 
%%cursorPosition%%
     * 
     */

expected after hitting enter:

    /**
     * 
     * %%cursorPosition%%
     * 
     */
@predragnikolic
Copy link
Contributor

I do not consider this to be a bag with the langage server. The text editor should hanle this behavior. I will move the discussion to the eclipse issue. 🙂

@mickaelistria
Copy link

See eclipse-wildwebdeveloper/wildwebdeveloper#599 (comment) , this is a typical LS feature: listening to file change and reacting sending an applyEdit.

@mickaelistria
Copy link

See https://microsoft.github.io/language-server-protocol/specification#textDocument_onTypeFormatting that's actually even more streamlined way to provide such things via LSP.

@DonnieWest
Copy link
Collaborator

@mickaelistria do you have an example of an LSP that currently does this? How do they handle editors that have this functionality built in without conflicting? Please provide any configs that you're using :)

I'm personally not aware of any LSP that shoulders this responsibility, instead handing the functionality over to the editor (in VIM we use syntax files to handle indentation and commenting)

@mickaelistria
Copy link

do you have an example of an LSP that currently does this

YAML Language Server is currently in the process of adding a similar feature to automatically add whitespaces and dashes at the beginning of lines. See redhat-developer/yaml-language-server#389
JDT-LS also does onTypeFormatting for similar things.

How do they handle editors that have this functionality built in without conflicting?

I guess they assume that the client doesn't do anything on that matter, since after all it's a language-specific capability. Or they just look at the clientId in capability to disable it for clients that are known to provide a similar functionality.

I'm personally not aware of any LSP that shoulders this responsibility, instead handing the functionality over to the editor (in VIM we use syntax files to handle indentation and commenting)

Yaml, JDT-LS and probably several others (look for onTypeFormatting) on GitHub do leverage this feature for this family of edits (extra prefix newlines with language-specific stuff). Also, if you look at VSCode, it actually does that for TypeScript, just not using LSP but directly tsserver.
This is actually the proper way, as it's language-specific semantic ("we're in a comment block, so we should start by a *" is not something a text editor can or should know about). The fact the some extra syntax files are necessary is somehow putting language-specific logic in the text editor and duplicate maintenance effort for that kind of things across multiple editors, breaking the whole purpose of LSP.

@rchl
Copy link
Member

rchl commented Jan 18, 2021

One could argue that it's not the responsibility of this server to format the documentation comments and I'm fairly sure that tssever has no capability to do that.

Many text editors (including VSCode), handle that (comments formatting) itself through internal functionality, not through language servers.

@mickaelistria
Copy link

One could argue that it's not the responsibility of this server to format the documentation comments

Documentation are part of the language, they are formatted according to language-specific specification or convention. If it's not the responsibility of the language server to provide support for language-specific parts of the text edition; then whose is it?

I'm fairly sure that tssever has no capability to do that.

My bad, I didn't look carefully at https://github.com/microsoft/vscode/blob/master/extensions/typescript-language-features/src/languageFeatures/formatting.ts#L60 says "client".

Many text editors (including VSCode), handle that (comments formatting) itself through internal functionality, not through language servers.

What other editor than VSCode do you have in mind that support proper comments formatting?
Think about it the other way round: imagine someone saying "Eclipse IDE has support for comments formatting in Java code, so why should we add it to JDT-LS? Other editors can just mimic what's done in Eclipse IDE (eg implement a parser, some formatting rules and custom actions, 1 per editor)". I hope you realize it doesn't really sound like what LSP is expected to resolve.

@rchl
Copy link
Member

rchl commented Jan 18, 2021

What other editor than VSCode do you have in mind that support proper comments formatting?

SublimeText, which is my primary text editor of choice.

I hope you realize it doesn't really sound like what LSP is expected to resolve.

My view on that is that LSP is supposed to provide enhancements to the code editor that are hard or impossible to implement in an editor that is not an IDE. LSP doesn't seem to be that well suited for formatting the document on typing as there is an inherent delay to all responses from an LSP server and that would be especially noticeable and potentially distracting when formatting on typing.

Also, JSDoc comments are not even typescript specific.

@predragnikolic
Copy link
Contributor

@mickaelistria thanks for all these information.

This is actually the proper way, as it's language-specific semantic ("we're in a comment block, so we should start by a *" is not something a text editor can or should know about)

After reading all what you said, I would say that it is the proper LSP way to do it,
but I would keep in mind the LSP way doesn't always guarantee the best user experience.

Implementing it in the client can of mean that the client has to start parsing the document to know whether the edit is in a comment area. That's a bad smell: with LSP, everything that requires parsing should take place in the LS, not in the client.
ref: eclipse-wildwebdeveloper/wildwebdeveloper#599 (comment)

Here is the reason why I thought that the text editor should implement this.
In cases where I expect immediate feedback, (example. I want the * as soon as I press enter)
if the client has its own parser that detects comment, it will do it probably noticeably faster that doing it over with LSP.

I tried the PR that you linked - redhat-developer/yaml-language-server#389
And while I can see a slight delay when pressing enter and showing the *, the user experience is still good, I would say great, but on the other hand it would be good if the delay didn't exist at all.

In both applications(VS Code, Sublime Text), I just held down enter. Here is the difference:
output

Comparing both of those approaches(text editor implementation vs language server implementation) can have it's pros and cons. But because this is LSP spec, doing it on the server now make more sense to me.

@rchl rchl changed the title New line in comment should show comment prefix Support onTypeFormatting to format comment blocks Jul 4, 2021
@rchl
Copy link
Member

rchl commented Jul 4, 2021

Personally I think it's very unlikely that this will get implemented but will keep it as a feature request.

@rchl rchl added this to To do in Roadmap via automation Dec 2, 2021
@rchl
Copy link
Member

rchl commented Dec 2, 2021

Actually my bad, the tsserver supports this feature. But not in the case requested in the first comment. It only sets up the following trigger characters: ;, }, \n and it doesn't insert * when pressing enter in a JSDoc block comment. I've tried in VSCode after disabling the built-in auto-indentation (it has worked in some other cases for auto-indenting the code).

@rchl rchl removed this from To do in Roadmap Dec 2, 2021
@p-bakker
Copy link
Author

The origin of this request (in WWD) has now been solved by proper use on onEnterRules through TM4E, so it's handled clientside, in the editor.

I do think however that lsp's shoulds support onTypeFormatting, to take care stuff like Block comments, trimming trailing whitespace or whatever else makes sense in the language at hand.

The lsp specification has the onTypeFormatting command (in addition to document and range formatting events) for a reason, so that any editor out of the box has everything that developers expect of an editor.

Editors can then enhance the UX by taking care of some things themselves, like the block comments or trimming whitespace.

Imho that wouldn't interfere with onTypeFormatting at all, assuming the editor does its formatting magic BEFORE sending the changes to the lsp for the onTypeFormatting command and the clientside formatting results in the lsp not having to do any formatting anymore, as the code is already formatted accordingly

@rchl
Copy link
Member

rchl commented Sep 18, 2022

Once this feature is supported, this server won't be attempting to implement additional logic on top of what tsserver (TypeScript) provides itself so I would suggest making enhancement suggestions in https://github.com/microsoft/TypeScript if it doesn't support something you would expect. You can test what's supported by testing VSCode although you should disable built-in VSCode logic before testing.

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

No branches or pull requests

5 participants