[PATCH] D55250: [clangd] Enhance macro hover to see full definition

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle marked an inline comment as done.
malaperle added inline comments.


================
Comment at: clangd/ClangdLSPServer.cpp:816
+            // If the client supports Markdown, convert from plaintext here.
+            if (*H && HoverSupportsMarkdown) {
+              (*H)->contents.kind = MarkupKind::Markdown;
----------------
ilya-biryukov wrote:
> malaperle wrote:
> > I don't know if you meant plain text as non-language-specific markdown or no markdown at all. In VSCode, non-markdown for multiline macros looks bad because the newlines are ignored.
> Did not know about that, thanks for pointing it out.
> It does not ignore double newlines though (that's what we use in declarations). I suspect it treats plaintext as markdown, but can't be sure.
> 
> In any case, converting to a markdown code block here looks incredibly hacky and shaky.
> Could we use double-newlines for line breaks in our implementation instead?
> 
> This aligns with what we did before this patch for declarations.
> If that doesn't work, breaking the multi-line macros in VSCode looks ok, this really feels like a bug in VSCode.
> In any case, converting to a markdown code block here looks incredibly hacky and shaky.

I'm not sure why? ClangdLSPServer knows about client capabilities so it made sense to me to convert it there. Either way, it sounds like I should just remove "HoverSupportsMarkdown" altogether if we're not going to use Markdown.

> Could we use double-newlines for line breaks in our implementation instead?

It looks odd. When I double them in the hover contents, the lines are displayed as doubled and some extra backslashes are added on all non-empty lines except the first line. Single new lines and correct backslashes are displayed correctly when Markdown is used. We can just display multiline line macros as one line until we want to handle Markdown (which is how exactly?). 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250