<div dir="ltr">This broke buildbots due to a failing test.<div>r361846 should fit it.<br></div><div><br></div><div>Sorry for the inconvenience.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 28, 2019 at 5:30 PM Ilya Biryukov via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: ibiryukov<br>
Date: Tue May 28 08:33:37 2019<br>
New Revision: 361841<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=361841&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=361841&view=rev</a><br>
Log:<br>
[clangd] Place cursor better after completing patterns<br>
<br>
Summary:<br>
By producing the $0 marker in the snippets at the last placeholder.<br>
This produces nicer results in most cases, e.g. for<br>
namespace <#name#> {<br>
<#decls#><br>
}<br>
<br>
we now produce ${0:decls} instead of ${2:decls} and the final cursor<br>
placement is more convenient.<br>
<br>
Reviewers: hokein<br>
<br>
Reviewed By: hokein<br>
<br>
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits<br>
<br>
Tags: #clang<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D62389" rel="noreferrer" target="_blank">https://reviews.llvm.org/D62389</a><br>
<br>
Modified:<br>
clang-tools-extra/trunk/clangd/CodeComplete.cpp<br>
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp<br>
clang-tools-extra/trunk/clangd/CodeCompletionStrings.h<br>
clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp<br>
clang-tools-extra/trunk/clangd/unittests/CodeCompletionStringsTests.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=361841&r1=361840&r2=361841&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=361841&r1=361840&r2=361841&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue May 28 08:33:37 2019<br>
@@ -394,8 +394,9 @@ struct CodeCompletionBuilder {<br>
Bundled.emplace_back();<br>
BundledEntry &S = Bundled.back();<br>
if (C.SemaResult) {<br>
+ bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern;<br>
getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix,<br>
- &Completion.RequiredQualifier);<br>
+ &Completion.RequiredQualifier, IsPattern);<br>
S.ReturnType = getReturnType(*SemaCCS);<br>
} else if (C.IndexResult) {<br>
S.Signature = C.IndexResult->Signature;<br>
<br>
Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=361841&r1=361840&r2=361841&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=361841&r1=361840&r2=361841&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Tue May 28 08:33:37 2019<br>
@@ -11,6 +11,8 @@<br>
#include "clang/AST/DeclObjC.h"<br>
#include "clang/AST/RawCommentList.h"<br>
#include "clang/Basic/SourceManager.h"<br>
+#include "clang/Sema/CodeCompleteConsumer.h"<br>
+#include <limits><br>
#include <utility><br>
<br>
namespace clang {<br>
@@ -73,8 +75,23 @@ std::string getDeclComment(const ASTCont<br>
}<br>
<br>
void getSignature(const CodeCompletionString &CCS, std::string *Signature,<br>
- std::string *Snippet, std::string *RequiredQualifiers) {<br>
- unsigned ArgCount = 0;<br>
+ std::string *Snippet, std::string *RequiredQualifiers,<br>
+ bool CompletingPattern) {<br>
+ // Placeholder with this index will be ${0:…} to mark final cursor position.<br>
+ // Usually we do not add $0, so the cursor is placed at end of completed text.<br>
+ unsigned CursorSnippetArg = std::numeric_limits<unsigned>::max();<br>
+ if (CompletingPattern) {<br>
+ // In patterns, it's best to place the cursor at the last placeholder, to<br>
+ // handle cases like<br>
+ // namespace ${1:name} {<br>
+ // ${0:decls}<br>
+ // }<br>
+ CursorSnippetArg =<br>
+ llvm::count_if(CCS, [](const CodeCompletionString::Chunk &C) {<br>
+ return C.Kind == CodeCompletionString::CK_Placeholder;<br>
+ });<br>
+ }<br>
+ unsigned SnippetArg = 0;<br>
bool HadObjCArguments = false;<br>
for (const auto &Chunk : CCS) {<br>
// Informative qualifier chunks only clutter completion results, skip<br>
@@ -124,8 +141,10 @@ void getSignature(const CodeCompletionSt<br>
break;<br>
case CodeCompletionString::CK_Placeholder:<br>
*Signature += Chunk.Text;<br>
- ++ArgCount;<br>
- *Snippet += "${" + std::to_string(ArgCount) + ':';<br>
+ ++SnippetArg;<br>
+ *Snippet +=<br>
+ "${" +<br>
+ std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) + ':';<br>
appendEscapeSnippet(Chunk.Text, Snippet);<br>
*Snippet += '}';<br>
break;<br>
<br>
Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.h?rev=361841&r1=361840&r2=361841&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.h?rev=361841&r1=361840&r2=361841&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.h (original)<br>
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.h Tue May 28 08:33:37 2019<br>
@@ -38,12 +38,16 @@ std::string getDeclComment(const ASTCont<br>
/// Formats the signature for an item, as a display string and snippet.<br>
/// e.g. for const_reference std::vector<T>::at(size_type) const, this returns:<br>
/// *Signature = "(size_type) const"<br>
-/// *Snippet = "(${0:size_type})"<br>
+/// *Snippet = "(${1:size_type})"<br>
/// If set, RequiredQualifiers is the text that must be typed before the name.<br>
/// e.g "Base::" when calling a base class member function that's hidden.<br>
+///<br>
+/// When \p CompletingPattern is true, the last placeholder will be of the form<br>
+/// ${0:…}, indicating the cursor should stay there.<br>
void getSignature(const CodeCompletionString &CCS, std::string *Signature,<br>
std::string *Snippet,<br>
- std::string *RequiredQualifiers = nullptr);<br>
+ std::string *RequiredQualifiers = nullptr,<br>
+ bool CompletingPattern = false);<br>
<br>
/// Assembles formatted documentation for a completion result. This includes<br>
/// documentation comments and other relevant information like annotations.<br>
<br>
Modified: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp?rev=361841&r1=361840&r2=361841&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp?rev=361841&r1=361840&r2=361841&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp Tue May 28 08:33:37 2019<br>
@@ -2382,6 +2382,28 @@ TEST(CompletionTest, ObjectiveCMethodTwo<br>
EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));<br>
}<br>
<br>
+TEST(CompletionTest, CursorInSnippets) {<br>
+ clangd::CodeCompleteOptions Options;<br>
+ Options.EnableSnippets = true;<br>
+ auto Results = completions(<br>
+ R"cpp(<br>
+ void while_foo(int a, int b);<br>
+ void test() {<br>
+ whil^<br>
+ })cpp",<br>
+ /*IndexSymbols=*/{}, Options);<br>
+<br>
+ // Last placeholder in code patterns should be $0 to put the cursor there.<br>
+ EXPECT_THAT(<br>
+ Results.Completions,<br>
+ Contains(AllOf(Named("while"),<br>
+ SnippetSuffix("(${1:condition}){${0:statements}\n}"))));<br>
+ // However, snippets for functions must *not* end with $0.<br>
+ EXPECT_THAT(Results.Completions,<br>
+ Contains(AllOf(Named("while_foo"),<br>
+ SnippetSuffix("(${1:int a}, ${2:int b})"))));<br>
+}<br>
+<br>
TEST(CompletionTest, WorksWithNullType) {<br>
auto R = completions(R"cpp(<br>
int main() {<br>
<br>
Modified: clang-tools-extra/trunk/clangd/unittests/CodeCompletionStringsTests.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompletionStringsTests.cpp?rev=361841&r1=361840&r2=361841&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompletionStringsTests.cpp?rev=361841&r1=361840&r2=361841&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/unittests/CodeCompletionStringsTests.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/unittests/CodeCompletionStringsTests.cpp Tue May 28 08:33:37 2019<br>
@@ -22,10 +22,12 @@ public:<br>
CCTUInfo(Allocator), Builder(*Allocator, CCTUInfo) {}<br>
<br>
protected:<br>
- void computeSignature(const CodeCompletionString &CCS) {<br>
+ void computeSignature(const CodeCompletionString &CCS,<br>
+ bool CompletingPattern = false) {<br>
Signature.clear();<br>
Snippet.clear();<br>
- getSignature(CCS, &Signature, &Snippet);<br>
+ getSignature(CCS, &Signature, &Snippet, /*RequiredQualifier=*/nullptr,<br>
+ CompletingPattern);<br>
}<br>
<br>
std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator;<br>
@@ -99,6 +101,25 @@ TEST_F(CompletionStringTest, EscapeSnipp<br>
EXPECT_EQ(Snippet, "(${1:\\$p\\}1\\\\})");<br>
}<br>
<br>
+TEST_F(CompletionStringTest, SnippetsInPatterns) {<br>
+ auto MakeCCS = [this]() -> const CodeCompletionString & {<br>
+ CodeCompletionBuilder Builder(*Allocator, CCTUInfo);<br>
+ Builder.AddTypedTextChunk("namespace");<br>
+ Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);<br>
+ Builder.AddPlaceholderChunk("name");<br>
+ Builder.AddChunk(CodeCompletionString::CK_Equal);<br>
+ Builder.AddPlaceholderChunk("target");<br>
+ Builder.AddChunk(CodeCompletionString::CK_SemiColon);<br>
+ return *Builder.TakeString();<br>
+ };<br>
+ computeSignature(MakeCCS(), /*CompletingPattern=*/false);<br>
+ EXPECT_EQ(Snippet, " ${1:name} = ${2:target};");<br>
+<br>
+ // When completing a pattern, the last placeholder holds the cursor position.<br>
+ computeSignature(MakeCCS(), /*CompletingPattern=*/true);<br>
+ EXPECT_EQ(Snippet, " ${1:name} = ${0:target};");<br>
+}<br>
+<br>
TEST_F(CompletionStringTest, IgnoreInformativeQualifier) {<br>
Builder.AddTypedTextChunk("X");<br>
Builder.AddInformativeChunk("info ok");<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>