[PATCH] D81380: [clangd] Don't produce snippets when completion location is followed by parenthesis

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall marked an inline comment as done.
sammccall added a comment.

Ship it!



================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1715
+                        CCContextKind, Opts, !IsUsingDeclaration,
+                        NextTokenKind);
       else
----------------
kbobyrev wrote:
> sammccall wrote:
> > now you've got the "should we generate snippets" logic split a different way :-) No-snippets-for-using-decl is expressed here, but no-args-if-parens-exist is inside the builder.
> > 
> > I think it's slightly neater (less plumbing) to compute it here - it's safe to do so based on the current item because we won't produce a bundle containing functions together with other things.
> > 
> > Also fine to pass in IsUsingDeclaration, but it should be to a parameter called "IsUsingDeclaration" rather than one called GenerateSnippets, as it's only part of the equation.
> > Also fine to pass in IsUsingDeclaration, but it should be to a parameter called "IsUsingDeclaration" rather than one called GenerateSnippets, as it's only part of the equation.
> 
> Yeah I thought it might be a bit confusing to split this up, but couldn't figure out what would be best option. This sounds logical, will do!
> 
> > I think it's slightly neater (less plumbing) to compute it here - it's safe to do so based on the current item because we won't produce a bundle containing functions together with other things.
> 
> I'm a bit confused: I moved the `l_brace` token detection logic into the builder because it resolves the completion kind, so computing it here would require the completion item kind to be resolved at this point. So are you suggesting moving the completion kind resolution out of the builder here and then passing it to the builder along with `GenerateSnippets`?
Yeah, I didn't see that we don't yet have access to the kind here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81380/new/

https://reviews.llvm.org/D81380