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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to