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); ---------------- ilya-biryukov wrote: > 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` Returning `HoverInfo` now, with a `FormattedString` and a 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
