Author: kadircet Date: Thu Aug 23 05:19:39 2018 New Revision: 340527 URL: http://llvm.org/viewvc/llvm-project?rev=340527&view=rev Log: [clangd] Move function argument snippet disable mechanism from LSP rendering to internal clangd reprensentation.
Summary: We were handling the EnableFunctionArgSnippets only when we are producing LSP response. Move that code into CompletionItem generation so that internal clients can benefit from that as well. Reviewers: ilya-biryukov, ioeric, hokein Reviewed By: ilya-biryukov Subscribers: MaskRay, jkorous, arphaman, cfe-commits Differential Revision: https://reviews.llvm.org/D51102 Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=340527&r1=340526&r2=340527&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Aug 23 05:19:39 2018 @@ -269,7 +269,8 @@ struct CodeCompletionBuilder { CodeCompletionString *SemaCCS, const IncludeInserter &Includes, StringRef FileName, const CodeCompleteOptions &Opts) - : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments) { + : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments), + EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets) { add(C, SemaCCS); if (C.SemaResult) { Completion.Origin |= SymbolOrigin::AST; @@ -385,10 +386,17 @@ private: } std::string summarizeSnippet() const { - if (auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>()) - return *Snippet; - // All bundles are function calls. - return "(${0})"; + auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>(); + if (!Snippet) + // All bundles are function calls. + return "($0)"; + if (!Snippet->empty() && !EnableFunctionArgSnippets && + ((Completion.Kind == CompletionItemKind::Function) || + (Completion.Kind == CompletionItemKind::Method)) && + (Snippet->front() == '(') && (Snippet->back() == ')')) + // Check whether function has any parameters or not. + return Snippet->size() > 2 ? "($0)" : "()"; + return *Snippet; } std::string summarizeSignature() const { @@ -402,6 +410,7 @@ private: CodeCompletion Completion; SmallVector<BundledEntry, 1> Bundled; bool ExtractDocumentation; + bool EnableFunctionArgSnippets; }; // Determine the symbol ID for a Sema code completion result, if possible. @@ -1413,16 +1422,8 @@ CompletionItem CodeCompletion::render(co LSP.additionalTextEdits.push_back(FixIt); } } - if (Opts.EnableSnippets && !SnippetSuffix.empty()) { - if (!Opts.EnableFunctionArgSnippets && - ((Kind == CompletionItemKind::Function) || - (Kind == CompletionItemKind::Method)) && - (SnippetSuffix.front() == '(') && (SnippetSuffix.back() == ')')) - // Check whether function has any parameters or not. - LSP.textEdit->newText += SnippetSuffix.size() > 2 ? "(${0})" : "()"; - else - LSP.textEdit->newText += SnippetSuffix; - } + if (Opts.EnableSnippets) + LSP.textEdit->newText += SnippetSuffix; // FIXME(kadircet): Do not even fill insertText after making sure textEdit is // compatible with most of the editors. Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=340527&r1=340526&r2=340527&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Thu Aug 23 05:19:39 2018 @@ -1140,7 +1140,7 @@ TEST(CompletionTest, OverloadBundling) { // For now we just return one of the doc strings arbitrarily. EXPECT_THAT(A.Documentation, AnyOf(HasSubstr("Overload with int"), HasSubstr("Overload with bool"))); - EXPECT_EQ(A.SnippetSuffix, "(${0})"); + EXPECT_EQ(A.SnippetSuffix, "($0)"); } TEST(CompletionTest, DocumentationFromChangedFileCrash) { @@ -1648,55 +1648,35 @@ TEST(SignatureHelpTest, IndexDocumentati SigDoc("Doc from sema")))); } -TEST(CompletionTest, RenderWithSnippetsForFunctionArgsDisabled) { +TEST(CompletionTest, CompletionFunctionArgsDisabled) { CodeCompleteOptions Opts; - Opts.EnableFunctionArgSnippets = true; - { - CodeCompletion C; - C.RequiredQualifier = "Foo::"; - C.Name = "x"; - C.SnippetSuffix = "()"; - - auto R = C.render(Opts); - EXPECT_EQ(R.textEdit->newText, "Foo::x"); - EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText); - } - Opts.EnableSnippets = true; Opts.EnableFunctionArgSnippets = false; + const std::string Header = + R"cpp( + void xfoo(); + void xfoo(int x, int y); + void xbar(); + void f() { + )cpp"; { - CodeCompletion C; - C.RequiredQualifier = "Foo::"; - C.Name = "x"; - C.SnippetSuffix = ""; - - auto R = C.render(Opts); - EXPECT_EQ(R.textEdit->newText, "Foo::x"); - EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet); + auto Results = completions(Header + "\nxfo^", {}, Opts); + EXPECT_THAT( + Results.Completions, + UnorderedElementsAre(AllOf(Named("xfoo"), SnippetSuffix("()")), + AllOf(Named("xfoo"), SnippetSuffix("($0)")))); } - { - CodeCompletion C; - C.RequiredQualifier = "Foo::"; - C.Name = "x"; - C.SnippetSuffix = "()"; - C.Kind = CompletionItemKind::Method; - - auto R = C.render(Opts); - EXPECT_EQ(R.textEdit->newText, "Foo::x()"); - EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet); + auto Results = completions(Header + "\nxba^", {}, Opts); + EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf( + Named("xbar"), SnippetSuffix("()")))); } - { - CodeCompletion C; - C.RequiredQualifier = "Foo::"; - C.Name = "x"; - C.SnippetSuffix = "(${0:bool})"; - C.Kind = CompletionItemKind::Function; - - auto R = C.render(Opts); - EXPECT_EQ(R.textEdit->newText, "Foo::x(${0})"); - EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet); + Opts.BundleOverloads = true; + auto Results = completions(Header + "\nxfo^", {}, Opts); + EXPECT_THAT( + Results.Completions, + UnorderedElementsAre(AllOf(Named("xfoo"), SnippetSuffix("($0)")))); } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
