jdtsmith / kind-icon

Completion kind text/icon prefix labelling for emacs in-region completion
GNU Affero General Public License v3.0
174 stars 4 forks source link

Icons background color with current selection #30

Closed grolongo closed 1 year ago

grolongo commented 1 year ago

Hi!

Quick question here, how should I configure kind-icon so the current selection background color goes all the way to the icons as well?
To better explain myself here are two screenshots:

This is company-mode. As you can see, background color is also applied under the icon:

company

And now corfu-mode with kind-icon:

kind-icon

I'm using (setq kind-icon-default-face 'corfu-default) and (setq kind-icon-blend-background nil), not sure how I should tweak those two to achieve my goal.

jdtsmith commented 1 year ago

This isn't possible currently. In order to do this, company renders a version of its SVG icons with and without the selection highlight color applied as icon background. kind-icon's interface with UI's like corfu does not provide information on which item is highlighted. You might consider not using 'corfu-default as the face, but instead pick a contrasting background color that makes it seem more like part of the border.

grolongo commented 1 year ago

Thanks for the suggestion. Last thing that bothers me though, is it possible to add one space in between the right border of the icon and the the start of the word? Maybe that's something to configure in corfu instead? Let me know if I'm not clear enough.

jdtsmith commented 1 year ago

I've included a new customization kind-icon-extra-space which can be set to 1 for your desired setup. I've also made a few other unrelated tweaks. Can you please try out the version from git and report back?

grolongo commented 1 year ago

I was just playing around with pad-right and pad-left and I just tested your patch.

It's working good, thank you, but let me a bit more finicky if I may:
I think it would be better if you made kind-icon-extra-space to be either nil or t, as I don't really see the point of having more than one space here.

Lastly, you should make it not a full space but a half-one, so it will be perfectly centered and balanced.

This is how it is with (setq kind-icon-extra-space 1):

Screenshot (281)

See how the new space is slightly larger than around the icons and their borders or at the right side next to the snippet description "Dabbrev".

jdtsmith commented 1 year ago

On nil or t, that makes sense. On the half width space.. it's complicated.

Like many UIs, corfu does not measure or deal in partial spaces or pixel-based widths, since they are quite slow and cumbersome to compute with Emacs. The icons, whether text or SVG, are therefore explicitly designed to occupy precisely 3 character widths (and their display replaces 3 characters). So it's as if they are just 3 chars of text over there. Which makes alignment/layout computations happy.

I could include 1/2 extra space instead of a full, but corfu will think it is a full space, and compute a wider frame width based on a 4 character prefix. This will have the result of a frame that is 1/2 char too wide, leaving a gap on the RHS. I presume you'd not like that either... (like so)

image

grolongo commented 1 year ago

Thanks for explaining. I'm not aware of all the intricacies and it's indeed pretty tricky.
What if you have both a full space on each side to center the text? Does it look alright or does it feel odd?

jdtsmith commented 1 year ago

Well in that case we can live with the current 1/2 space on each side. If and when corfu started worry about non-fixed-width fonts, this would go away. I pushed a new version you can check out.

grolongo commented 1 year ago

I tested your latest commit and I think it looks better this way. Actually it looks like company has the same issue, even though I never noticed before.

Screenshot (286)

Thanks again for your help and the follow up!