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