-
Notifications
You must be signed in to change notification settings - Fork 53
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 LSP based source code formatting on file save #761
Comments
Hi @ghentschke , you can already implement format on save with current LSP4E. See #117. Would that work for you? |
I'll ckeck that. Thanks for the hint! |
Unfortunately is TextDocumentSyncOptions [
openClose = true
change = Incremental
willSave = null
willSaveWaitUntil = null
save = Either [
left = true
right = null
]
] |
This package is needed in DocumentProviders of LSP based editors to support the format on file save action. Only needed for servers who do not support willSaveWaitUntil fixes eclipse#761
@rubenporras: Is there a way a user can disable the format-on-safe when using #117 |
If there is an standard way of doing that, I could not find it. For our implementation, we extended the client with an API
and with a configuration page. Then, the server ask the client if it should apply autoformat. That works for us because we develop our own server and our own client, but since cdt-lsp is an independent project, that will not work. If you figure it out, I would be interested. |
One solution might be a OSGi service in LSP4E. This service can be provided by vendors who provides a server via the languageServer extension point as well. The provided interface of that service could look something like: public interface IFormatOnSaveEnabler{
Optional<Boolean> isFormatOnSaveEnabled(URI uri);
} |
@ghentschke I'm not much enthusiast in opening an internal package as API for a use-case that already has a LSP-standard approach. Couldn't clangd be changed to support standard LSP approach with willSaveUntil? Or maybe could clangd be changed to send an |
I know that this approach is not the best solution. I'll open an issue in the clangd repo. This should be a temporary solution until clangd supports Maybe make |
If this feature is very urgent or critical for CDT, we can add a x-friends. However, it's better if we can just do nothing in LSP4E exactly to ensure that the work is addressed where it's the most valuable (clangd) |
I am little stuck in the middle now. I've to argue why
I can share this evaluation. I would propose to support both approaches in LSP4E: the formatting via private void formatCode(IProgressMonitor monitor, ITextFileBuffer buffer) {
var document = buffer.getDocument();
if (document != null) {
try {
IRegion[] changedRegions = isLimitedFormatCode()
? EditorUtility.calculateChangedLineRegions(buffer, monitor)
: new IRegion[] { new Region(0, document.getLength()) };
var textSelection = new MultiTextSelection(document, changedRegions);
formatter.requestFormatting(document, textSelection).get(1000, TimeUnit.MILLISECONDS)
.ifPresent(edits -> {
try {
edits.apply();
} catch (final ConcurrentModificationException ex) {
// ServerMessageHandler.showMessage(Messages.LSPFormatHandler_DiscardedFormat,
// new MessageParams(MessageType.Error,
// Messages.LSPFormatHandler_DiscardedFormatResponse));
} catch (BadLocationException e) {
Platform.getLog(getClass()).error(e.getMessage(), e);
}
});
} catch (BadLocationException | CoreException | InterruptedException | ExecutionException
| TimeoutException e) {
Platform.getLog(getClass()).error(e.getMessage(), e);
}
}
}
The settings could be fetched from the editor as proposed in this comment. |
I now how it feels to be stuck between two projects with two opinions myself :) Regarding on why the server should support willSaveWaitUntil, to me it has the advantage than it works in all the clients, which is kind of the point of LSP. The documentation on this is sparse, but for example, Neovim announces it: https://neovim.io/doc/user/news-0.9.html
I agree that the user (and thus the client) should control when to apply the edits and which ones, and that looks like it is not an standard in the protocol. Yet, from the examples, one can imagine that the intention of the client capabilities is to enable/disable it, as taken from https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#clientCapabilities
Which such a document selector, the client can tell the server for which languages to apply Maybe you could open a ticket for the LSP project and ask for clarification? |
Agree. But that could be a long road (clangd has to implement it. See my answer in my clangd issue) Could we implement my proposal above as a fallback in case the server does not support |
if server does not support willSaveWaitUntil the code can be formatted prior to save. fixes eclipse#761
if server does not support willSaveWaitUntil the code can be formatted prior to save. fixes eclipse#761
Following on this, I have found this morning https://learn.microsoft.com/en-us/visualstudio/extensibility/adding-an-lsp-extension?view=vs-2022, it says:
The text gives then the example of the setting So it looks to me that the usual way of support it would be that the server can autoformat and that the behaviour can be controlled by such options. You could just return the value compatible by clangd by overriding the class that this PR https://github.com/eclipse/lsp4e/pull/792/files fixes (I have realized all this by reviewing this change). I think that this technically much easier than #783 and its all part of the spec. Could you maybe still convince clangd developers or make a PR for them? It looks to me that we are creating mid-size custom code with #783, so maybe it is worth checking again that it is indeed needed? |
I could but since there are ~567 open issues and the last commit was on April 14 I guess it will take a long time until this feature is implemented. Unfortunately I have not the time to implement it by myself in clangd. |
if server does not support willSaveWaitUntil the code can be formatted prior to save. fixes #761
As a user of a LSP based editor I want to format my source code on a file save action. (see this cdt-lsp issue)
Therefor I want to use the
org.eclipse.lsp4e.operations.format.LSPFormatter
class. The packageorg.eclipse.lsp4e.operations.format
should be exported.The text was updated successfully, but these errors were encountered: