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

Take line-spacing into account when calculating height #72

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yurikhan
Copy link

The frame height is calculated in company-box--set-frame-position as the character height in pixels multiplied by the number of items. The character height does not include line-spacing; this causes the calculated height to be too small.

This PR adds handling of possible non-nil line-spacing values.

@yurikhan
Copy link
Author

Additionally/alternatively, it would be nice if company-box--render-buffer called a hook at the end, so that the user could set line-spacing to nil specifically for the company-box buffer.

@stardiviner
Copy link
Contributor

I applied your patch in my branch, but still not fix this issue.
Here is my screenshot:

image

@yurikhan
Copy link
Author

yurikhan commented Nov 2, 2019

@stardiviner Are you sure you don’t have a stale .elc lying somewhere around?

@stardiviner
Copy link
Contributor

After testing out, I found this line-space wrong issue is caused by icons which from all-the-icons. If this patch can fit to all-the-icons line space or I have to disable icons.

@yurikhan
Copy link
Author

yurikhan commented Nov 3, 2019

Icons are a different issue. Feel free to take it.

@stardiviner
Copy link
Contributor

Any idea to calculate the icon height? I want to take a try to add patch. @yurikhan

@mpanarin
Copy link

I think its better to leave the calculation here naive and create hooks, that allow manual repositioning of the frame. This way it will be working properly with the defaults as well as allow further compatibility changes

i.e. if you are in emacs27 you'll have an issue with the new tab-line mode, where the company-box position will be completely broken :\

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.

None yet

3 participants