Add kind formatter

[minad]

@jdtsmith I made a few more simplifications. The prefix and suffix are not trimmed anymore. I could now add this kind formatter which would make integration with icon libraries a bit easier (but not by much, your affixation approach would still work perfectly fine). Furthermore separate margin configuration for the and right side is missing. Then you could set the margin left to zero and your 2x1 icons would be nicely aligned. Please let me know what you think.

[jdtsmith]

I'll take a look later today thanks. Trimmed as in truncated? Left and right side configurable for prefix would be great, but I'd want to stick to a margin-width for each. Note that I can't add slim padding in kind-icon, since corfu would not then know how to properly offset the prefix.

[jdtsmith]

et the margin left to zero and your 2x1 icons would be nicely aligned.

For maximum visibility, I've dialed the icons to occupy the full space, so they go right the very edge of that 2x1 block. No padding around them would not look good I suspect.

[minad]

Left and right side configurable for prefix would be great, but I'd want to stick to a margin-width for each.

No, for now I don't want to add this. It is an unnecessary complication as I see it. Currently you are correct that you cannot add slim padding, since the icon should have sizes which are multiples of the character width. You should still get a reasonably good looking result with your 2x1 icons?

Maybe it makes then more sense to relax the character width requirement later on, using a pixel width computation to compute the alignment?

[minad]

For maximum visibility, I've dialed the icons to occupy the full space, so they go right the very edge of that 2x1 block. No padding around them would not look good I suspect.

Then you can occupy 3x1 characters instead I guess, since the margin width is 0.6 by default it won't make much of a difference as long as you set corfu-left-margin-width=0? My goal is to still keep things simple on the side of Corfu. Initially I did not even have plans to add icons. But since we can accommodate for them with reasonable effort and via an external provider that's fine. Using no padding or character width padding is for now the more natural choice since this is what Company and the default UI is also doing, in particular the affixation/annotation functions add spaces themselves. Furthermore we don't have to add the face leaking into padding/margin.

[minad]

Example formatter functions:

;; 1 char icon, 0.5 margin (Problem because of width computation!)
(setq corfu-kind-formatter (lambda (_)
                             (propertize
                              (concat
                               (propertize " " 'display '(space :width 0.5))
                               "#"
                               (propertize " " 'display '(space :width 0.5)))
                              'face '(:background "cyan")))
      corfu-left-margin-width 0
      corfu-right-margin-width corfu-bar-width)

;; 2 char icon, 0.5 margin
(setq corfu-kind-formatter (lambda (_)
                             (propertize
                              (concat
                               (propertize " " 'display '(space :width 0.5))
                               (propertize " " 'display "12")
                               (propertize " " 'display '(space :width 0.5)))
                              'face '(:background "cyan")))
      corfu-left-margin-width 0)

;; 3 char icon, 0.5 margin
(setq corfu-kind-formatter (lambda (_)
                             (propertize
                              (concat
                               (propertize " " 'display '(space :width 0.5))
                               (propertize "  " 'display "123")
                               (propertize " " 'display '(space :width 0.5)))
                              'face '(:background "magenta")))
      corfu-left-margin-width 0)

;; 1 or 2 char icons, 0.5 margin
(setq corfu-kind-formatter (lambda (kind)
                             (if (eq kind 'variable)
                                 (propertize " v " 'face '(:background "cyan"))
                               (propertize
                                (concat
                                 (propertize " " 'display '(space :width 0.5))
                                 (propertize " " 'display "fu")
                                 (propertize " " 'display '(space :width 0.5)))
                                'face '(:background "magenta"))))
      corfu-left-margin-width 0)

;; 2 or 3 char icons, 0.5 margin
(setq corfu-kind-formatter (lambda (kind)
                             (if (eq kind 'variable)
                                 (propertize
                                  " xy "
                                  'face '(:background "cyan"))
                               (propertize
                                (concat
                                 (propertize " " 'display '(space :width 0.5))
                                 (propertize "  " 'display "fun")
                                 (propertize " " 'display '(space :width 0.5)))
                                'face '(:background "magenta"))))
      corfu-left-margin-width 0)

The only text icon which does not work properly is the one character icon with two 0.5 margins. All the other icons are not an issue since we have enough characters to attach display properties to. This seems good enough. For the svg icons you won't have the same issues anyways since you can use scalings 2x1, 3x1, ...

[minad]

There is an issue with the solution however - if no kind function is available we won't get a margin. So I am thinking now to simply disable the left margin when company-kind/corfu-kind-formatter is present. The kind formatter has then full control over the left margin.

[jdtsmith]

I've given kind-formatter a try. Good news first: I had a function ready to go for it. I've renamed it kind-icon-formatted and it works beautifully, no completion-in-region-function hackery needed. One wrinkle: I still have to tell kind-icon about corfu's background color; perhaps that color should be passed as an optional 2nd argument to the kind-formatter function so it can style it sensibly?

I also thought of pretending my 2x1 icon was one char and adding half-space padding on each side, turning off corfu's left margin. Problem: prefixes are not always present. And this isn't just an issue by mode which you can configure around. E.g. lsp-mode sometimes "forgets" to provide kind values. Without a prefix, no left margin = ugly. [OK I see you found this too].

I consider it a breach of data/presentation separation to encourage backends be in charge of formatting. IMO, that will make so many problems down the road if you or your users want to alter how things appear. I understand it may feel simpler to get corfu out of some aspects of the presentation business, but since it's the very last step before drawing on the screen, to me it's the obvious place to have such niceties as margins, padding, color inheritance, etc. occur. It's also barely any additional complexity:

• No prefix? normal margin, then candidate.
• Prefix? normal color-matched margin, prefix, another color-matched margin, then candidate.

Just two small code paths, and as a result, you can drop your left-margin configuration option in favor of a single value, and entirely avoid the complexities of pixel-level size computation. Your contract with backends is then "give me char-aligned data without any extra padding/alignment, and I'll arrange it for you beautifully".

[minad]

I am not letting backend taking control of presentation. In my latest proposal the idea is to let the kind formatter function take full control. The kind formatter belongs to the frontend. You keep on arguing in favor of the solution where Corfu adds the margins on both sides of the icon and leaks the color into this margin. This is a solution I am not happy with since the margin/icon formatting is then scattered over two places.

In comparison to your two code paths my favorite solution is this:

1. No company-kind or no corfu-kind-formatter: left margin - prefix - candidate - suffix - right margin (this also applies if affixations are given)
2. Both company-kind and corfu-kind-formatter are specified: no margin - formatted kind - candidate - suffix - right margin

The advantage of this solution is that the kind formatter has full control. Maybe with the small restriction that the formatted kinds should be multiples of full chars. (However this restriction could be lifted easily. The kind function could return a pair of a string and a float. But probably lifting the restriction is not even necessary since one can get a beautiful result with 2x1 or 3x1 icons, including padding).

[minad]

Since in practice no capf/completion-in-region function implements an affixation function and company-kind pushed in another direction one could also drop affixation support, leaving us with:

1. No company-kind or no corfu-kind-formatter: left margin - candidate - suffix - right margin
2. Both company-kind and corfu-kind-formatter are specified: no margin - formatted kind - candidate - suffix - right margin

[jdtsmith]

The kind formatter belongs to the frontend.

Aha! See I think of the kind formatter as part of the backend. Reorganizing data as it flows through. But I understand your perspective better given this.

[minad]

The patch is very simple now, this is what I wanted to achieve. Please try it again. If it works for you I would like to merge this and finally close the icon issues.

One wrinkle: I still have to tell kind-icon about corfu's background color; perhaps that color should be passed as an optional 2nd argument to the kind-formatter function so it can style it sensibly?

I forgot to answer to this point. I see that this is convenient for kind formatters which perform blending. I am a bit wary of blending personally, since blending basically disallows themes to apply styling. I consider it better style if your kind formatter specifies two colors/faces for these monochrome icons, one for the foreground and one for the background. However you may want to avoid the face explosion, so you instead rely directly on the font-lock-faces. Nevertheless I don't want to encourage blending or reflect it directly in the API. Furthermore the color parameter is not useful for formatters which don't perform blending. Therefore I prefer to go with the simplest possible type of the function: Symbol->String.

[jdtsmith]

1. No company-kind or no corfu-kind-formatter: left margin - candidate - suffix - right margin
2. Both company-kind and corfu-kind-formatter are specified: no margin - formatted kind - candidate - suffix - right margin

Personally, I'd leave the option for affixation functions to specify unusual prefixes we haven't thought of. It's brand new to the docs in Emacs 28 so backends might start supporting it, especially smaller/newer backends. For safety they probably do need a left margin. I think I'll also keep the option to translate kind->affixation in kind-icon, even as I push the corfu kind-formatter as the first choice.

[minad]

Personally, I'd leave the option for affixation functions to specify unusual prefixes we haven't thought of. It's brand new to the docs in Emacs 28 so backends might start supporting it, especially smaller/newer backends. For safety they probably do need a left margin.

I will keep support for the affixation function in Corfu for now. But in the current implementation I don't allow it to affect the formatting to the same extent as the kind formatter (belonging to the frontend). I argue that this is the correct approach since the affixation function is backend (your argument!), may only specify suffixes (so we would need some additional complicated detection), it is used in very few cases in the Emacs 28 code base (symbol class - simple string, nothing fancy) and above all the affixation function is quite ill-defined/under-defined. There has been a lot of discussion about removing/replacing it. Furthermore Dmitry already pushed company-kind support throughout the Emacs code base, lsp-mode and eglot. So company-kind seems like the more future proof solution, maybe under a different name completion-kind/kind if someone is interested in the churn.

(EDIT: Rereading what you wrote above - I think my current solution implements what you were also thinking of. Margins are enabled for safety.)

This also answers your second point:

I think I'll also keep the option to translate kind->affixation in kind-icon, even as I push the corfu kind-formatter as the first choice.

You should not keep this. At least not in this hackish form, if you really insist on it. There is no point in keeping it since all relevant frontends will support company-kind in the future. There are three frontends which are relevant for your package: Company (if they also have some kind formatter API), Corfu via the kind-formatter and maybe consult-completion-in-region, which is used by Vertico users who want to complete in the minibuffer. consult-completion-in-region does not yet have support for company-kind, but I will probably add it there too. Interestingly I will exactly take the affixation approach to make the company-kind available to Vertico. I already use company-docsig as annotation-function (https://github.com/minad/consult/blob/7708dc760ac2fc80219b226be27e11bbc9adf5b4/consult.el#L2290). So the adapter you have in your kind-icons package will happen on another level!

[minad]

Hmm, but thinking about it - you may also decide to keep the affixation function in your package. Then I wouldn't have to modify consult-completion-in-region and wouldn't have to add an appropriate adapter there and a consult-completion-in-region-kind-formatter. I have no strong opinion on this honestly. It would simplify things for me certainly!

(EDIT: There are no other UIs which support affixation and which don't support company-kind. This is all quite pointless. So if I were you I would just kick out the affixation function support and ask UIs to offer a proper hooking point like corfu-kind-formatter. This leaves your package with a clean API. Your package is about icons and not about how to hook these into a UI via weird affixation trickery. Maybe you are aware of https://github.com/iyefrat/all-the-icons-completion? This uses affixations for icons in the minibuffer based on the completion category.)

[minad]

Btw you could write a function kind-icons-enhance-with-icons which takes as argument a completion in region function and returns the enhanced completion in region function. This would then be used as follows for example:

(setq completion-in-region (kind-icons-enhance-with-icons #'consult-completion-in-region))
(setq completion-in-region (kind-icons-enhance-with-icons #'completion--in-region)) ;; the default ui

[jdtsmith]

Other UI's may arrive, who knows. Only drawback to keeping my affixationifier is cognitive load for the user. Maybe "cleanness" is significant enough of an argument.

Btw you could write a function kind-icons-enhance-with-icons which takes as argument a completion in region function and returns the enhanced completion in region function. This would then be used as follows for example:

Yes! This is clearly the right approach vs. my more forceful/hackish wrapping. Not sure why I didn't think of this! A "user-driven" wrapper is so much cleaner. The wrapper will do what kind-icon--completion-in-region-function already does. Any advice on how best to append affixation to the metadata (which after all lives inside the code of the collection table function). I don't like adding to completion-extra-properties, as I'm not sure who actually looks at that (vs. the 'metadata table call).

[minad]

Any advice on how best to append affixation to the metadata (which after all lives inside the code of the collection table function). I don't like adding to completion-extra-properties, as I'm not sure who actually looks at that (vs. the 'metadata table call).

completion-extra-properties is the right place, like it or not :)

[jdtsmith]

OK back on the topic of prefixes: I'm trying to "half-space-wrap" them, but I have an issue. My icons are 2x1 chars which I can fake as 3 characters, but "short-text" badges can be 1 char, 2 char, 3 char, whatever the user wants, totally configurable ("va" for variable, "f" for function). The kind-formatter function is looking at one kind at a time, so it can't be in the padding business. Corfu does the prefix-padding ( (make-string (- pw (string-width prefix)) ?\s)). Since this padding comes after the prefix, i.e after the half-space, it therefore cannot be formatted with the correct BG color (in my PR #72 I also inherited color into the padding).

Thoughts? Only solution I can think of (if you insist on making me do the padding and its coloring ;) is to pass the kind-function all the kinds, and let it return all the styled prefixes (like affixation function basically). Thus it would have to do the max-width prefix padding itself. I don't love that.

[jdtsmith]

completion-extra-properties is the right place, like it or not :)

Is there a way to get to other metadata like annotation-function inside the affixation function? Now I pass them in from the completion wrapper and save in closure vars.

[jdtsmith]

OK, kind-icon is now updated, and the results are really good I think.

Have a look for plenty more screenshots. Yes, sticking to precisely 3 chars for the prefix, whether 1/2 + 2x1 icon + 1/2, 1 + 1 char + 1, or 1/2 + 2 char + 1/2 does add a very nice visual consistency.

@dgutov If you adopt the dead-simple "kind-formatter" config into company it should work pretty easy for an alternative approach to the vscode.

@minad Thanks for the suggestions and for opening a config into corfu. If you don't mind testing it a bit I'd appreciate.

In terms of the package, are people moving to non-GNU ELPA now?

[minad]

@jdtsmith Looks very good indeed! Thank you very much for putting together this icon package so quickly. My recommendation would be to put this package on GNU ELPA since it is relevant for the GNU ELPA packages Company and Corfu!

[minad]

Regarding Company - Company has a slightly more general margin-function. The difference is that this margin function has to determine the company-kind itself. But it has a bit more flexibility as a result, one could for example generate icons based on the completion category, e.g., for files add icons based on the file extension. Furthermore the margin function takes an argument selected, which can be used to adjust the background.

[dgutov]

I suppose if kind-icon has a formatter for Corfu, it can just as well carry a company-format-margin-function, to be configured similarly.

[jdtsmith]

Aha, so quite tied in. One questions for @dgutov — do backends ever request more kinds? I noticed your list is quite similar to the lsp spec, but I also noticed a few I was missing in the lsp protocol: e.g. Constructor, Reference, TypeParameter, which I've added.

BTW, I added kind-icon-blend-background boolean to disable the blending. Then you can configure :background in the :face mappings to set custom backgrounds per kind.

In terms of theming, I guess themes don't like to straight-up edit config vars, but only update faces. Maybe this means I should have a set of generalized faces like kind-icon-variable-face that by default inherit from, e.g. font-lock-variable-name-face, but that themes can alter at will. Of course themes only get interested when there are lots of users, so maybe that's premature.

[jdtsmith]

I suppose if kind-icon has a formatter for Corfu, it can just as well carry a company-format-margin-function, to be configured similarly.

This gets a bit complicated because this function would be in charge of locating and calling :company-kind on the candidate, which it seems requires directly referencing company internals.

The nice thing about the simple kind-formatter API is I don't need to know or care who called the function. Maybe a small company-kind-icon package that can explicitly depend on company, reference its internals, and setup the kind-icon-formatted <-> company-format-margin-function connection is the right approach.

[minad]

Maybe a small company-kind-icon package that can explicitly depend on company...

While I like decoupled packages, I don't think it is justified in this case. The company specific icon function would only require a tiny amount of integration code which could be put inside a with-eval-after-load block. For example in Consult I carry a bit of completion-system specific code, e.g., consult-vertico.el, Embark does the same.

For now I am happy with the kind formatter and I like its simplicity. But it is a little bit limited in the sense that it can only add kind icons and no other icons, e.g., file extension icons which would be relevant for Corfu in eshell. So maybe I also have to generalize this in the future, replacing the kind-formatter with a more general margin function similar to Company.

We could decide to make this change right now. No worries, the overall design will continue to function correctly and you won't have to adjust the formatting. The idea is this:

;; Registry for margin formatting functions, instead of the less general corfu-kind-formatter
(defcustom corfu-margin-format-hook nil
  "List of formatter hooks, which should return an icon formatter function if applicable."
  :type 'hook)

;; Example hook
(defun corfu-margin-format-kind ()
  (when-let (kind (plist-get corfu--extra :company-kind)) ;; A tiny bit of corfu specific code!
    (lambda (cand)
      (kind-icon-formatted (funcall kind cand)))))

[dgutov]

I noticed your list is quite similar to the lsp spec, but I also noticed a few I was missing in the lsp protocol: e.g. Constructor, Reference, TypeParameter, which I've added.

I think those arrived with some recent update. Good occasion to update the list: both in the mapping, and the icons themselves.

This gets a bit complicated because this function would be in charge of locating and calling :company-kind on the candidate, which it seems requires directly referencing company internals.

It would call (company-call-backend candidate 'kind).

Not need for eval-after-load, just a declare-function, maybe.

But then... I guess it would also need to reference some of Company's faces?

The nice thing about the simple kind-formatter API is I don't need to know or care who called the function. Maybe a small company-kind-icon package that can explicitly depend on company, reference its internals, and setup the kind-icon-formatted <-> company-format-margin-function connection is the right approach.

You seem to be overthinking it a little, but this can work too.

[jdtsmith]

@dgutov I think there's value in keeping kind-icon generic. Perhaps the easiest thing then is for company to maintain its own internal translator to grab output from simple kind-formatters (I imagine others might appear).

@minad So these hook functions would get access to the candidate and you could layer several of them, c-a-p-f style, and whoever speaks up first wins?

Another perhaps simpler idea is just to expand the set of possible "kinds" that the kind-formatter is fed. It's not limited to LSP kinds at the moment. E.g. file-image, file-binary, file-code, file-other or some such. The key of course would be standardizing those across front and back-ends.

[minad]

@jdtsmith

So these hook functions would get access to the candidate and you could layer several of them

Yes, the first one returning non-nil wins.

Another perhaps simpler idea is just to expand the set of possible "kinds" that the kind-formatter is fed.

From the perspective of corfu this is not simpler - this means I would have to add a file-image-formatter and check for the corfu-file-type function etc. Furthermore no completion table supports this. The solution with the hook stays out of the debate and pushes more of the process outside of Corfu to the icon provider. The hook solution could already work for files, if the icon provider checks the completion category.

Edit: I misunderstood you - yes it would be simplest to just add more different kinds. However for files we can do quite well with file extension - and this would already work if we don't only rely on company-kind.

[minad]

@jdtsmith The icon set you use - are there also file type icons for the most important formats? Then you could add a function there which resolves by file extension. The name of kind-icon is a bit too narrow then. What about rainbow-icons, prism-icons, or mono-icons?

[jdtsmith]

There are quite a few, but not necessarily designed for automation (many are contributed/named by the community). So is your idea to have kind-icon get into the business of "I have a file kind, and my candidates is `*.jpg', so return "file-image-outline"?

[minad]

Yes, you would need a separate mapping for file extension to icon. It could be out of scope of your package if you want to concentrate exclusively on kind icons. However you have already some of the infrastructure setup and it would be an easy addition. But this debate is somewhat secondary from the perspective of Corfu - currently the corfu-kind-formatter is restricted to kinds, which is too limited to support file icons. First a solution is needed like the one with the hooks.

[dgutov]

currently the corfu-kind-formatter is restricted to kinds, which is too limited to support file icons

As a reminder: company-format-margin-function currently receives the completion itself as an argument. So it could fetch the kind, see that it's file, and then pick the icon depending on the file's extension (parsing it from the completion string).

[dgutov]

BTW, have either of you encountered Reference, TypeParameter or Unit kinds in practice?

[jdtsmith]

Here's what lsp-mode returns from the backend. TypeParameter is probably a Java generics thing.

[minad]

@dgutov

As a reminder: company-format-margin-function currently receives the completion itself as an argument.

Yes. I replaced the more restricted corfu-kind-formatter with corfu-margin-formatters, which also take the candidate as argument. I don't want to stick to the less flexible solution in the longer term. See 61e520b.

@jdtsmith The complication is minimal. In order to avoid corfu dependencies in icon packages I can add the following helper. Alternatively you add a tiny bit of integration code to your package.

;; Helper
(defun corfu-kind-formatter (formatter)
  (lambda ()
    (when-let (kind (plist-get corfu--extra :company-kind))
      (lambda (cand)
        (funcall formatter (funcall kind cand))))))

;; Register kind formatter
(setq corfu-margin-formatters (corfu-kind-formatter #'kind-icon-formatted))

[jdtsmith]

Why not pass the kind formatter candidate and Kind so it doesn’t have to look up the completion property?

[minad]

Why not pass the kind formatter candidate and Kind so it doesn’t have to look up the completion property?

This would be insufficient. The kind property is not always available. The aforementioned file icons depending on the file extension are the prime example.

[dgutov]

The aforementioned file icons depending on the file extension are the prime example.

Wouldn't they have the file kind?

[minad]

The aforementioned file icons depending on the file extension are the prime example. Wouldn't they have the |file| kind?

No, they have the completion category `file`, but a `:company-kind` function is not necessarily provided. Furthermore it is kind of pointless to have a constant `:company-kind` function returning kind `file` for all candidates of a file completion table. I see it like this - the category describes the entire set of candidates and the kind is a property of each individual candidate.

[dgutov]

Furthermore it is kind of
pointless to have a constant :company-kind function returning kind
file for all candidates of a file completion table.

I don't see it as a problem: some formatting functions will show the same icon (though differentiate between files and directories), and some, which possess a large library of icons, can show different icons for different file extensions.

BTW, the latter doesn't seem feasible for text-based kind formatters, which have to be used in the terminal.

[dgutov]

@jdtsmith

Here's what lsp-mode returns from the backend. TypeParameter is probably a Java generics thing.

Not really what I asked, but I suppose it's a fair argument anyway.

[jdtsmith]

The answer was implicit ;). No I haven't seen them in the wild. But lsp-mode will certainly throw them if the server provides that kind (which I hadn't known until I looked).

BTW, another topic on lsp-mode completion is the docsig (see emacs-lsp/lsp-mode#3201)), which lsp-mode does not currently expose, since lsp servers only lazily serve up documentation. So no calling syntax in the echo area. But the lsp-mode maintainer mentions :company-doc-buffer. I'm suggesting to them to expose a docsig function which queries the same documentation, caches it, and serves up the 1st line as doc-buffer. Thoughts? Ideally this would be async, though UI's could put a "only if you pause for XX seconds" timer on.

[dgutov]

Heads-up: I've updated the mappings.

constructor was missing only in company-text-icons-mapping: added it there. Also added type-parameter and unit; somehow, they needed the same icons as existing entries ruler and parameter. Neither of those actually exists in the LSP standard, so those I removed. ruler seems like a straight mistake (confusion with the file name or something), but parameter could have existed in some previous revision, IDK (though I've looked down to 3.14, and nada). There is still a type of "semantic tokens" with that name.

@jdtsmith, looking at kind-icon-mapping in your package, you're missing unit (lsp-mode doesn't have that mapping, but the LSP standard does), you can also delete ruler and parameter, and type-parameter is missing a dash.

[jdtsmith]

Thanks, fixed. ruler did seem like an odd category!