<html>
<head>
<base href="https://bugs.llvm.org/">
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW - onInitialize: Capabilities.textDocumentSync.save should be of type SaveOptions"
href="https://bugs.llvm.org/show_bug.cgi?id=45979">45979</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>onInitialize: Capabilities.textDocumentSync.save should be of type SaveOptions
</td>
</tr>
<tr>
<th>Product</th>
<td>clang-tools-extra
</td>
</tr>
<tr>
<th>Version</th>
<td>unspecified
</td>
</tr>
<tr>
<th>Hardware</th>
<td>PC
</td>
</tr>
<tr>
<th>OS</th>
<td>Linux
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>normal
</td>
</tr>
<tr>
<th>Priority</th>
<td>P
</td>
</tr>
<tr>
<th>Component</th>
<td>clangd
</td>
</tr>
<tr>
<th>Assignee</th>
<td>unassignedclangbugs@nondot.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>planger@eclipsesource.com
</td>
</tr>
<tr>
<th>CC</th>
<td>llvm-bugs@lists.llvm.org
</td>
</tr></table>
<p>
<div>
<pre>The property textDocumentSync is of type TextDocumentSyncOptions, as defined
below (see also [1])
export interface TextDocumentSyncOptions {
...
save?: SaveOptions;
}
and its property save should follow the type SaveOptions (see also [1]):
export interface SaveOptions {
/**
* The client is supposed to include the content on save.
*/
includeText?: boolean;
}
Thus, imho the definition of capabilities should imho be:
capabilities: {
...
textDocumentSync: {
openClose: true,
...,
save: { includeText: true }
}
...
}
instead of what clangd sends since commit
596b63ad4019e61030803789a1844a0f1aeb34db:
capabilities: {
...
textDocumentSync: {
openClose: true,
...,
save: true
}
...
}
If you agree, this should be a rather easy fix in
clang-tools-extra/clangd/ClangdLSPServer.cpp:582 [2], since it returns just a
static value here.
Note that the LSP spec is (imho) a bit unclear on that, because it also
defines:
"
Server Capability:
property name (optional): textDocumentSync.save
property type: boolean | SaveOptions where SaveOptions is defined as follows:
" [1]
So here the spec defines that property save can also be boolean (as used by
clangd currently). However, in the type definition of TextDocumentSyncOptions,
it doesn't say TextDocumentSyncOptions.save?: SaveOptions | boolean.
Either way, since it is such an easy fix in clangd and because the type
definition of TextDocumentSyncOptions is more explicit, I'd find it great, if
clangd would follow the type definition of TextDocumentSyncOptions.
Otherwise LSP frameworks/clients may fail to parse what is currently sent by
clangd. I've seen, for instance, that LSP4J [3] fails to use clangd-11 right
now because of this inconsistency.
Thanks so much in advance!
[1]
<a href="https://microsoft.github.io/language-server-protocol/specifications/specification-current/">https://microsoft.github.io/language-server-protocol/specifications/specification-current/</a>
[2]
<a href="https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clangd/ClangdLSPServer.cpp#L582">https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clangd/ClangdLSPServer.cpp#L582</a>
[3] <a href="https://github.com/eclipse/lsp4j">https://github.com/eclipse/lsp4j</a></pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>