kadircet 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); ---------------- 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`). 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
