Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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

Open
kiennq wants to merge 1 commit into emacs-lsp:master
base: master
Choose a base branch
Loading
from kiennq:bug/posframe-create-too-much

Conversation

@kiennq
Copy link
Member

@kiennq kiennq commented Jun 28, 2020

Per this #464 (comment)

seagle0128 reacted with thumbs up emoji
Copy link
Member

yyoncho commented Jun 28, 2020

Copy link
Contributor

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

Copy link
Member Author

kiennq commented Jun 28, 2020

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.

Copy link
Contributor

Sorixelle commented Jun 28, 2020
edited
Loading

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.

Copy link
Member Author

kiennq commented Jun 28, 2020

@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 tumashu/posframe#65

Copy link
Member Author

kiennq commented Jun 28, 2020

@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

Copy link
Contributor

Sorixelle commented Jun 28, 2020
edited
Loading

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.

Copy link
Member Author

kiennq commented Jun 28, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericdallo ericdallo ericdallo approved these changes

@yyoncho yyoncho yyoncho approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /