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

Fix corfu get wrong size for frame in HiDPI screen, use fit-frame-to-buffer-1 adjust frame automatically. #165

Closed
wants to merge 1 commit into from

Conversation

manateelazycat
Copy link

Hi

I'm author of lsp-bridge, lsp-bridge's global is become fastest LSP client for Emacs.

I found corfu get wrong size for frame when corfu running in HiDPI screen.

Below is screenshot before patch:
2022-05-10 17-45-39 的屏幕截图

Below is screenshot after patch:
2022-05-10 17-45-52 的屏幕截图

We should use fit-frame-to-buffer-1 adjust frame automatically.

@minad
Copy link
Owner

minad commented May 10, 2022

Thanks! This addresses #149. I am not sure if I trust fit-frame-to-buffer to do the right thing. Sometimes these resizing functions don't produce the expected result. I will try this for a while. The problem is also that the wrong height computation will break the child frame positioning.

@minad
Copy link
Owner

minad commented May 10, 2022

How does lsp-bridge compare to eglot, lsp-mode, nox?

@manateelazycat
Copy link
Author

How does lsp-bridge compare to eglot, lsp-mode, nox?

its the fastest one, hahaha.

@manateelazycat
Copy link
Author

How does lsp-bridge compare to eglot, lsp-mode, nox?

lsp-bridge use python threading technologic to parse message from LSP server, Emacs don't need wait lsp-bridge.

lsp-bridge will compare LSP message with newest changed ticker, it will drop LSP message if message has expired.

lsp-bridge will push completion items to Emacs if LSP message not expired, then call corfu completion frame manually.

BTW, can you help me review code https://github.com/manateelazycat/lsp-bridge/blob/26ba3d108fbeef00c6b1246d1a4f318284a3855f/lsp-bridge.el#L380 ?

I turn off corfu-auto in lsp-bridge-mode, above code use for popup corfu frame manually, have better way to refresh corfu completion items?

Thanks.

@minad
Copy link
Owner

minad commented May 11, 2022

What you could do alternatively is to write a completion-at-point-function which is more efficient and only returns if a valid result is available. When the Capf is called, the lsp server is contacted, nil is returned. When the capf is called and a result is available, return it. Furthermore you could show asynchronously incoming results via corfu--auto-complete, I mean forcing Corfu to show the display. Then you avoid replicating the code in your project.

@manateelazycat
Copy link
Author

What you could do alternatively is to write a completion-at-point-function which is more efficient and only returns if a valid result is available. When the Capf is called, the lsp server is contacted, nil is returned. When the capf is called and a result is available, return it. Furthermore you could show asynchronously incoming results via corfu--auto-complete, I mean forcing Corfu to show the display. Then you avoid replicating the code in your project.

I don't know how to change my capf function to async: https://github.com/manateelazycat/lsp-bridge/blob/dfdbd53c4cee4c1ce83822009c9cb497a675bcef/lsp-bridge.el#L394

Because lsp-bridge will push result to Emacs, elisp side haven't async function wait lsp-bridge return.

Anyway, I push push new commit to use corfu--auto-complete instead copy code from corfu, manateelazycat/lsp-bridge@dfdbd53

Thanks for tips! ;)

@manateelazycat
Copy link
Author

@minad This patch is work for you?

@minad
Copy link
Owner

minad commented May 17, 2022

I didn't have time to explore this yet, but this is unlikely to work. See my comment #165 (comment).

@manateelazycat
Copy link
Author

manateelazycat commented May 22, 2022

I didn't have time to explore this yet, but this is unlikely to work. See my comment #165 (comment).

Maybe you can add below code in corfu to fix this issue:

;; Adjust default font height when running in HiDPI screen.
(when (> (frame-pixel-width) 3000)
      (custom-set-faces '(corfu-default ((t (:height 1.3))))))

@minad
Copy link
Owner

minad commented May 23, 2022

I tend to avoid such workarounds. It would be better to fix the font size computation, see #149 for the issue which tracks this.

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 this pull request may close these issues.

2 participants