lsp-ui icon indicating copy to clipboard operation
lsp-ui copied to clipboard

Markdown rendering problems

Open andreyorst opened this issue 4 years ago • 14 comments

Hi. I've disabled the rendering of horizontal rulers in Markdown, by setting markdown-hr-display-char to nil, because it has some problems computing the width of those in the Company Box documentation popup. But I've noticed that changing these options did not had any effect on LSP-UI package. While LSP-UI doesn't have problems with calculating width, this introduces inconcistency with the look of documentation in company box and lsp ui popups:

image

(on the top LSP-UI doc popup, on the right company box doc popup)

After a bit of investigation I saw that LSP-UI does a lot of trickery to make documentation look differently from what it would look if rendered with gfm-view-mode or markdown-view-mode. Mainly, the horizontal rulers font size is changed to very small one, and they're rendered as white space, with tweaks to the face to make it look like a horizontal ruler.

I would like to stop this behavior without resorting to define-advice, like this one:

(define-advice lsp-ui-doc--handle-hr-lines (:override () aorst:lsp-ui-dont-handle-hr-lines)
  nil)

As a matter of fact, a lot of faces seem wrong with the DOOM theme suite:

image

For example, here the background color for the first two lines leaked into the third line. If I disable child frame usage in the settings, the docs renders is as follows:

image

Note that faces do not extend beyond the line end.

None of these errors occur when using markdown-view-mode with the buffers containing the same text. And as seen on the the first screenshot, company box also manages to render documentation correctly.

Additionally, the link face only applied to the tail of the link for some reason (and clicking on it does nothing).

andreyorst avatar May 14 '21 17:05 andreyorst

Hi, maybe @yyoncho knows about this implementation?

About the face difference, the first two lines are inside a code block so probably that's why it has different colors.

Off: why are you using company-box? Is there any feature that company doesn't have? Recently company added support for icons

ericdallo avatar May 14 '21 22:05 ericdallo

About the face difference, the first two lines are inside a code block so probably that's why it has different colors.

Correction: the difference is not the problem, it's code block face as you say. The face leaking into the next line is what bothers me:

image

It's a bit hard to see it, but line 1 has no face on first cell, and lines 2 and 3 have extra face on the first cell. Which doesn't happen in company-box documentation popup:

image

Off: why are you using company-box? Is there any feature that company doesn't have? Recently company added support for icons

I use company-box because it provides company in the childframe. As you can see here I've disabled all the nonsense, like the icons and scrollbars, but the fact that this is not an overlay is very important to me, because it's not restricted by current window.

Additionally the documentation popup company-box has is the best I've found of all frontends - it supports scrolling with mouse, and actually renders very nicely. Previously I've used company-posframe, which had some issues with documentation rendering, and childframe positioning. Company-quickhelp also has some problems with size calculation of the popup, and for some reason eats newlines in the docstring, concatenating most of the separate lines into single line. Company-box nails all of this.

Off: also, is it possible to turn on fringes in the documentation popup? Scrolling sideways has these ugly $ marks and I would like fringe arrow symbols instead

andreyorst avatar May 15 '21 06:05 andreyorst

I didn't know about those things, good to know.

Correction: the difference is not the problem, it's code block face as you say. The face leaking into the next line is what bothers me:

Yeah, that's seems really weird indeed, maybe @sebastiencs can help?

ericdallo avatar May 15 '21 14:05 ericdallo

Should have noted this earlier: I would like to render documentation popup as close to how markdown-view-mode renders as possible. This is how this issue should be summarized.

I've read the code of lsp-ui-doc.el, and there's this function:

(defvar lsp-ui-doc-render-function nil
  "Function called to format the documentation.
The function takes a string as parameter and should return a string.
If this variable is nil (the default), the documentation will be rendered
as markdown.")

But it seems it is never actually used? Right now I have to override internal functions with advises, which is not good in the long term, I guess. If this function were actually used, would it be sufficient to make rendering look like I want to?

andreyorst avatar May 15 '21 18:05 andreyorst

Yeah, it's not used, it used to be called here

ericdallo avatar May 15 '21 18:05 ericdallo

Hi @andreyorst, can you try this branch https://github.com/emacs-lsp/lsp-ui/pull/612 and set lsp-ui-doc-enhanced-markdown to nil ?

sebastiencs avatar May 15 '21 19:05 sebastiencs

@sebastiencs thanks! I can confirm that horizontal rulers and empty lines are no longer changed:

image

But the face leaking problem is still here:

image

andreyorst avatar May 15 '21 19:05 andreyorst

@andreyorst I think this is intended as it's the continuation of the code block from the above line
image

ericdallo avatar May 16 '21 15:05 ericdallo

I don't think so, as this doesn't happen in markdown buffer with the same contents, in company-quickhelp and company-box's doc popups.

And when disabling childframe usage for LSP-UI, the faces aren't extended at all

andreyorst avatar May 16 '21 15:05 andreyorst

Another example of face leaking into next lines:

image

The link face goes all the way to the end of the buffer scrollback. And for some reason documentation has the code block face, which I'm not sure is intended?

andreyorst avatar May 17 '21 07:05 andreyorst

@andreyorst that documentation code block should be fixed on latest release

ericdallo avatar May 17 '21 11:05 ericdallo

Thanks, though I thought that maybe this was done to add syntax highligting to forms and prevent indented text being highlighted as preformatted text

andreyorst avatar May 17 '21 16:05 andreyorst

@sebastiencs @ericdallo I have a related issue to textDocument/hover which I mentioned to @yyoncho on Discord using Emacs with python (pyright) and Scala (metals). There is an open issue over on lsp-pyright about this lack of formatting on doc strings currently open. Both are returning markdown but newlines are being stripped when displayed in the hover popup. The display is fine in a company popup documentation and an eldoc buffer. Two problems are occurring - the horizontal rule isn't being displayed correctly and newlines are stripped so that the formatting is incorrect. The fix above doesn't clear the newlines issue, however I narrowed both problems down to the use of fill-region in lsp-ui-doc--render-buffer.

Commenting out the following lines resolve both the formatting of HR and newlines are correctly rendered.

(let ((fill-column (- lsp-ui-doc-max-width 5)))
        (fill-region (point-min) (point-max)))

Here's what I'm currently seeing in lsp with pyright and a custom simple doc string: doc-hover-bug

Commenting our the fill-region lines above leads to the following: fill-region-removed

Changing the width of lsp-ui-doc-max-width has no effect - i.e. you can make this 1000 wide and it doesn't solve the problem.

dsyzling avatar Jul 19 '21 09:07 dsyzling

#643 has resolved by issue with the fill-region behaviour above.

dsyzling avatar Jul 21 '21 08:07 dsyzling