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

posframe-integration: dont create posframe on every request #465

Open kiennq opened 4 years ago

kiennq commented 4 years ago

Per this https://github.com/emacs-lsp/lsp-ui/issues/464#issuecomment-650664735

yyoncho commented 4 years ago

cc @Sorixelle

Sorixelle commented 4 years ago

Gave this PR a test on my setup, it looks like the windows aren't appearing in the correct place; almost a similar issue to what we were trying to solve with posframe in the first place.

Example

kiennq commented 4 years ago

What's your configuration for lsp-ui-doc? The change is simple, and from the logic it just stop calling lsp-ui-doc--delete-frame so the posframe-show works as expected and doesn't create new frame so often. If there's problem, I doubt it would be problem on posframe itself (and previous child frame implementation), so just switch to posframe will not make the problem going away.

Sorixelle commented 4 years ago

Actually, I think this is a problem on posframe. It seems like, from https://github.com/tumashu/posframe/blob/master/posframe.el#L703-L711, that the posframe's position is only updated when the position is different from last time (we're always passing the same poshandler so it doesn't change), and when the parent frame's size changes (which is unlikely to happen).

When you create a new frame, the variables that store the previous values are zeroed out (https://github.com/tumashu/posframe/blob/master/posframe.el#L306-L308), so the frame gets positioned properly.

kiennq commented 4 years ago

@Sorixelle The issue is indeed inside posframe implementation, specifically with posframe--set-frame-position. I guess you're setting the lsp-ui-doc-position to bottom, which will trigger the poshandler of posframe-poshandler-frame/window-bottom-right-corner. posframe-show will call posframe--set-frame-position and since the position is the same (relative from right edge and bottom), it will not trigger set-frame-position again. Well, the child frame size does changed, so without calling set-frame-position, the position will be wrong.

See this PR https://github.com/tumashu/posframe/pull/65

kiennq commented 4 years ago

@sorixelle, I've created PR to fix problem on posframe repo, can you test it with your setup? Note that problem is not happening with default lsp-ui setup that show popup at point

Sorixelle commented 4 years ago

Positioning is better, but there also seems to be an issue where the window in the frame isn't correctly sized all the time. In this recording, once I focus the frame, I hold down l (I use evil) to move through the line. Moving the point right doesn't jump to the next line once it gets to the end, so it seems like the content of the buffer is all one line; just the window isn't fit to the frame and is too small to display it all.

Example

EDIT: Just checked in case, this issue is happening for me as well when lsp-ui-doc-position is set to 'at-point.

kiennq commented 4 years ago

@Sorixelle What's your setup again? Can you have minimal repro for this? Did this issue not repro without this change? I doubt it may related to some of your special minor modes that automatically breaks line visually but doesn't inform Emacs so the fit-frame-to-buffer does not work. Actually it seems that the size of frame is calculated based on all of your 3 lines as a single line.