[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

Ilya Biryukov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 24 09:53:38 PDT 2018


ilya-biryukov added inline comments.


================
Comment at: lib/Sema/CodeCompleteConsumer.cpp:557
+      if (!Tags.empty())
+        OS << " (" << llvm::join(Tags, ",") << ")";
+      if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString(
----------------
ioeric wrote:
> ilya-biryukov wrote:
> > NIT: Maybe add a space between the items, i.e. join with `", "` as a separator?
> Considering we put everything in one line, I think it's reasonable to drop the spaces to keep the output compact. These tags are more expected to be read by tests than human after all.
I would argue this output is also read by humans, therefore we should add a space.
We do "grep" the results with FileCheck in tests and technically it does not matter if we have a space or not. But the users still look at the output when (1) something goes wrong or (2) they read through the test cases.


Repository:
  rL LLVM

https://reviews.llvm.org/D53635





More information about the llvm-commits mailing list