Remove kind-icon--metdata-get

[minad]

This function has a typo in the name ;)

Seriously, it is a minor nit. There is the function completion-metadata-get which should be used for this purpose instead of assq. For affixation/annotation-function one should call both completion-metadata-get and look into completion-extra-properties. For :company-kind it is sufficient to consult only completion-extra-properties. I would remove the helper and avoid the unnecessary string mangling, since you don't save much. I always write it like this:

https://github.com/minad/corfu/blob/9d71246945d59015255ea7845c619ef49c14c5fe/corfu.el#L583-L584

[minad]

This issue just reminded me of the custom metadata function I have in Corfu because of Marginalia

https://github.com/minad/corfu/blob/9d71246945d59015255ea7845c619ef49c14c5fe/corfu.el#L608-L612

Not the cleanest solution admittedly. Maybe I should rather use completion-metadata-get there and detect Corfu in Marginalia.

[jdtsmith]

Thanks. I'm not totally clear when a give metadata property will end up in completions-extra-properties vs. the 'metadata response of the table function, so you have to look for both every time I guess.

And yes what motivated this is I saw your comment on "in order to avoid Marginalia" and in fact noticed the same thing: marginalia advises completion-metadata-get (which seems a bit... dangerous)! I think that's why I reimplemented it, similar to what you yourself had done in corfu--metadata-get, but simply all in one convenience function.

[minad]

Thanks. I'm not totally clear when a give metadata property will end up in completions-extra-properties vs. the 'metadata response of the table function, so you have to look for both ever time I guess.

For all company properties only completion-extra-properties is ever used because this is required by company-capf.el. For most other properties, the property only comes from the table, category etc. The only other properties coming from completion-extra-properties are documented in the docstring of completion-extra-properties.

marginalia advises completion-metadata-get (which seems a bit... dangerous)! I

Yes, it is intrusive. There is no other way however and it is not really problematic.

[jdtsmith]

Well it does require people (including you) using this very simple minibuffer "getter" function to be aware and route around it, as in this case! Since marginalia is never intended to operated in buffer, isn't it possible to skip its applied advice while completion-in-region-mode is non-nil?

[minad]

Since marginalia is never intended to operated in buffer, isn't it possible to skip its applied advice while completion-in-region-mode is non-nil?

Yes, this is what I was thinking. Note that Marginalia predates Corfu, so I wasn't aware of these implications back then. Furthermore if you use the default UI with completion-in-region-mode, you actually want Marginalia!

[jdtsmith]

Funny, I guess it's been so long I think of completion-in-region-mode as synonymous with completing in buffer...

BTW, what does marginalia do with lsp completions, etc.? Add much of interest? Docsig?

[minad]

BTW, what does marginalia do with lsp completions, etc.? Add much of interest? Docsig?

Nothing. As far as I know lsp mode defines its own completion category which is ignored by Marginalia. Marginalia only adds annotations for built-in completion commands/categories.

[minad]

But consult-completion-in-region reuses company-docsig for annotations:

https://github.com/minad/consult/blob/c2fed383c9c555ed017200a22efad0a9734725b0/consult.el#L2296-L2303

[jdtsmith]

Aha. I've never used consult-completion-in-region. Do these kind-icons work with it?

[minad]

Aha. I've never used consult-completion-in-region. Do these kind-icons work with it?

Your affixation-function hack should work as I mentioned before.

[jdtsmith]

Just wondering how it looks...