Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documents displayed in hover do not retain line breaks #95

Open
lh123 opened this issue Jul 13, 2019 · 9 comments
Open

Documents displayed in hover do not retain line breaks #95

lh123 opened this issue Jul 13, 2019 · 9 comments
Assignees

Comments

@lh123
Copy link

@lh123 lh123 commented Jul 13, 2019

image

@smhc
Copy link

@smhc smhc commented Jul 14, 2019

Using coc.nvim with neovim I have a slightly different issue - there is no space between "Declared in" and "global namespace".

space

Possibly from this line of code?: https://github.com/llvm-mirror/clang-tools-extra/blob/480d1d002ab582d82d996ea0909ab07e068d2fbb/clangd/XRefs.cpp#L1276

@ilya-biryukov
Copy link

@ilya-biryukov ilya-biryukov commented Aug 12, 2019

Thanks for reporting this! Fixing the multi-line issue should be easy enough in our code that renders the markdown strings. I'll take a look.

@smhc, for the coc.nvim issue - it's true that we probably don't add a space between the text and the inline block, but most markdown renderers seem to add it anyway.
Does markdown specify whether the space should be added when rendering? If yes, should this be fixed in coc.nvim?

@ilya-biryukov ilya-biryukov self-assigned this Aug 12, 2019
@ilya-biryukov
Copy link

@ilya-biryukov ilya-biryukov commented Aug 12, 2019

Looking at how GitHub renders the corresponding markdown block, we should probably add a space too:
Declared inglobal namespace
vs
Declared in global namespace

The space does make a difference and the version with a space looks nicer.

@ilya-biryukov
Copy link

@ilya-biryukov ilya-biryukov commented Aug 12, 2019

A fix for the missing space in coc.nvim is under review: https://reviews.llvm.org/D66086

@ilya-biryukov
Copy link

@ilya-biryukov ilya-biryukov commented Aug 12, 2019

Preserving line breaks: https://reviews.llvm.org/D66087

@sam-mccall
Copy link
Member

@sam-mccall sam-mccall commented Aug 12, 2019

Regarding newlines: It's not clear that preserving them is an improvement overall.

If the code looks like this:

// This is a comment that describes how a class works and
// rambles a little bit, spanning two lines by the end.

then preserving line breaks in the markdown will often result in rendering like this:

This is a comment that describes how
a class works and
rambles a little bit, spanning two
lines by the end

(which probably looks familiar to anyone who deals with mailing lists a lot...)

Comments tend to be a mish-mash of paragraphs, code examples, bulleted lists, and other structure (like doxygen-ish comments). My guess is that paragraphs are generally more common than structure, and merging structure does more damage than failing to merge paragraphs. i.e. simply preserving all breaks will make a few cases much better and most cases slightly worse. I'm not sure that's actually an improvement.

We can improve on this by using heuristics to understand what's likely, such as:

  • a break after a "short" line is probably a hard one
  • a break after a ., or before an uppercase letter, is likely hard
  • a change in indentation indicates some structure
  • double-line-breaks are always hard

Patches welcome-ish - I think we actually need some design here first.

Ilya points out we're in no-man's-land at the moment: we don't strip the newlines when rendering as plaintext, but we don't preserve them in markdown. Though there's some virtue here too: plaintext clients are going to tend towards using a code font, and preserving newlines is relatively more useful when the font is monospace (as it does a better job of preserving structure).

(The missing space is uncontroversial I think)

dtzWill pushed a commit to llvm-mirror/clang-tools-extra that referenced this issue Aug 12, 2019
Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in clangd/clangd#95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66086

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@368581 91177308-0d34-0410-b5e6-96231b3b80d8
llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Aug 12, 2019
Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in clangd/clangd#95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66086

llvm-svn: 368581
chapuni pushed a commit to llvm-project/llvm-project-20170507 that referenced this issue Aug 12, 2019
Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in clangd/clangd#95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66086
chapuni pushed a commit to llvm-project/llvm-project-submodule that referenced this issue Aug 12, 2019
Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in clangd/clangd#95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66086
llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Aug 13, 2019
------------------------------------------------------------------------
r368581 | ibiryukov | 2019-08-12 16:35:30 +0200 (Mon, 12 Aug 2019) | 18 lines

[clangd] Separate chunks with a space when rendering markdown

Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in clangd/clangd#95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66086
------------------------------------------------------------------------

llvm-svn: 368670
dtzWill pushed a commit to llvm-mirror/clang-tools-extra that referenced this issue Aug 13, 2019
------------------------------------------------------------------------
r368581 | ibiryukov | 2019-08-12 16:35:30 +0200 (Mon, 12 Aug 2019) | 18 lines

[clangd] Separate chunks with a space when rendering markdown

Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in clangd/clangd#95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66086
------------------------------------------------------------------------


git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/branches/release_90@368670 91177308-0d34-0410-b5e6-96231b3b80d8
@kadircet
Copy link
Member

@kadircet kadircet commented Jan 15, 2020

note that new rendering behavior strips newlines in plaintext mode too.

@lolleko
Copy link

@lolleko lolleko commented Mar 3, 2020

Screenshot 2020-03-03 at 21 24 07

I agree current rendering behaviour looks weird. No line breaks which breaks markdown rendering (missing bullets)

@sam-mccall
Copy link
Member

@sam-mccall sam-mccall commented Mar 3, 2020

We talked about heuristics to detect line breaks in the past, telling the difference between a soft-wrap and a hard-wrap isn't trivial but usually possible (punctuation, is the line shorter than the longest line, etc).

A maybe-simpler alternative/fallback: detect when the documentation has "interesting formatting" and render the whole thing as a verbatim code block.

Regarding the example, detecting the comment is markdown and preserving it is interesting but also challenging. I don't know that markdown comments are all that common but I kind of wish they were, and it's partly from lack of tooling. Maybe we should be the change we want to see...

sam-mccall added a commit to llvm/llvm-project that referenced this issue Mar 24, 2020
`parseDocumentation` retains hard line breaks and removes soft line
breaks inside documentation comments.
Wether a line break is hard or soft is determined by the following rules
(some of which have been discussed in
clangd/clangd#95):

Line breaks that are preceded by a punctuation are retained
Line breaks that are followed by "interesting characters" (e.g. Markdown
syntax, doxygen commands) are retained
All other line breaks are removed

Related issue: clangd/clangd#95

Differential Revision: https://reviews.llvm.org/D76094
arichardson added a commit to arichardson/llvm-project that referenced this issue Apr 2, 2020
`parseDocumentation` retains hard line breaks and removes soft line
breaks inside documentation comments.
Wether a line break is hard or soft is determined by the following rules
(some of which have been discussed in
clangd/clangd#95):

Line breaks that are preceded by a punctuation are retained
Line breaks that are followed by "interesting characters" (e.g. Markdown
syntax, doxygen commands) are retained
All other line breaks are removed

Related issue: clangd/clangd#95

Differential Revision: https://reviews.llvm.org/D76094
shrouxm pushed a commit to sourcegraph/lsif-clang that referenced this issue Jul 12, 2020
Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in clangd/clangd#95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66086

llvm-svn: 368581
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants