oantolin / embark

Emacs Mini-Buffer Actions Rooted in Keymaps
GNU General Public License v3.0
944 stars 56 forks source link

Default action for consult-*grep doesn't work #80

Closed oantolin closed 3 years ago

oantolin commented 3 years ago

This is because what the default action does is feed the target back into the command you got it from, but the candidates from the consult-*grep commands are not something you can type into the same consult-*grep command to go to the location!

I'm not sure what to do about this. It is not a bug per se, embark-default-action is doing what it is supposed to do. I think maybe the best thing to do would be to add an action for xref-location that takes you to the specified location and just accept the fact that embark-default-action doesn't work with consult-*grep.

Any thoughts, @minad?

minad commented 3 years ago

Interesting! Now I understand why ivy has an :action argument. I never understood how the default action actually worked! I just took some things in Embark as granted magically! Can we capture the continuation of completing-read? ;)

minad commented 3 years ago

Are there more commands like this? We could also fix the grep commands themselves. But I would only do this if there are no other broken commands, i.e., every command should accept its own return value as input!

minad commented 3 years ago

And now I am getting a request for ivy-resume which would also require me to capture the continuation or add an :action argument:

https://github.com/minad/consult/issues/6#issuecomment-753721334

😀

minad commented 3 years ago

I guess the best would be to just hard code a jump as default action for xref-location. Then it would also work with the Emacs 28 show xref in minibuffer functionality. Alternatively implement some alist for configuration. I don't think default-action is a sane concept in general and unfortunately needs configuration.

oantolin commented 3 years ago

Can we capture the continuation of completing-read? ;)

People constantly propose rewriting Emacs in some other language, I say keep Emacs Lisp but rewrite Emacs in continuation passing-style! 😛

I guess the best would be to just hard code a jump as default action for xref-location.

I'll do this, and keep an eye out for other cases where feeding the target back to the command it came from doesn't work. If other such cases arise, I'll make the default action configurable.

oantolin commented 3 years ago

There's no function for jumping to a grep result? I was expecting one in compile.el, but I see you had to implement this in consult--grep-matches and consult--grep-marker. I don't want to duplicate that code, nor to depend on it.

Maybe I can make a one line grep mode buffer and use next-error. 😆

minad commented 3 years ago

Uuh :)

I think I wrote the replica since I wanted to use consult--jump or consult--preview-position which has quite some special logic.

Basically everything seems to bring its own special jump command. Imenu, flycheck, next-error... I am using consult--jump for all of them since it is better.

oantolin commented 3 years ago

I see. I think I'll go with the one-line temporary grep-mode buffer hack. I was somewhat surprised to see it works just fine.

minad commented 3 years ago

Yes, I think in your case it is good. It is just not a good solution for consult where I have stronger requirements for the preview jumps to only do benign things.