Page MenuHomePhabricator

[clangd] Parse `foo` in documentation comments and render as code.
ClosedPublic

Authored by sammccall on Apr 4 2020, 12:12 AM.

Diff Detail

Event Timeline

sammccall created this revision.Apr 4 2020, 12:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2020, 12:12 AM

Some saturday morning procrastination.

But I'm wondering how good an idea this is for plaintext: "Returns 'foo' on failure" is better than "Returns foo on failure", right? (With backticks instead of single quotes, phab is eating my formatting)
Maybe we should have an Emphasis flag or something on plaintext to preserve the quotes? (We don't want to include them e.g. in the return type chunks)

Also, realized our model for whitespace around paragraph chunks isn't ideal after all :-(

kadircet accepted this revision.Apr 4 2020, 2:44 AM

LGTM, thanks!

Regarding spaces between code and text chunks, are you suggesting we should print:

Tests primality of`p`

if so, i do believe having a space before the backtick vs not having it is two totally different rendering decisions. And I think the former is more common, why do you think we should not put that space?
I suppose we can modify the spacing logic for markdown to only put a separator between same chunk types, but I don't think that'll look any better especially in plugins like coc.nvim.

clang-tools-extra/clangd/Hover.cpp
873

nit: s/Input/Line/

This revision is now accepted and ready to land.Apr 4 2020, 2:44 AM

But I'm wondering how good an idea this is for plaintext: "Returns 'foo' on failure" is better than "Returns foo on failure", right? (With backticks instead of single quotes, phab is eating my formatting)

ah sorry missed that one.

That's actually a good point, I didn't dwell on it too much because I don't think having backticks in plaintext is that useful. But thinking about it again some people might find it useful especially in long
documentations to figure out which parts of it to focus on. So this might be a regression for them, it would be nice to have some feedback mechanism for driving such decisions. So far it has been just
the reviewer and the author of the patch :D

So having a raw paragraph chunk(in addition to plaintext and inline code). might be a middle ground here. Plaintext renderers will keep showing the documentation as it is written in the source code,
including backticks, whereas markdown renderers would display a more rich text. WDYT?

LGTM, thanks!

Regarding spaces between code and text chunks, are you suggesting we should print:

Tests primality of`p`

No, I'm complaining about the space before the period in

Tests primality of `p` .

and the plaintext rendering too

Tests primality of p .

So having a raw paragraph chunk(in addition to plaintext and inline code). might be a middle ground here. Plaintext renderers will keep showing the documentation as it is written in the source code, including backticks, whereas markdown renderers would display a more rich text. WDYT?

Two main objections to the idea of "raw":

  • we're going to emit arbitrary, potentially malformed markdown into the markdown stream, which can have arbitrary effects/glitches. I'd rather the emitter always emits valid markup and thus can't lose track of the context.
  • this assumes the input is markdown. I want \c foo to also render as code-font foo in markdown and as backtick-foo in plaintext. So what do we do there, generate markdown as a string and then emit it as a raw chunk? What a mess.

I really do think what we want is a chunk with semantics "emphasized code" that renders as a code span in markdown and as backtick-delimited text in plaintext. Thus the proposal to put an emphasis bit on the code chunk. WDYT?

No, I'm complaining about the space before the period in

Tests primality of `p` .

and the plaintext rendering too

Tests primality of p .

Ah I see, yes this looks annoying, and it is only because the next block starts with a punctuation :/

Two main objections to the idea of "raw":

  • we're going to emit arbitrary, potentially malformed markdown into the markdown stream, which can have arbitrary effects/glitches. I'd rather the emitter always emits valid markup and thus can't lose track of the context.
  • this assumes the input is markdown. I want \c foo to also render as code-font foo in markdown and as backtick-foo in plaintext. So what do we do there, generate markdown as a string and then emit it as a raw chunk? What a mess.

    I really do think what we want is a chunk with semantics "emphasized code" that renders as a code span in markdown and as backtick-delimited text in plaintext. Thus the proposal to put an emphasis bit on the code chunk. WDYT?

Sorry I wasn't clear on my comment. I was also suggesting putting some markdown generated by us while parsing the documentation into this raw field, which would be rendered as-is. In case we would like to perform this for other markers like *.
So going with an emphasis-only solution is also OK.

sammccall marked an inline comment as done.Apr 29 2020, 3:22 PM

Discussed offline, will follow up with fix for spaces and preserving backticks in text.

This revision was automatically updated to reflect the committed changes.