[PATCH] D79142: [clangd] Render doc-comment code spans with `backticks` in plaintext mode
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 30 04:31:58 PDT 2020
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
LGTM thanks!
================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:365
+ llvm::StringRef Marker = "";
+ if (C.Preserve && C.Kind == Chunk::InlineCode)
+ Marker = "`";
----------------
sammccall wrote:
> kadircet wrote:
> > should we rather use `renderInlineBlock` here ? because in presence of backticks inside the C.Contents it might become confusing e.g:
> >
> > ```
> > this is`foo(`x`)`
> > ```
> >
> > this would become:
> > ```
> > this is `foo(``x``)`
> > ```
> >
> > and instead of keeping a marker maybe just:
> >
> > ```
> > if (Preserve && ..)
> > OS << Sep << "`" << C.Contents << "`";
> > else
> > OS << Sep << C.Contents;
> > ```
> > rather use renderInlineBlock
>
> I don't think humans deal with escaping better than with ambiguity - those two examples seem equally confusing to me :-\
>
> I'd suggest better is to pick another marker that's unused in the chunk:
> ```
> this is 'foo(`x`)'
> ```
>
> Happy to do that in this patch if you like it.
> instead of keeping a marker maybe just:
>
> > instead of keeping a marker maybe just
>
> Happy to do that for now if you like, but I don't think it's much clearer. I factored it this way because I think the marker is likely to have more options in future (bold spans, avoiding conflicts) and wanted to hint at that.
> I don't think humans deal with escaping better than with ambiguity - those two examples seem equally confusing to me :-\
> I'd suggest better is to pick another marker that's unused in the chunk:
> this is 'foo(`x`)'
> Happy to do that in this patch if you like it.
Yeah that looks like a better approach, feel free to leave a fixme as well. As I believe this should be a rather rare occurrence and we can take care of it once it becomes a real issue or someone has time.
> > instead of keeping a marker maybe just
> Happy to do that for now if you like, but I don't think it's much clearer. I factored it this way because I think the marker is likely to have more options in future (bold spans, avoiding conflicts) and wanted to hint at that.
OK makes sense let's keep it that way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79142/new/
https://reviews.llvm.org/D79142
More information about the cfe-commits
mailing list