ilya-biryukov added a comment.

Sorry for late response.



================
Comment at: clangd/ClangdUnit.cpp:610
+        ParameterInformation Info;
+        OptionalParameterLabel = getOptionalString(*Chunk.Optional);
+        Result.label += OptionalParameterLabel;
----------------
Are we first concatenating the `CodeCompletionString` inside optional chunks 
and then trying to parse them again here?
Can we extract `ProcessChunks` function and recursively call it with 
`Chunk.Optional->Chunks`?


================
Comment at: clangd/ClangdUnit.cpp:650
+
+template <class ReturnType, class CodeCompleteConsumerType>
+ReturnType
----------------
rwols wrote:
> Not sure if these template names are according to style.
It's ok. See my comment below, though, it looks like we can avoid making this 
function template in the first place.


================
Comment at: clangd/ClangdUnit.cpp:652
+ReturnType
+invokeClangAction(PathRef FileName, tooling::CompileCommand Command,
+                  PrecompiledPreamble const *Preamble, StringRef Contents,
----------------
Maybe rename to `invokeCodeCompletion`? `invokeClangAction` seems too generic.


================
Comment at: clangd/ClangdUnit.cpp:705
+  Clang->setCodeCompletionConsumer(
+      new CodeCompleteConsumerType(FrontendOpts.CodeCompleteOpts, Results));
 
----------------
Maybe receive `CodeCompleteConsumer` and not make function template?
We can return whether calling an action succeeded instead.


================
Comment at: test/clangd/signature-help.test:39
+# I'm just putting the questionable result in here now as the expected result.
+# CHECK-DAG: {"label":"bar(float x = 0, int y = 42) -> 
void","parameters":[{"label":"float x = 0, int y = 42"}]}
+# 
----------------
rwols wrote:
> When there are multiple defaulted parameters after each other, the 
> CK_Optional chunk consists of *all* of those parameters, instead of a 
> CK_Optional chunk per parameter. This might require us to dive into 
> SemaCodeComplete.cpp to fix this. I'm just leaving it as-is right now.
But does `CodeCompletionString` inside `Chunk->Optional` contain those extra 
parameters as a separate chunk?



https://reviews.llvm.org/D38048



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to