lsp-ui icon indicating copy to clipboard operation
lsp-ui copied to clipboard

posframe-integration: dont create posframe on every request

Open kiennq opened this issue 4 years ago • 8 comments

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

kiennq avatar Jun 28 '20 01:06 kiennq

cc @Sorixelle

yyoncho avatar Jun 28 '20 05:06 yyoncho

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

Sorixelle avatar Jun 28 '20 07:06 Sorixelle

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.

kiennq avatar Jun 28 '20 07:06 kiennq

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.

Sorixelle avatar Jun 28 '20 10:06 Sorixelle

@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 avatar Jun 28 '20 10:06 kiennq

@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

kiennq avatar Jun 28 '20 11:06 kiennq

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.

Sorixelle avatar Jun 28 '20 12:06 Sorixelle

@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.

kiennq avatar Jun 28 '20 12:06 kiennq