This revision was automatically updated to reflect the committed changes.
Closed by commit rG138adb098032: [clangd] Refine logic on $0 in completion
snippets (authored by zyounan).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145319/new/
zyounan added inline comments.
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:60
+bool shouldPatchPlaceholder0(CodeCompletionResult::ResultKind ResultKind,
+ CXCursorKind CursorKind) {
nridge wrote:
> nit: let's add
zyounan updated this revision to Diff 506001.
zyounan marked an inline comment as done.
zyounan added a comment.
Add description to shouldPatchPlaceholder0
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145319/new/
https://reviews.llvm.org/D145319
nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.
Thanks! I have just one final nit, then please feel free to merge.
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:60
+bool
zyounan added a comment.
Updated now. Thank you again for your patient review. :)
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:213
++SnippetArg;
- if (SnippetArg == CursorSnippetArg) {
+ if (ShouldPatchPlaceholder0 && SnippetArg ==
zyounan updated this revision to Diff 505420.
zyounan marked 3 inline comments as done.
zyounan added a comment.
Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145319/new/
https://reviews.llvm.org/D145319
Files:
nridge added a comment.
Thanks for the update! I have a couple of more minor suggestions, hope you
don't mind; I'm trying to make sure the next person to look at this will easily
understand what the code is trying to do.
Comment at:
zyounan added inline comments.
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.h:45
///
/// When \p CompletingPattern is true, the last placeholder will be of the form
/// ${0:…}, indicating the cursor should stay there.
Sorry, I forget to revise
zyounan updated this revision to Diff 504950.
zyounan added a comment.
Update doxygen comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145319/new/
https://reviews.llvm.org/D145319
Files:
clang-tools-extra/clangd/CodeComplete.cpp
zyounan added inline comments.
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:441
getSignature(*SemaCCS, , ,
- , IsPattern);
+ , IsPattern, C.SemaResult);
if (!C.SemaResult->FunctionCanBeCall)
nridge
zyounan updated this revision to Diff 504947.
zyounan marked 3 inline comments as done.
zyounan added a comment.
Address the comments. Revise parameters of `getSignature` to avoid passing CCR
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
nridge added a comment.
Thanks for the patch!
In the future we may want to add other cursor kinds for which
`ShouldPatchPlaceholder0` should be false, but I think this is a good start
which addresses the case from the bug.
Comment at:
zyounan added a comment.
Ping :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145319/new/
https://reviews.llvm.org/D145319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
zyounan created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan added reviewers: nridge, kadircet.
zyounan published this revision for review.
zyounan added inline comments.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald
14 matches
Mail list logo
