search

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

[lsp-ui-sideline] Don't re-render hovers/diagnostics on same line #490

Closed sebastiencs closed 4 years ago

sebastiencs commented 4 years ago

Render hover informations and diagnostics only when changing lines. Only actions are updated when moving point on the same line

Fixes #434

yyoncho commented 4 years ago

and diagnostics

I haven't tested the PR, but the diagnostics might be updated if the server sends a new set of them. In this case, we trigger the rerender of the sideline which would require rerendering the new diagnostics.

sebastiencs commented 4 years ago

Thanks, I didn't take that into account.

we trigger the rerender of the sideline

What function is called when this happen ? From where ? Are you referring to this hook:

(add-hook 'flycheck-after-syntax-check-hook 'lsp-ui-sideline--diagnostics-changed nil t)

Also, right now lsp-ui-sideline is calling flycheck functions to get diagnostics, which make it useless when flymake is used. Is there a variable from lsp-mode to pick the diagnostics ?

yyoncho commented 4 years ago

What function is called when this happen ? From where ? Are you referring to this hook:

(add-hook 'flycheck-after-syntax-check-hook 'lsp-ui-sideline--diagnostics-changed nil t)

Yes.

Also, right now lsp-ui-sideline is calling flycheck functions to get diagnostics, which make it useless when flymake is used.

There is lsp-after-diagnostics-hook - it runs if there is a change in diagnostics in any buffer.

Is there a variable from lsp-mode to pick the diagnostics ?

Check - lsp--get-buffer-diagnostics.

sebastiencs commented 4 years ago

@yyoncho I set a hook:

(add-hook 'lsp-diagnostics-updated-hook 'my-function nil t)

in my-function I'm calling lsp--get-buffer-diagnostics but it returns always nil. Calling manually lsp--get-buffer-diagnostics in the buffer returns always nil too, even if there are errors in the buffer.

Am I missing something ?

yyoncho commented 4 years ago

in my-function I'm calling lsp--get-buffer-diagnostics but it returns always nil. Calling manually lsp--get-buffer-diagnostics in the buffer returns always nil too, even if there are errors in the buffer.

Am I missing something ?

The only guess that I have is that the errors in the buffer do not come from lsp-mode. We are using this function to feed flycheck with diagnostics. I tested locally and it returns results when invoked in buffer with diagnostics,

(add-hook 'lsp-diagnostics-updated-hook 'my-function nil t)

Most likely you should not use local = t. The hook might not be called in the target buffer. We do that because finding the buffer corresponding to a uri is an extremely slow operation.

sebastiencs commented 4 years ago

The only guess that I have is that the errors in the buffer do not come from lsp-mode. We are using this function to feed flycheck with diagnostics. I tested locally and it returns results when invoked in buffer with diagnostics,

Thanks, you were right the diagnostics didn't come from lsp-mode.

I will keep using flycheck, for this PR, because flycheck handle buffer modifications and move diagnostics automatically.

I updated the PR, it will avoid any flickering, even when typing. I also changed the default of lsp-ui-sideline-diagnostic-max-lines from 20 to 1: sideline is supposed to be just a hint, and also for #494

If you have some time, or someone else, to test the branch, please do.

yyoncho commented 4 years ago

If you have some time, or someone else, to test the branch, please do.

I will be able to test but most likely tomorrow.

sebastiencs commented 4 years ago

I will be able to test but most likely tomorrow.

Thanks, lsp-ui-sideline is easy to break/misbehave so some tests/feedbacks before merging would be great

yyoncho commented 4 years ago

I saw the following error:

 Error processing message (wrong-number-of-arguments (((symbols ("new" (169 . 172) (:line 10 :character 8)) ("String" (173 . 179) (:line 10 :character 12))) (doc-id :uri "file:///home/yyoncho/Sources/lsp/lsp-mode/test/fixtures/org-mode/java-project/src/test/java/temp/AppTest.java") (line-modified . t) (this-line . "        new String()") (new-tick . t) (line-changed) (this-tick . 1312) (line-widen . 11) (line . 11) (tag 11 . 180) (bol . 161) (eol . 181) (this-line . "        new String()") (eol . 181) (bol . 161) t) (&rest) (lsp-ui-sideline--delete-kind 'actions)) 1).

(FYI I am using lsp-mode with plists instead of hts).

sebastiencs commented 4 years ago

Great, thanks for testing.

I saw the following error:

It should be fixed with abcd0ed (#490)