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

Horizontal rule in markdown forces max width popup #442

Closed
alanz opened this issue May 10, 2020 · 17 comments · Fixed by emacs-lsp/lsp-mode#1798
Closed

Horizontal rule in markdown forces max width popup #442

alanz opened this issue May 10, 2020 · 17 comments · Fixed by emacs-lsp/lsp-mode#1798

Comments

@alanz
Copy link
Contributor

alanz commented May 10, 2020

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
Copy link
Contributor Author

alanz commented May 10, 2020

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

@covercash2
Copy link

covercash2 commented Jun 4, 2020

i went ahead and closed the other issue, #449, as duplicate, but this is happening with the Rust docs as well.

@yyoncho
Copy link
Member

yyoncho commented Jun 5, 2020

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
Copy link

bryandmc commented Jun 6, 2020

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
Copy link
Contributor Author

alanz commented Jun 6, 2020

@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
Copy link
Member

yyoncho commented Jun 6, 2020

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

@yyoncho
Copy link
Member

yyoncho commented Jun 6, 2020

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

@alanz
Copy link
Contributor Author

alanz commented Jun 6, 2020

I have proposed an expanded list of "bad" elements in emacs-lsp/lsp-mode#1750.

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

@yyoncho yyoncho closed this as completed Jun 6, 2020
@bryandmc
Copy link

bryandmc commented Jun 8, 2020

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

@yyoncho
Copy link
Member

yyoncho commented Jun 8, 2020

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

@alanz
Copy link
Contributor Author

alanz commented Jun 8, 2020

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
Copy link
Member

yyoncho commented Jun 8, 2020

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

@ghost
Copy link

ghost commented Jun 10, 2020

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
Copy link

@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
Copy link

ghost commented Jun 17, 2020

is this fixed

@bryandmc
Copy link

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
Copy link

tko commented Sep 25, 2021

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

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

Successfully merging a pull request may close this issue.

5 participants