kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
thanks this LGTM just a couple nits.
my biggest concern is old endpoint will vanish all of a sudden and it might
create some confusion, especially for editors that turn on inlayhints without
an explicit user interaction.
but i suppose plugin maintainers that implement the extension shouldn't have a
hard time finding out the new endpoint and migrating to it. (and I don't think
we've got (m)any plugins implementing the extension today.) So all should be
fine.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:611
+ // FIXME: once inlayHints can disabled per-file in config, always advertise.
if (Opts.InlayHints)
----------------
s/can/can be
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:168
+ addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
+ InlayHintKind::ParameterHint, "", Name, ": ");
}
----------------
nit: parameter name comments for `""` and `": "` (same for other calls)
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:320
- void addInlayHint(SourceRange R, InlayHintKind Kind, llvm::StringRef Label) {
+ // We pass HintSide rather than SourceLocation because we want to ensure we
+ // it is in the same file as the common file range.
----------------
drop the last `we`
================
Comment at: clang-tools-extra/clangd/InlayHints.h:26
+/// Compute and return inlay hints for a file.
+///
+/// If RestrictRange is set, return only hints whose location is in that range.
----------------
nit: drop the empty line?
================
Comment at: clang-tools-extra/clangd/Protocol.h:1555
+ ///
+ /// For example, an parameter hint may be positioned before an argument.
+ Position position;
----------------
s/an parameter/a parameter/
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116699/new/
https://reviews.llvm.org/D116699
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
