emacs-lsp / lsp-ui

UI integrations for lsp-mode
https://emacs-lsp.github.io/lsp-ui
GNU General Public License v3.0
1.04k stars 139 forks source link

Horizontal rule in markdown forces max width popup #442

Closed alanz closed 4 years ago

alanz commented 4 years ago

If a hover response includes a markdown HR (* * *\n), then the width calculation for the hover window calculates it as the full width of the window. Example

[Trace - 03:56:39 ] Received response 'textDocument/hover - (28)' in 17ms.
Result: {
  "range": {
    "end": {
      "character": 9,
      "line": 13
    },
    "start": {
      "character": 6,
      "line": 13
    }
  },
  "contents": {
    "value": "\n```haskell\n_ :: ([String] -> String) -> [[String]] -> [String]\n```\n```haskell\n_ :: forall a b. (a -> b) -> [a] -> [b]\n```\n* * *\n\n```haskell\nmap :: forall a b. (a -> b) -> [a] -> [b]\n```\n\n*Defined in ‘GHC.Base’*\n\n*O(n)* .  `map`   `f xs`  is the list obtained by applying  `f`  to each element\n of  `xs` , i.e., \n```haskell\nmap f [x1, x2, ..., xn] == [f x1, f x2, ..., f xn]\nmap f [x1, x2, ...] == [f x1, f x2, ...]\n```\n \n```haskell\n>>> map (+1) [1, 2, 3]\n```\n\n",
    "kind": "markdown"
  }
}

results in hover-hr

I have narrowed the window for the screenshot, if I maximise it uses the full width.

alanz commented 4 years ago

And I looked through the lsp-ui code, have no idea where this is happening.

covercash2 commented 4 years ago

i went ahead and closed the other issue, https://github.com/emacs-lsp/lsp-ui/issues/449, as duplicate, but this is happening with the Rust docs as well.

yyoncho commented 4 years ago

I had that same issue with --- in markdown and thus we have this code in lsp--render-markdown:

(while (re-search-forward "^[-]+$" nil t)
      (replace-match ""))

Apparently br is causing the same problem. We may thing of general solution.

bryandmc commented 4 years ago

Can we add this to that replacement as well? Or is that not a viable solution? Any hacks that would help in the short term..?

alanz commented 4 years ago

@bryandmc @covercash2 I am updating the hacky regex, to include what haskell uses, ^\* \* \*$.

Is there anything else that should be added? Should it be a substitution instead?

yyoncho commented 4 years ago

I guess the other option is setting markdown-hr-display-char to nil. I guess this will prevent rendering of the hrs

yyoncho commented 4 years ago

The complete solution will be to do a PR against markdown-mode to allow setting width in markdown-fontify-hrs .

alanz commented 4 years ago

I have proposed an expanded list of "bad" elements in https://github.com/emacs-lsp/lsp-mode/pull/1750.

And learned about rx and re-builder in the process :)

bryandmc commented 4 years ago

BTW this doesn't seem to have fixed it.. At least on my side. It looks like the horizontal rule is still there.

yyoncho commented 4 years ago

@bryandmc can you show the log + screenshot so we can find out what is causing the issue?

alanz commented 4 years ago

I think we are going to have to add case-by-case exceptions, based on real world reports.

See how many things map to <hr /> in https://github.github.com/gfm/

yyoncho commented 4 years ago

Another hacky solution will be to flet window-body-width to function returning reasonable for us width.

ghost commented 4 years ago

I'm on Emacs-28 on my usage with 3 languages Python,Go & Rust I can see the problem. This issue can be kept open until it solves for most of users.

bryandmc commented 4 years ago

@yyoncho What log are you looking for specifically (what log?)? I'm only a novice emacs+elisper but I'm a professional programmer so I can probably handle relatively short directions if you got them..

Here's a screenshot of what it looks like: image

ghost commented 4 years ago

is this fixed

bryandmc commented 4 years ago

It is for additional cases. Unfortunately, there are apparently a lot of ways to specify the <hr/> in markdown, and I think we've covered most of them. This is definitely fixed for rust w/ rust-analyzer b/c that's what I use, and used to verify the fix. If you still encounter it with the newest builds, though, definitely mention it.

tko commented 2 years ago

I suspect this may have reappeared. For me with spacemacs defaults and rust-analyzer the documentation popup is covering the whole screen (maximized emacs window on macos)

Screenshot 2021-09-25 at 11 34 00