Make sure completion-metadata is fetched when formatting candidates #2875
Conversation
|
I have not checked. @gexplorer In #2780 I listed multiple things which didn't work with Marginalia. Are all of these issues fixed? |
|
I don't think all the problems get solved. The commands I use the most do work out of the box: find-file, switch-buffer, describe-variable, describe-function. On the other hand describe-face does not set the category right, but it can easily be solved adding it to the symbol-category config in marginalia. Once added it gets recognized by marginalia but I think that counsel's formatter messes with marginalia and some colors are lost in translation. I think it has something to do with the comment about overriding faces. This is probably because some of counsel's format functions are not overlapping with marginalia. It would be nice being able to opt out that formatting and lettong marginalia do its job if desired. |
gexplorer
force-pushed
the
fix/getMetadata
branch
from
c75973c
to
182e40a
Compare
|
I tried all the commands with custom format functions and I found out that the ones with problems are:
I managed to get them working properly:
The only change needed for this to work is how (add-face-text-property
olen (length str) 'ivy-completions-annotations t str) |
* ivy.el (ivy--minibuffer-metadata): Remove. All callers changed. (ivy--format-minibuffer-line, ivy--wnd-cands-to-str): Arrange for ivy--format-minibuffer-line to take annotation-function as argument (PR abo-abo#2875). Re: abo-abo#2780.
* ivy.el (ivy--format-minibuffer-line): Append ivy-completions-annotations face (PR abo-abo#2875).
basil-conto
force-pushed
the
fix/getMetadata
branch
from
cc0da2d
to
040d458
Compare
Other parts of Ivy, specifically ivy--wnd-cands-to-str, use ivy-state-dynamic-collection to differentiate between programmed completion functions and Ivy's dynamic collections. By disabling ivy-state-dynamic-collection in ivy-restrict-to-matches, we lose this ability. This leads to errors when collections are mishandled. * ivy.el (ivy-restrict-to-matches): When disabling ivy-state-dynamic-collection, also make sure that minibuffer-completion-table no longer holds a dynamic collection. Re: #2875. Fixes #2893.
* ivy.el (ivy--format-minibuffer-line): Accept result of either affixation-function or annotation-function. Don't add ivy-completions-annotations face to an annotation with an existing face, as per the documentation of annotation-function (#2875). (ivy--metadata-get): New function. (ivy--wnd-cands-to-str): Use it. Let affixation-function override annotation-function. Remove read-only property from candidates instead of setting it to (or adding it as) nil (#28). Fixes #2732.
|
I tried improving how we handle the |
This PR refactors the fetching of completion-metadata closer to it's usage to prevent the formatting of candidates without the expected metadata.
The problem
The minibuffer metadata was being stored in a variable
ivy--minibuffer-metadataused byivy--format. Theivy--formatis called from two places:ivy-update-candidatesivy--update-minibufferBut the value of
ivy--minibuffer-metadatawas only being set inivy-update-candidates.The bonus
I've been using ivy with this change for a few hours and as far as I can see it's now is compatible with marginalia.