dgoldman marked 2 inline comments as done.
dgoldman added inline comments.

================
Comment at: clang-tools-extra/clangd/AST.cpp:224
 
+  if (const auto *C = dyn_cast<ObjCContainerDecl>(&ND))
+    return printObjCContainer(*C);
----------------
sammccall wrote:
> I'm not sure this fits with the contract of printName, which really is to 
> print the name of the symbol rather than describe it.
> 
> Examples:
>  - the title of a hovercard for a method will be "instance-method `-[foo]`" 
> instead of "instance-method `foo`", which is inconsistent with c/c++
>  - the title of a hovercard for a category will be "extension 
> `PotentiallyLongClassName(CatName)`" instead of `extension ClassName`
>  - the documentSymbol response will include the extra details in the `name` 
> field rather than the `detail` field
> 
> We don't actually populate `detail` in documentSymbol, so maybe we do need a 
> separate function that gives that level of detail.
> Though I wonder whether it should mosty just print the decl (e.g. `class 
> X{};`, `@implementation PotentiallyLongClassName(CatName)` etc. WDYT?
> 
> See also https://github.com/clangd/clangd/issues/520 
> https://github.com/clangd/clangd/issues/601 though both appear to have 
> stalled.
Maybe instead this should be split somehow into a separate specialization for 
Document Symbols?

e.g.

Hovercards:
`instance-method foo`
`extension (anonymous)` (current) or `class-extension BTNButtonModel` or 
`BTNButtonModel()`

Document Symbols:
`-method`
`ClassName(Ext)`

Since I think for the case here we would prefer to keep them different. Maybe 
if the LSP spec differentiated between instance methods and class/static 
methods `-method` that wouldn't be necessary? I guess I could revert the Method 
change here and just keep a special case for categories/class extensions?

The `detail` work seems worth doing too, but I think we would similarly just 
put the type information in there (for methods), for categories/class 
extensions I feel like it would be odd since it would at the very least repeat 
the name



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96612/new/

https://reviews.llvm.org/D96612

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to