lsp-eldoc-enable-hover hides flymake errors

stevemolitor commented 3 years ago

Thank you for the bug report

[X] I am using the latest version of lsp-mode related packages.
[X] I checked FAQ and Troubleshooting sections
[x] You may also try reproduce the issue using clean environment using the following command
```
emacs -q -l lsp-start-plain.el
```
where lsp-start-plain.el can be downloaded here. Alternatively, it will be great if you can reproduce the issue using lsp-docker which provides the minimal configurations for lsp-mode and ships with most of the language servers.

Bug description

I am using lsp with the default emacs flymake, using emacs 28.

With lsp-eldoc-enable-hover turned on, I can't see flymake errors in the echo area. The error at point message flashes by briefly, but then is overridden by the symbol info. When there is an error I would rather see that than the symbol info. Showing both would be fine too.

I prefer seeing both symbol and error info in the echo area, not in lsp-ui overlays as I find that distracting.

Steps to reproduce

(use-package lsp-mode
   :custom (lsp-eldoc-enable-hover t))

Make sure you do not have company-mode installed. In say typescript, type this in an lsp buffer:

const x: string = 'hi';

You should see a red squigly under the x. Put your cursor on the 'x'.

I want to see the error message:

'x' is declared but its value is never read.

But I see the symbol type:

const x: string

Showing both would be great too, but I need to at least see the error, when there is an error, and see the symbol info when no error.

Here's a pic of what I see:

Expected behavior

I want to see the error message:

'x' is declared but its value is never read.

For example, here's what I see with lsp-eldoc-enable-hover off - I'd like to see this with it on, when there is an error at point:

Or, showing the symbol and the error message would work fine too:

'x' is declared but its value is never read.
const x: string

But it's important to at least show the error at point if there is one.

Which Language Server did you use?

ts-ls

OS

MacOS

Error callstack

No response

Anything else?

I tried setting eldoc-documentation-strategy to eldoc-documentation-compose but that didn't work.

I also tried messing with eldoc-documentation-functions in elisp, adding both the flymake and the lsp doc functions to the list, but it seems they take different number of args (one for flymake-eldoc-function, none for lsp-eldoc-function), and eldoc wanted the documentation function to take one arg.

Similarly I tried addings flymake-eldoc-function to lsp-eldoc-hook but same problem in reverse - lsp-mode didn't like that flymake-eldoc-function took one arg. I got a little lost there TBH - it seems flymake and lsp-mode are setting up eldoc in different (incompatible?) ways.

stevemolitor commented 3 years ago

So I've implemented a super hacky fix for myself. I just re-wrote lsp-hover to do what I want: if there's a flymake error at point show that, else show symbol info (if available):

  (defun lsp-hover ()
    "Display hover info (based on `textDocument/signatureHelp')."
    (if (and lsp--hover-saved-bounds
             (lsp--point-in-bounds-p lsp--hover-saved-bounds))
        (lsp--eldoc-message lsp--eldoc-saved-message)
      (setq lsp--hover-saved-bounds nil
            lsp--eldoc-saved-message nil)
      (if (looking-at "[[:space:]\n]")
          (lsp--eldoc-message nil)
        (when (and lsp-eldoc-enable-hover (lsp--capability :hoverProvider))
          ;; get flymake diagnostics at point
          (let ((flymake-diags  (flymake-diagnostics (point))))
            ;; if there's flymake diagnostics at point, show them:
            (if flymake-diags
                (lsp--eldoc-message (mapconcat #'flymake-diagnostic-text flymake-diags "\n"))
              ;; else show symbol info if available
              (lsp-request-async
               "textDocument/hover"
               (lsp--text-document-position-params)
               (-lambda ((hover &as &Hover? :range? :contents))
                 (when hover
                   (when range?
                     (setq lsp--hover-saved-bounds (lsp--range-to-region range?)))
                   (lsp--eldoc-message (and contents
                                            (lsp--render-on-hover-content
                                             contents
                                             lsp-eldoc-render-all)))))
               :error-handler #'ignore
               :mode 'tick
               :cancel-token :eldoc-hover)
              ))))))

In my case I've set up flymake to only show errors, so this works for me. In general though maybe you wouldn't want flymake warning or info messages to hide symbol info, so maybe showing everything would be better.

This is not a good solution naturally, but perhaps it might be of some help coming up with a better fix.

stevemolitor commented 3 years ago

I tried installing and switching to flycheck - same issue, similar workaround.

yyoncho commented 3 years ago

On my side, it seems to work fine with flycheck(emacs 27.2/spacemacs) - it shows the error. Have you considered using a different frontend for flymake/flycheck. E. g. flycheck-posframe/flycheck-pos-tip?

stevemolitor commented 3 years ago

On my side, it seems to work fine with flycheck(emacs 27.2/spacemacs) - it shows the error. Have you considered using a different frontend for flymake/flycheck. E. g. flycheck-posframe/flycheck-pos-tip?

@yyoncho thanks for trying, and thanks for the fast response.

flymake-posframe worked fine; I decided I preferred eldoc. The issue only occurs when I turn on lsp-eldoc-enable-hover.

The problem is only with eldoc. Did you try with (lsp-eldoc-enable-hover t)? If so I'll see if I can come up with a cleanroom reproduction of the issue.

Thanks again.

yyoncho commented 3 years ago

(lsp-eldoc-enable-hover t)

That is the default. I tested with lsp-start-plain.el as described in the bug template and it works just fine (I had to disable lsp-ui).

stevemolitor commented 3 years ago

(lsp-eldoc-enable-hover t)

That is the default. I tested with lsp-start-plain.el as described in the bug template and it works just fine (I had to disable lsp-ui).

Ah that was it - if I disable lsp-ui the problem goes away. I still use lsp-ui on demand for things like lsp-ui-doc-show.

So it's some weird interaction with lsp-ui. If I turn off lsp-eldoc-enable-hover and leave lsp-ui on, it works. If I turn on lsp-eldoc-enable-hover and turn off lsp-ui, it also works. But with lsp-eldoc-enable-hover and lsp-ui both on, the issue occurs.

yyoncho commented 3 years ago

I moved the issue in lsp-ui. Keep it open if your preference is to use lsp-mode in combination with lsp-ui and this issue is problematic for you.

stevemolitor commented 3 years ago

So I may have spoken too soon: I'm still seeing some weirdness with lsp-ui turned off. When I put my cursor over an error I see the symbol info ("const x: string") briefly, and then it is quickly replaced with the error message. I didn't notice that initially.

Also, if navigate to the error via ie flycheck-next-error, the order of eldoc messages is reversed: first the error message is briefly displayed, and then the symbol info. But if I navigate to an error, I want to see the error message not other info.

I'll make an isolated example using lsp-start-plain.el in a day or two.

Thanks again for your help on this.

stevemolitor commented 3 years ago

@yyoncho Using lsp-start-plain.el can I install the eslint language server as described here?

I fired up emacs -q -l lsp-start-plain.el but I don't see the eslint server:

To summarize the issue, with lsp-ui disabled and lsp-eldoc-enable-hover on, I see both the symbol info and the error message in the echo area. Usually first the symbol info flashes by briefly and then I see error message, but sometimes the reverse for example when I do flycheck-next-error.

Thanks again.

yyoncho commented 3 years ago

@stevemolitor most likely eslint server is present.

stevemolitor commented 3 years ago

@yyoncho eslint was installed, thanks.

OK I've reproduced it using lsp-start-plain.el. Reproduction steps:

emacs -q -l lsp-start-plain.el
Eval (setq lsp-eldoc-enable-hover t)
Open a new buffer called foo.ts and paste this into it:

function f() {
    const x: string = 'hi';
}

Put your cursor over the x. You will first briefly see const x: string in the echo area, and then 'x' is declared but its value is never read. [6133]
Go to the top of the buffer, and run flycheck-next-error. You see the following sequence in the echo area:
- 'x' is declared but its value is never read. [6133] (briefly)
- You can run the command flycheck-next-error with ...
- 'x' is declared but its value is never read. [6133] (briefly)
- const x: string (this one sticks around)

(The 'You can run the command...' message is expected and will go away if you map it to a key.)

The main problem here in my mind is that when you navigate to the next / previous error, the eldoc message that sticks around is the symbol info message, hiding the error message. In general though the order of the messages feels a bit random and 'flickery'.

Here is a video demonstrating the problem, using emacs -q -l lsp-start-plain.el:

eldoc-error

I moved my cursor at the end there after flycheck-next-error to stop the video, but const x: string stays around forever in the echo area in that scenario, until the cursor is moved.

emacs-lsp / lsp-ui