sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clangd/CodeComplete.cpp:301
+ } else if (!IncludePath->empty()) {
+ if (auto Edit = Includes.insert(*IncludePath)) {
+ I.detail += ((I.detail.empty() ? "" : "\n") +
----------------
This is fine for now, but can you add a FIXME to consider what to show if
there's no include to be inserted, and for sema results? There's some value in
showing the header consistently I think.
================
Comment at: clangd/CodeComplete.cpp:302
+ if (auto Edit = Includes.insert(*IncludePath)) {
+ I.detail += ((I.detail.empty() ? "" : "\n") +
+ StringRef(*IncludePath).trim('"'))
----------------
can we add the \n for now even if detail is empty?
That way the editors that "inline" the detail into the same line as the label
won't show it (if we patch YCM to truncate like VSCode does).
The inconsistency (sometimes showing a return type and sometimes a header)
seems surprising.
================
Comment at: clangd/CodeComplete.cpp:313
+ I.label =
+ (InsertingInclude ? Opts.IncludeInsertionIndicator : " ") + I.label;
I.scoreInfo = Scores;
----------------
string(IncludeInsertionIndicator.size(), ' ')?
================
Comment at: clangd/CodeComplete.cpp:334
Opts.EnableSnippets ? (Name + "(${0})").str() : Name.str();
- First.label = (Name + "(…)").str();
+ // Keep the first character of the original label which is an visual
+ // indicator for include insertion or an indentation.
----------------
again, need to account for the length of the indicator
================
Comment at: clangd/CodeComplete.h:63
- // Populated internally by clangd, do not set.
+ /// Populated internally by clangd, do not set.
/// If `Index` is set, it is used to augment the code completion
----------------
if you want this to be part of the doc comment, move it to the bottom?
I think the intent was that this comment apply to all following members. So
keeping the line as `//` and adding the new member above it would also make
sense
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48163
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
