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

completion-extra-properties #8

Closed
daimrod opened this issue Dec 12, 2012 · 14 comments
Closed

completion-extra-properties #8

daimrod opened this issue Dec 12, 2012 · 14 comments

Comments

@daimrod
Copy link

daimrod commented Dec 12, 2012

completion-extra-properties doesn't seem to be supported by ido.

https://github.com/daimrod/manual-tagging/blob/master/manual-tagging.el#L85

I use it here to customize the completion buffer, it lets me display additional information.

@DarwinAwardWinner
Copy link
Owner

Comparing the completion code between stock completion and ido completion, it looks like both standard Emacs completion and ido call display-completion-list through minibuffer-completion-help and ido-completion-help respectively, but minibuffer-completion-help implements some preprocessing including adding annotation while ido-completion-help does not.

The easy solution would be to fall back to standard completing-read if completion-extra-propertiesis non-nil, and I think that's the first change I will make. The better solution would be to implement support for at least the two existing properties :annotation-function and :exit-function. I think it should be possible to implement both.

What do you think? Is the fallback strategy viable for now?

@daimrod
Copy link
Author

daimrod commented Dec 12, 2012

Thanks for the quick reply.

Yes, I think that using completing-read when ido-completing-read can not
do the job is a correct approach.

ATM I don't have much time, but I plan to work on this issue ASAP. I've
just report the issue because I've started to use ido-ubiquitous and it

says that every incompatibilities should be reported :)

Daimrod/Greg

@DarwinAwardWinner
Copy link
Owner

Ok, I will push an update for now that disables ido-ubiquitous when completion-extra-properties is non-nil. Then we can work on a real solution. I'm thinking that one could write a wrapper display-completion-list-with-annotations that handles the :annotation-function property and then calls display-completion-list. Then define some advice on ido-completion-help that lexically replaces the binding of display-completion-list with the wrapper. (Can you lexically modify the function slot of a symbol?).

@DarwinAwardWinner
Copy link
Owner

Is completion-extra-properties a newly-added variable?

DarwinAwardWinner added a commit that referenced this issue Dec 12, 2012
"completion-extra-properties" appears to be a new variable that has
important effects on how completion occurs, and ido doesn't support
it.

This provides a workaround for #8.
@DarwinAwardWinner
Copy link
Owner

Can you test out the latest master to see if it works as you expect? If so, I will make it a release. Thanks.

@daimrod
Copy link
Author

daimrod commented Dec 12, 2012

Though it does detect the completion-extra-properties stuff, it still
uses ido and I don't understand why because it does execute the correct
ad-do-it.

Daimrod/Greg

@DarwinAwardWinner
Copy link
Owner

Ok, I'll have to investigate this when I have more time.

@daimrod
Copy link
Author

daimrod commented Dec 12, 2012

I don't know if it was recently added, I've started to play with it the

last month. I think it has been here for a long time but is rarely used.

Daimrod/Greg

@DarwinAwardWinner
Copy link
Owner

Actually, it looks to have been added in Emacs 24. I'm still on Emacs 23 and I don't have it. Time for me to upgrade, I guess.

@daimrod
Copy link
Author

daimrod commented Dec 14, 2012

Ok, I'll have more time to try to fix this issue the next week.

@DarwinAwardWinner
Copy link
Owner

Whoops, I forgot that ido-ubiquitous now has two code paths, one for Emacs 23 and below and another (cleaner) one for Emacs 24 and above. I put the fix in the code path for 23, where it had no effect. I've just pushed 0dff868 which moves the fix to the correct code path, and it works for me in Emacs 24.

@daimrod
Copy link
Author

daimrod commented Dec 15, 2012

Nice, it works as expected.

Thank you.

Daimrod/Greg

@cichli
Copy link

cichli commented Apr 5, 2015

@DarwinAwardWinner adding annotation support to ido-completion-help sounds like a great solution, and :exit-function seems certainly doable. Do you still plan to implement support for this? :-)

If not, I'm happy to take a stab at a PR.

@DarwinAwardWinner
Copy link
Owner

I'm not currently planning to implement this, but I would certainly be willing to merge a patch implementing it. Make sure you check all the properties in completion-extra-properties and fall back if any of them are ones your code can't handle.

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

No branches or pull requests

3 participants