kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/CodeComplete.cpp:483 bool EnableFunctionArgSnippets; + bool CompleteArgumentList; }; ---------------- maybe rather use `GenerateSnippets`? Since this doesn't generate completions for the snippets, but rather disables generation of any snippets at all. also I think it makes sense to document this one, because of the field just above it. ================ Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1466 Output.Completions.back().Score = C.second; Output.Completions.back().CompletionTokenRange = ReplacedRange; } ---------------- ilya-biryukov wrote: > kadircet wrote: > > why not handle `SnippetSuffix` in here ? > > > > instead of propagating `IsUsingDeclaration`, we can just drop > > `Output.Completions.back().SnippetSuffix` in here, which sounds like a more > > appropriate layering. > > considering we don't really have context specific knowledge in > > `CodeCompletionBuilder` ? > I was trying to keep all processing of snippets in one place. > Code completion code is hard enough to navigate as it stands today... > > Although I agree doing this when summarizing items in a bundles looks like > the wrong layer, but this place is also after bundling, so I'm not sure if > it's actually winning us much. > ah ok, that's also a good concern. feel free to choose one or the other then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69382/new/ https://reviews.llvm.org/D69382 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits