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

Multiline markdown documentation paragraphs get bad linebreaks in documentation hover #3720

Closed
oblitum opened this issue Mar 22, 2022 · 19 comments

Comments

@oblitum
Copy link
Member

oblitum commented Mar 22, 2022

Result from CocInfo

## versions

vim version: NVIM v0.7.0-dev+1275-g00effff56
node version: v17.7.2
coc.nvim version: 0.0.80-93ed9c68 2022-03-20 00:49:33 +0800
coc.nvim directory: /home/francisco/.local/share/nvim/site/pack/packer/opt/coc.nvim
term: tmux
platform: linux

## Log of coc.nvim

2022-03-21T21:23:49.346 INFO (pid:1054917) [services] - registered service "languageserver.graphql"
2022-03-21T21:23:49.348 INFO (pid:1054917) [services] - registered service "languageserver.haskell"
2022-03-21T21:23:49.349 INFO (pid:1054917) [services] - registered service "languageserver.rescript"
2022-03-21T21:23:49.349 INFO (pid:1054917) [services] - registered service "languageserver.purescript"
2022-03-21T21:23:49.349 INFO (pid:1054917) [services] - registered service "languageserver.zig"
2022-03-21T21:23:49.385 INFO (pid:1054917) [services] - registered service "diagnostic-languageserver"
2022-03-21T21:23:49.460 INFO (pid:1054917) [coc-git] - Looking for git in: git
2022-03-21T21:23:50.444 INFO (pid:1054917) [plugin] - coc.nvim initialized with node: v17.7.2 after 1373ms
2022-03-21T21:23:50.466 INFO (pid:1054917) [attach] - receive notification: diagnosticRefresh []
2022-03-21T21:23:50.832 INFO (pid:1054917) [services] - registered service "rust-analyzer"
2022-03-21T21:23:50.833 INFO (pid:1054917) [services] - Rust Analyzer Language Server state change: stopped => starting
2022-03-21T21:23:50.859 INFO (pid:1054917) [language-client-index] - Language server "rust-analyzer" started with 1055046
2022-03-21T21:23:50.906 INFO (pid:1054917) [services] - Rust Analyzer Language Server state change: starting => running
2022-03-21T21:23:50.917 INFO (pid:1054917) [services] - service rust-analyzer started
2022-03-21T21:23:50.927 ERROR (pid:1054917) [core-watchman] - Error on watchman create: watchman::CommandValidationError: failed to validate command: resolve_projpath:  None of the files listed in global config root_files are present in path `/home/francisco/Projects/bevy` or any of its parent directories.  root_files is defined by the `/etc/watchman.json.default` config file and includes `.watchmanconfig`.  One or more of these files must be present in order to allow a watch. Try pulling and checking out a newer version of the project?
2022-03-21T21:24:19.091 INFO (pid:1054917) [attach] - receive notification: showInfo []

Describe the bug

A clear and concise description of what the bug is.

The linebreaks from documentation are transported to the hover docs, the hover window can often be narrower than the line length in documentation, so this creates spurious linebreaks in addition to the ones that exists due to window size.

Reproduce the bug

We will close your issue when you don't provide minimal vimrc and we can't
reproduce it

Screenshots (optional)

If applicable, add screenshots to help explain your problem.

img-2022-03-21-210527

Notice how in this example the docs strangely breaks at "the" and at "direction", it's because the documentation comment is:

    /// Updates and returns this [`Transform`] by rotating it so that its unit vector in the
    /// local z direction is toward `target` and its unit vector in the local y direction
    /// is toward `up`.
@yaegassy
Copy link
Contributor

yaegassy commented Mar 22, 2022

This is the result of textDocument/hover, but it appears to be normal behavior since it is carriage-returned at the \n location.

[Trace - 12:57:50 PM] Received response 'textDocument/hover - (19)' in 5ms.
Result: {
    "contents": {
        "kind": "markdown",
        "value": "\n```rust\nbevy_transform::components::transform::Transform\n```\n\n```rust\npub fn looking_at(self, target: Vec3, up: Vec3) -> Self\n```\n\n---\n\nUpdates and returns this [`Transform`](https://docs.rs/bevy_transform/0.6.0/bevy_transform/components/transform/struct.Transform.html) by rotating it so that its unit vector in the\nlocal z direction is toward `target` and its unit vector in the local y direction\nis toward `up`."
    },
    "range": {
        "start": {
            "line": 3,
            "character": 37
        },
        "end": {
            "line": 3,
            "character": 47
        }
    }
}

@chemzqm
Copy link
Member

chemzqm commented Mar 22, 2022

Configure maxWidth in your "hover.floatConfig" configuration.

@chemzqm chemzqm closed this as completed Mar 22, 2022
@oblitum
Copy link
Member Author

oblitum commented Mar 22, 2022

This is the result of textDocument/hover, but it appears to be normal behavior since it is carriage-returned at the \n location.

Indeed, to be honest, I think this is a duplicate, I think I've gone through this before and was against applying "formatting" to Result.contents.value.

Configure maxWidth in your "hover.floatConfig" configuration.

It's an option, but I wished better flowed text in smaller-by-default floats. Large floats get much in the way while coding.

@oblitum
Copy link
Member Author

oblitum commented Mar 22, 2022

I see this could potentially add a bad side effect for say, docs with box drawing contents, when they would display fine, in a large float, but would get broken if linebreaks are removed. Well, guess there's not a good way out without a side-effect, so better be as it is.

@chemzqm
Copy link
Member

chemzqm commented Mar 22, 2022

The language server should be responsible for format the document.

@oblitum
Copy link
Member Author

oblitum commented Mar 22, 2022

Right, agree, it should be a fix in rust-analyzer.

@oblitum
Copy link
Member Author

oblitum commented Mar 22, 2022

Please consider reopening, see rust-lang/rust-analyzer#11787, markdown rendering should not present newlines (unless the line ends with 2 spaces or more). If comments are treated as markdown, I think that should be respected.

@oblitum oblitum changed the title Multiline documentation paragraphs get bad linebreaks in documentation hover Multiline markdown documentation paragraphs get bad linebreaks in documentation hover Mar 22, 2022
@chemzqm
Copy link
Member

chemzqm commented Mar 24, 2022

Not supported by marked, you can try to hack marked if you want the lines to get merged, can't be done with custome render of marked.

@oblitum
Copy link
Member Author

oblitum commented Mar 24, 2022

Ok, thanks. Well, then maybe they need this fixed upstream, I'll check out what's up.

@chemzqm
Copy link
Member

chemzqm commented Mar 24, 2022

const {marked} = require('marked')

marked.setOptions({
  gfm: true,
  breaks: true
})

const html = marked.parse('foo\nbar')

results:

<p>foo<br>bar</p>

Same behavior on github.

@oblitum
Copy link
Member Author

oblitum commented Mar 24, 2022

Same behavior on github.

Do you mean github comments? Indeed that's the case, but I suspect they're not following original/standard markdown guidelines on purpose because people in general wish a newline when they insert one when commenting, instead of having to learn that they have to end line with 2 spaces or more for hard line break.

See this issue for example:

Possibly coc.nvim should adopt breaks: false to have standard markdown soft and hard line breaks? I think this is the best behavior for Markdown in documentation context (where people does not expect line breaks on a new line).

@chemzqm
Copy link
Member

chemzqm commented Mar 24, 2022

That option possible cause unnecessary new lines as well.

@oblitum
Copy link
Member Author

oblitum commented Mar 24, 2022

Yeah, I'm gonna have to check it on my dev machine asap. Their docs seem confusing indeed, because it says it should be false by default (supposedly normal markdown behavior, requiring two spaces), which was the case when I opened this issue. It's now explicitly true after late commits, which would generate line breaks on new lines alone, according to the linked issue. In any case, marked has to support original and CommonMark markdown somehow at this, because they support that, include tests for this, otherwise they would be supporting GFM as in GitHub comments solely.

@oblitum
Copy link
Member Author

oblitum commented Mar 24, 2022

Ok, just tried marked locally and it doesn't generate <br> when breaks: false, which is the default (I also checked it by simply not setting the option). Only when breaks: true it generates <br>. What happens is that a \n is left as-is when breaks: false, and when breaks: true, then the \n is consumed in the output, and transformed into a <br>. If we consider how HTML works, a <br> presents as a new line, but a \n will simply not show up, it only exists in the HTML source. So this is the confusion here, not sure how it should be handled, but I think it would be ideal to have rendering of newlines only in case of <br> output it seems (when breaks: false), and have \ns ignored, as it happens in HTML. Otherwise, there'll be a strange/inconsistent mix of behavior, where <br> output results in a line break just as a \n, the two cases would have no difference.

@chemzqm
Copy link
Member

chemzqm commented Mar 24, 2022

breaks: true works better with unnecessary lines get removed, you can't simply replace \n with empty space to fix the issue.
IMO, the line break makes the text easier to read on most cases, configure "maxWidth" for you window should help to avoid your issue.

@oblitum
Copy link
Member Author

oblitum commented Mar 25, 2022

breaks: true works better with unnecessary lines get removed

Okay, I'm not sure which ones are removed though. At the moment, either ending a line with two spaces or not (which results either in <br> or \n, respectively) will generate line breaks in documentation float, so I don't see any case where they get removed, in any case it simply results in line breaks. Nevermind, breaks: true is supposed to give <br>s all the way, ending with spaces makes no difference.

IMO, the line break makes the text easier to read on most cases, configure "maxWidth" for you window should help to avoid your issue.

I realize what you mean, it's some kind of tension between honoring the usual Markdown specs (GFM-comments is unusual) vs preserving the original text layout as in the source code comments, even though by that you can break usual Markdown rendering rules. If there was a way to render it properly, I'd prefer to have Markdown honored, but even then, it seems you would prefer to preserve newlines from comment sources, for them to display verbatim in a large window.

@chuck-sys
Copy link

Hello, just joining in. I was wondering if we could make the breaks: true or breaks: false a preference, which by default goes to breaks: true.

Something like this (untested)? https://github.com/cheukyin699/coc.nvim/commit/1e1c057696049fd694ab197c69ff1803e243bb78

@chemzqm
Copy link
Member

chemzqm commented Nov 20, 2022

Add configuration coc.preferences.enableGFMBreaksInMarkdownDocument

@chuck-sys
Copy link

After some testing, it appears that simply setting breaks: false doesn't give the intended effect of removing soft line breaks because a soft line break and a hard line break are both displayed with \n, which the terminal doesn't distinguish between. Ergo a solution would be to plug into the Renderer itself.

See https://github.com/cheukyin699/coc.nvim/commit/728172e5572193d061fac1f98209867d3b97ebd1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants