sammccall added a comment.
Still LG
I forgot to mention - it would be nice to clang-format the definition, what do
you think?
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:592
+ )cpp",
+ {/*NamespaceScope=*/std::string("ns1::ns2"),
+ /*LocalScope=*/std::string(""),
----------------
kadircet wrote:
> sammccall wrote:
> > I think this should be "ns1::ns2::", as we use scope internally.
> > This means we can simply concatenate parts to form a qname.
> >
> > For the current rendering, it's easy to strip ::
> should it also be "::" for global namespace ? which would also result in
> prefixing any symbol in global namespace with "::" when printing.
We tend to use empty string for global namespace, and explicitly add it where
we need it.
Using :: for global causes as many problems as it solves (e.g. in C).
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:599
+ /*Definition=*/"void foo()",
+ /*Type=*/llvm::None,
+ /*ReturnType=*/std::string("void"),
----------------
kadircet wrote:
> sammccall wrote:
> > It would be nice to add `void()` or `void (&)()` or so if it's easy.
> > This is what `:YcmCompleter GetType` would show
> just put the type without any parameter names, but I am not sure whether
> users would want that. I believe people find current hover behavior a lot
> more useful then just showing type(which is done by libclang)
I don't think we should include it in the actual hover, but we can handle that
in rendering (if it's a Function, don't print the type).
It does seem odd not to include it in the structured API, but up to you.
I thought specifically the `:GetType` might still be useful, but maybe not.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61497/new/
https://reviews.llvm.org/D61497
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
