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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
