[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov added a comment. In https://reviews.llvm.org/D37101#868207, @rwols wrote: > Thanks! Thinking ahead now so we're on the same page, you will be > implementing the client's initialize request, and I'll start work on > textDocument/signatureHelp. Sounds good. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
rwols added a comment. Thanks! Thinking ahead now so we're on the same page, you will be implementing the client's initialize request, and I'll start work on textDocument/signatureHelp. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov closed this revision. ilya-biryukov added a comment. Marking as closed, had to commit by hand without `arc patch` as it couldn't find base revision to apply the patch on. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov added a comment. Done. It's now in. Thanks for the contribution! Had to make a few changes before committing. - Fix compilation of tests. - Update VSCode "engine" dependency in vscode toy client. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
rwols added a comment. No, I don't have access to the repo, sorry forgot to mention this. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov added a comment. @rwols, do you have access to svn repo? If not, I can submit this change for you. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
rwols updated this revision to Diff 114415.
rwols added a comment.
Update the description for the "-enable-snippets" option.
https://reviews.llvm.org/D37101
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/Protocol.cpp
clangd/clients/clangd-vscode/package.json
clangd/tool/ClangdMain.cpp
test/clangd/authority-less-uri.test
test/clangd/completion-snippet.test
test/clangd/completion.test
Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -16,25 +16,25 @@
# nondeterministic, so we check regardless of order.
#
# CHECK: {"jsonrpc":"2.0","id":1,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
Content-Length: 148
{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":5}}}
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":2,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
# Update the source file and check for completions again.
Content-Length: 226
@@ -47,7 +47,7 @@
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":3,"result":[
-# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","insertText":"func"}
+# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","insertText":"func","insertTextFormat":1}
# CHECK: ]}
Content-Length: 44
Index: test/clangd/completion-snippet.test
===
--- /dev/null
[PATCH] D37101: [clangd] Add support for snippet completions
krasimir accepted this revision. krasimir added a comment. Great! https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
Thanks for the change! Looks good minus an outdated description of
`enable-snippets` flag.
Do you have commit access to llvm repository?
Comment at: clangd/tool/ClangdMain.cpp:31
+EnableSnippets("enable-snippets",
+ llvm::cl::desc("Disable \"snippet\" completions and only "
+ "present plaintext completions."),
Description still mentions `disable-snippets`.
https://reviews.llvm.org/D37101
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
rwols updated this revision to Diff 114242.
rwols added a comment.
Change command-line option back to "-enable-snippets", disable snippets by
default. This is a temporary solution and we should instead inspect the
"initialize" request from the client to check wether the client supports
snippets.
https://reviews.llvm.org/D37101
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/Protocol.cpp
clangd/clients/clangd-vscode/package.json
clangd/tool/ClangdMain.cpp
test/clangd/authority-less-uri.test
test/clangd/completion-snippet.test
test/clangd/completion.test
Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -16,25 +16,25 @@
# nondeterministic, so we check regardless of order.
#
# CHECK: {"jsonrpc":"2.0","id":1,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
Content-Length: 148
{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":5}}}
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":2,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
# Update the source file and check for completions again.
Content-Length: 226
@@ -47,7 +47,7 @@
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":3,"result":[
-# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","insertText":"func"}
+# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","insertText":"func","insert
[PATCH] D37101: [clangd] Add support for snippet completions
rwols marked an inline comment as done. rwols added inline comments. Comment at: clangd/tool/ClangdMain.cpp:33 + "present plaintext completions."), +llvm::cl::init(false)); + ilya-biryukov wrote: > After putting some thought into it, let's disable snippets by default. Sorry > for driving this in a wrong direction in my previous comments. > Instead of using flag, LSP servers should look at initial `initialize` > request and a value of > `TextDocumentClientCapabilities.completion.completionItem.snippetSupport`. > (see > https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#initialize-request). > And it seems bad from clangd side to break clients that don't support > snippets **by default**. It's probably ok if user passed custom command-line > arguments, overriding the defaults. > > The problem is that we don't parse the `ClientCapabilities` now. It's not > hard to add parsing of it, but that should probably be done in a separate > commit and I can do this work if you want. > I think it's fine to commit this as is, **defaulting to not return > snippets**, and add parsing of `ClientCapabilities` with a follow-up commit. Yes, I agree snippets should be enabled/disabled by reading the client's capabilities. This command line flag should be a temporary solution. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.cpp:376
+
+CompletionItem Item{InsertTextFormat::PlainText};
+
rwols wrote:
> ilya-biryukov wrote:
> > Implementations of this function in `PlainTextCompletionItemsCollector` and
> > `SnippetCompletionItemsCollector` are the same.
> > Maybe make `ProcessChunks` virtual instead?
> >
> > Or maybe consider replacing inheritance with a `bool` flag. Inheritance
> > does not seem to buy us much here. This looks simpler:
> > ```
> > if (EnableSnippets)
> > ProcessChunksWithSnippets(CCS, Item);
> > else
> > ProcessChunksWithoutSnippets(CCS, Item);
> >
> > ```
> >
> >
> I went with the "make ProcessChunks virtual" approach, wouldn't your
> suggestion have an impact on performance? There'd be a check for every
> completion item now.
Oh, that should not make any difference performance-wise. There is many more
branching on each completion item even if we assume all the function calls are
inlined.
And virtual calls are not free either.
https://reviews.llvm.org/D37101
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov added a comment.
Looks good, just one major drawback to address. We shouldn't break clients by
default (see comment in `ClangdMain.cpp`).
And a minor comment about flag naming.
Comment at: clangd/tool/ClangdMain.cpp:30
+static llvm::cl::opt
+DisableSnippets("disable-snippets",
+llvm::cl::desc("Disable \"snippet\" completions and only "
Maybe replace a negated flag with positive alternative, e.g. `enable-snippets`
or `use-snippets`?
Reading such "implicit" double negations is sometimes confusing (e.g.
`!DisableSnippets` instead of `EnableSnippets`).
Comment at: clangd/tool/ClangdMain.cpp:33
+ "present plaintext completions."),
+llvm::cl::init(false));
+
After putting some thought into it, let's disable snippets by default. Sorry
for driving this in a wrong direction in my previous comments.
Instead of using flag, LSP servers should look at initial `initialize` request
and a value of
`TextDocumentClientCapabilities.completion.completionItem.snippetSupport`.
(see
https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#initialize-request).
And it seems bad from clangd side to break clients that don't support snippets
**by default**. It's probably ok if user passed custom command-line arguments,
overriding the defaults.
The problem is that we don't parse the `ClientCapabilities` now. It's not hard
to add parsing of it, but that should probably be done in a separate commit and
I can do this work if you want.
I think it's fine to commit this as is, **defaulting to not return snippets**,
and add parsing of `ClientCapabilities` with a follow-up commit.
https://reviews.llvm.org/D37101
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
rwols marked 7 inline comments as done.
rwols added inline comments.
Comment at: clangd/ClangdUnit.cpp:376
+
+CompletionItem Item{InsertTextFormat::PlainText};
+
ilya-biryukov wrote:
> Implementations of this function in `PlainTextCompletionItemsCollector` and
> `SnippetCompletionItemsCollector` are the same.
> Maybe make `ProcessChunks` virtual instead?
>
> Or maybe consider replacing inheritance with a `bool` flag. Inheritance does
> not seem to buy us much here. This looks simpler:
> ```
> if (EnableSnippets)
> ProcessChunksWithSnippets(CCS, Item);
> else
> ProcessChunksWithoutSnippets(CCS, Item);
>
> ```
>
>
I went with the "make ProcessChunks virtual" approach, wouldn't your suggestion
have an impact on performance? There'd be a check for every completion item now.
https://reviews.llvm.org/D37101
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] Add support for snippet completions
rwols updated this revision to Diff 113899.
rwols added a comment.
- Don't use const for non-reference parameters
- `lowerCamelCase` for functions, verb phrase
- Field definitions at the bottom of the class
- Make ProcessChunks virtual
- Add comment that fallthrough to the next case is intended
- Revert new constructors for CompletionItem, they are pointless
- Bump minimum version for vscode-languageclient and vscode-languageserver to
3.3.0
https://reviews.llvm.org/D37101
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/Protocol.cpp
clangd/clients/clangd-vscode/package.json
clangd/tool/ClangdMain.cpp
test/clangd/authority-less-uri.test
test/clangd/completion-snippet.test
test/clangd/completion.test
Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -16,25 +16,25 @@
# nondeterministic, so we check regardless of order.
#
# CHECK: {"jsonrpc":"2.0","id":1,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
Content-Length: 148
{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":5}}}
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":2,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
# Update the source file and check for completions again.
Content-Length: 226
@@ -47,7 +47,7 @@
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":3,"result":[
-# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","insertText":"f
[PATCH] D37101: [clangd] Add support for snippet completions
rwols updated this revision to Diff 113901.
rwols added a comment.
- Fix failing clangd tests
https://reviews.llvm.org/D37101
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/Protocol.cpp
clangd/clients/clangd-vscode/package.json
clangd/tool/ClangdMain.cpp
test/clangd/authority-less-uri.test
test/clangd/completion-snippet.test
test/clangd/completion.test
Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -1,4 +1,4 @@
-# RUN: clangd -run-synchronously < %s | FileCheck %s
+# RUN: clangd -run-synchronously -disable-snippets < %s | FileCheck %s
# It is absolutely vital that this file has CRLF line endings.
#
Content-Length: 125
@@ -16,25 +16,25 @@
# nondeterministic, so we check regardless of order.
#
# CHECK: {"jsonrpc":"2.0","id":1,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
Content-Length: 148
{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":5}}}
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":2,"result":[
-# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
-# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb"}
-# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc"}
-# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator="}
-# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake"}
-# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f"}
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a","insertTextFormat":1}
+# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"00035bb","filterText":"bb","insertText":"bb","insertTextFormat":1}
+# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"00035ccc","filterText":"ccc","insertText":"ccc","insertTextFormat":1}
+# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"00034operator=","filterText":"operator=","insertText":"operator=","insertTextFormat":1}
+# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"00034~fake","filterText":"~fake","insertText":"~fake","insertTextFormat":1}
+# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"00035f","filterText":"f","insertText":"f","insertTextFormat":1}
# CHECK: ]}
# Update the source file and check for completions again.
Content-Length: 226
@@ -47,7 +47,7 @@
# Repeat the completion request, expect the same results.
#
# CHECK: {"jsonrpc":"2.0","id":3,"result":[
-# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","insertText":"func"}
+# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"00034func","filterText":"func","in
[PATCH] D37101: [clangd] Add support for snippet completions
ilya-biryukov added a comment.
Looks good overall, just a bunch of comments before accepting the revision.
Mostly minor code style issues, sorry for spamming you with those.
Comment at: clangd/ClangdLSPServer.h:30
ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
+ const bool SnippetCompletions,
llvm::Optional ResourceDir);
Code style: we don't use `const` for non-reference parameters.
Comment at: clangd/ClangdUnit.cpp:275
+std::string SnippetEscape(const llvm::StringRef Text) {
+ std::string Result;
Code style: function names are `lowerCamelCase`.
Also maybe rename to `escapeSnippet` to follow the style guide: "function names
should be verb phrases, e.g. `openFile()` or `isFoo()`" (see
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly)
Comment at: clangd/ClangdUnit.cpp:315
+protected:
+ std::vector &Items;
+ std::shared_ptr Allocator;
Code style: we put all field definitions at the bottom of the class in clangd.
Also: why not make the fields private?
Comment at: clangd/ClangdUnit.cpp:376
+
+CompletionItem Item{InsertTextFormat::PlainText};
+
Implementations of this function in `PlainTextCompletionItemsCollector` and
`SnippetCompletionItemsCollector` are the same.
Maybe make `ProcessChunks` virtual instead?
Or maybe consider replacing inheritance with a `bool` flag. Inheritance does
not seem to buy us much here. This looks simpler:
```
if (EnableSnippets)
ProcessChunksWithSnippets(CCS, Item);
else
ProcessChunksWithoutSnippets(CCS, Item);
```
Comment at: clangd/ClangdUnit.cpp:454
+// a declarator or macro.
+Item.filterText = Chunk.Text;
+ case CodeCompletionString::CK_Text:
Maybe add a comment that fallthrough to the next case is intended?
It's very easy to miss that there's no break intentionally when reading the
code.
Comment at: clangd/ClangdUnit.cpp:533
+Item.insertText += Chunk.Text;
+// Don't even add a space to the label.
+ }
Maybe add `break` here?
To avoid unintentional fallthrough if new `case` statement gets added.
Comment at: clangd/Protocol.h:393
+ CompletionItem() = default;
+ CompletionItem(const InsertTextFormat Format);
+
Maybe remove these constructors?
It seems weird that `insertTextFormat` is initialized on construction, while
all other fields are filled up later.
Having to write all the fields explicitly is pretty low-level, but it's
consistent and is done in all other structs from `Protocol.h`.
Comment at: clangd/tool/ClangdMain.cpp:31
+static llvm::cl::opt EnableSnippets(
+"enable-snippets",
+llvm::cl::desc("Present completions with embedded snippets instead of "
I've managed to get snippet completion working in VSCode.
The problem is that we're using outdated version of `vscode-languageclient`
dependency in our npm module.
Simply bumping versions of dependencies in
`clangd\clients\clangd-vscode\package.json` worked.
Had to change
```
"dependencies": {
"vscode-languageclient": "^2.6.3",
"vscode-languageserver": "^2.6.2"
},
```
to
```
"dependencies": {
"vscode-languageclient": "^3.3.0",
"vscode-languageserver": "^3.3.0"
},
```
Maybe include this in this commit and make `-enable-snippets` true by default?
https://reviews.llvm.org/D37101
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
