ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdServer.h:186 void findHover(PathRef File, Position Pos, - Callback<llvm::Optional<Hover>> CB); + Callback<llvm::Optional<FormattedString>> CB); ---------------- kadircet wrote: > sammccall wrote: > > Not sure about switching from `Hover` to `FormattedString` as return type > > here: LSP hover struct contains a range field (what are the bounds of the > > hovered thing?) that we may want to populate that doesn't fit in > > FormattedString. > > I'd suggest the function in XRefs.h should return `FormattedString` (and > > later maybe `pair<FormattedString, Range>`), and the `ClangdServer` version > > should continue to provide the rendered `Hover`. > > > > (We'll eventually want ClangdServer to return some HoverInfo struct with > > structured information, as we want embedding clients to be able to render > > it other ways, and maybe use it to provide extension methods like YCM's > > `GetType`. But no need to touch that in this patch, we'll end up producing > > HoverInfo -> FormattedString -> LSP Hover, I guess) > I agree with Sam on the layering. I was also working in a patch that was > changing `ClangdServer`s output from `Hover` to `HoverInfo`(a structured > version of `Hover`). `ClangdServer` does not know whether to render the `FormattedString` into markdown or plaintext and I believe it should stay that way. Happy to return `HoverInfo`, though. I'll introduce one with `FormattedString` and `Range` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58547/new/ https://reviews.llvm.org/D58547 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
