Inability to consider current context as matching multiple target types

[jyp]

I would be able to use Embark as a general context-dependent way to execute actions. This may mean that several things may be relevant in the current context (and thus several keymaps may apply). Currently, only the first matching target is taken into account.

Why do I want to do this? For example, there may be simultaneously 1. a flymake error at point to deal with and 2. a symbol whose definition can be looked up. Another use-case would be to handle all xref-identifiers uniformly (do xref-find-definition on the identifier at point), which currently with the elisp symbol definition (which is only valid in elisp contexts, but offers a wider set of actions).

Do you think that the scope of Embark can be widened to support this sort of use-case? Perhaps there is another package which is more suitable for this?

[oantolin]

I've wanted this too for a while! The case that bug me the most is functions, variables or macros with the same name as a package. For example, I've tried several times to run describe-symbol on pcase, only to be reminded that ffap got to it first and I can only do file actions on pcase.el.gz.

The fairly recent change from separate target finders and classifiers to functions that return a pair of a calssification symbol and a string target was made with an eye toward this.

There are some choices about how to present this functionality. I think that to start somewhere I might just gather all the targets found in the current context, and enable all their keymaps simultaneously. If this proves too frustrating with actions shadowing other actions, I guess I could prompt for which target you mean before enabling a keymaps. Something like that, probably some experimentation is needed to find a good balance.

(On a related note, for embark-become I recently switched from enabling the first key map containing the current command to enabling all such key maps.)

[minad]

What about nesting the keymaps somehow using a reserved prefix key, e.g., x?

keymap of first target
x -> keymap of second target
x x -> keymap of third target
x x x -> keymap of first target

I don't know if this works but one could even cycle through the keymaps using long prefix key sequences :)

Maybe Emacs refuses to handle cyclic prefix keymaps?

[jyp]

@oantolin Cool! For my fingers memory I think that I need not clash between keymaps, so enabling them all seems the right option. I might try to code it up, if so I'll submit a PR.

[oantolin]

I wrote a quick sketch in the multiple-targets branch 73a4c62. Please test, @jyp.

It at least solves my problem of wanting to run h on pcase. But there is some shadowing, for example, d on pcase will delete the file pcase.el.gz, and not go to definition.

Now, shadowing doesn't mean the simple enable-all-keymaps approach has to be abandoned, it might be possible to just rearrange the target finders in an order where the shadowing is OK. If not, maybe @minad's idea works.

[minad]

I would also prefer the merge actually. But I think there are multiple cases where the best key is used for multiple different twings. And then you probably don't want to make the single target keymaps worse in order to avoid shadowing? Maybe combine the merging and add the cycle option via a prefix key. Maybe only bind the alternative bindings in the nested maps.

a action
x a alternative 1
x x a alternative 2
...

Or just create the alternative keymaps by rotating the different keymaps and then merge.

[oantolin]

Or how about: we enable all keymaps, read a key sequence, if it specifies a unique command run it, otherwise prompt using completing-read. The candidates when prompting would be (format "%s on %s" cmd target).

[oantolin]

I think the cycling you suggest @minad would leave you flying blind. How do you know how many times to press x?

Ah, nevermind, your answer is: which-key.

[minad]

What you suggest is also good. Maybe better. Downside is only that you introduce some kind of intermediate logic instead of just using the keymaps. But it is probably good for usability. I tried to make a proposal purely with keymaps.

[minad]

Yes I thought about which-key. I think this will look like cycling through keymaps. And you can always go back since it is a cycle.

I think the idea is kind of cute, but maybe not very usable. Nevertheless, I wonder how Embark can be used without some kind of indicator for the actions. If you merge keymaps it is already pretty much like flying blind to me since you have to know which keymaps have been activated. But probably in many cases you know what will happen for the specific target.

[oantolin]

I just noticed another problem: what if the command you use as an action is not bound in any keymaps (I know, I know, I'm the only crazy person that cares about that case 😛)? You'd need to specify somehow which of the targets it should act on.

[minad]

Now, I am confused. You already specify the action you want to execute. So it will just do what it is supposed to do? I mean the question here is what happens if a target matches multiple target types. Maybe I misunderstood?

(Edit: I am not very familiar with the Embark actions in arbitrary contexts, since I have been focused mainly on the completion context now.)

[oantolin]

By the way, here are two examples I often use that are not bound in any keymap:

• Sometimes I use find-file to open a file that under version control, but along the way realize I should pull changes first. In this case I use embark-become to switch to magit-status. I don't bind magit-status in a become keymap, I just use it's global binding.

• I install Embark and my other packages from Melpa, but I develop them in some other directory. When I'm working on Embark, M-. or embark-act d would both take me to the file from Melpa, so I use consult-imenu as an action instead to stay in the same file. Again, I don't bother binding consult-imenu, I just use my global binding for it.

[oantolin]

@minad

Now, I am confused. You already specify the action you want to execute. So it will just do what it is supposed to do? I mean the question here is what happens if a target matches multiple target types. Maybe I misunderstood?

The target finders return pairs of type and target, there is no restriction that all targets are equal and just the types differ. To use my pcase example yet again, if you are in an Emacs Lisp file and point is on pcase, you get both (file . "path/to/pcase.el.gz") (from ffap), and (symbol . "pcase").

[minad]

Ok I understand, for arbitrary commands you have to select if you either want to act on the file or symbol. Then I think it is better to always go with the completing-read solution for consistency instead of doing some prefix keymap trickery.

[jyp]

I did only a couple of test, but as far as I can see it's exactly what I wanted! 👍

[oantolin]

Good to hear @jyp! It needs some work, including design work, before I can merge it, but knowing it's a step in the right direction, I'll work on it some more. Also, I'm sure it's buggy in its current state, I just banged out the first thing I thought of as quickly as I could.

[jyp]

Alright. Perhaps you already know about it, and it's not clear if its related to the changes in the branch, but I'm getting an error due to the definition at line 705:

        (target-indicator (mapconcat #'cdr pairs " | "))

Indeed, pairs don't necessarily contain strings in their second elements. (At least, I imagine it does not need to.)

[jyp]

I decided to see what I could do about this. After cleaning up the code from everything that I did not need, I ended up with just a page of code plus the configuration. So it may not make sense to combine minibuffer actions with general context-based actions.
But perhaps you'll be able to find useful ways to combine the two anyways. In case you're interested, here is what I am using now:
https://github.com/jyp/dap/blob/main/dap.el

[minad]

@jyp I did also propose originally to split up the Embark package. The cutting point that you made here is certainly a possibility and I had considered that too (minibuffer actions vs other contexts). I wonder what you are missing from Embark functionality now in the other contexts? It seems you added a few more targets? Is it possible to cut out the other contexts from Embark and only use Embark for minibuffer actions and completions, then binding dap and embark to the same key, but Embark only to the minibuffer map? If that is the case the packages could cooperate cleanly in a non overlapping fashion. But I don't think it is possible since you may want to reuse certain Embark keymaps in minibuffer and other contexts? In that case such a split could not be made. However it still may make sense to have a dap package for people who don't want minibuffer actions or want to combine it with another completion system which already brings actions. But it wouldn't fit with the spirit of having small orthogonal independent packages.

[oantolin]

I like it, @jyp! Nice and simple. I think I need something more general for Embark, but if dap does everything you need, by all means use it!

If I understand your code correctly:

1. The "calling convention" it uses is that it funcalls the action with the target (or just calls the command in the special dap-no-arg case). That's perfectly reasonable for action commands of 0 or 1 arguments, but Embark needs support for higher arity actions (by "needs", I mean I personally use them all the time 😛). For example, you can use rename-file as an action with Embark, the target is the original file and you still get prompted for the new file name.

   I know Embark's "calling convention", injecting the target at the first minibuffer prompt, is a little funky, but it was the simplest way I could figure out of calling an action of several arguments, like rename-file and having the target become the first argument, but still getting prompted for the remaining arguments interactively. Even for unary actions Embark's weird approach has advantages: some commands call completing-read directly (instead of using interactive) and do not actually have the thing they act on as a function argument! @minad does that a lot in Consult for example, and I can use those Consult commands as actions in Embark with no trouble.

2. You just merge all applicable keymaps as I did above. I guess you made sure the actions you care about don't shadow each other. I think for Embark I do need some form of disambiguation.

3. I can't use M-x to run any command I want as a dap action. I'm unreasonably fond of that feature in Embark even though I doubt anyone else has ever used it. This would be easy to add to dap, of course.

4. If you run dap-dap and press C-h C-g the prompt sticks around, even though the keymap seems to no longer be active.

I do like your strategy of partially applying the action to the target in the composed keymap. Much nicer than my strategy of searching for the right target once the action is known.

[oantolin]

@minad

The cutting point that you made here is certainly a possibility and I had considered that too (minibuffer actions vs other contexts).

Those share like 95% of the code. You need to (1) get the target, (2) figure out which actions to offer for it, (3) prompt the user for an action, (4) run the action on the target. The only part that differs from minibuffer targets and non-minibuffer targets is step (1).Since non-minibuffer targets are varied it makes sense to have a configurable list of target finders for the non-minibuffer case. Once you have that, you can just add the minibuffer target finder to that list and you've also covered minibuffer actions!

[minad]

@oantolin

Those share like 95% of the code.

Okay, then the dap package by @jyp is mainly useful if you do not need minibuffer actions or already have them due to ivy/helm. Otherwise Embark covers everything or is there some other selling point of the dap package that I missed?

I know Embark's "calling convention", injecting the target at the first minibuffer prompt, is a little funky.

Since you mentioned the calling convention - I do believe that the Embark calling convention you are using is the right method to reuse interactive commands! I somehow implicitly assumed that dap also uses this technique.

You just merge all applicable keymaps as I did above. I guess you made sure the actions you care about don't shadow each other. I think for Embark I do need some form of disambiguation.

You don't have these multi actions merged yet, right? I agree that it would be good to have this with some prompt to select the action has been discussed above. It is not good if keymaps are not allowed from shadowing each other. Shadowing should be minimized, but this cannot be avoided always in particular if you want to use the best keys in different keymaps.

I can't use M-x to run any command I want as a dap action. I'm unreasonably fond of that feature in Embark even though I doubt anyone else has ever used it.

I am using this feature now that I learned that it exists 😄

[jyp]

1. The "calling convention" it uses is that it funcalls the action with the target (or just calls the command in the special `dap-no-arg` case). 

Correct.

That's perfectly reasonable for action commands of 0 or 1 arguments, but Embark needs support for higher arity actions (by "needs", I mean I personally use them all the time stuck_out_tongue). For example, you can use rename-file as an action with Embark, the target is the original file and you still get prompted for the new file name.

   I know Embark's "calling convention", injecting the target at the first minibuffer prompt, is a little funky, but it was the simplest way I could figure out of calling an action of several arguments, like `rename-file` and having the target become the first argument, but still getting prompted for the remaining arguments interactively. Even for unary actions Embark's weird approach has advantages: some commands call `completing-read` directly (instead of using `interactive`) and do not actually have the thing they act on as a function argument! @minad does that _a lot_ in Consult for example, and I can use those Consult commands as actions in Embark with no trouble.

I see. I must say I did not understand this bit at all and this is why I did it the way I did. It would still possible to take the apply-partially approach, and read the argument interactively (using call-interactively instead of funcall). One has to duplicate the reading of the argument in the command which is bound in the keymap, but If you do this for only a few functions I think it's viable.

2. You just merge all applicable keymaps as I did above. I guess you made sure the actions you care about don't shadow each other. I think for Embark I do need some form of disambiguation.

I did not do any disambiguation. I configured my targets in such a way that there is either a clear target to prioritize, or use different keys for targets which may overlap. Perhaps I can do this because I never need to "become" something else.

3. I can't use `M-x` to run any command I want as a `dap` action. I'm unreasonably fond of that feature in Embark even though I doubt anyone else has ever used it. This would be easy to add to `dap`, of course.

Since I am never using it from the minibuffer, using M-x has not much (any?) use.

4. If you run `dap-dap` and press `C-h C-g` the prompt sticks around, even though the keymap seems to no longer be active.

That would be a bug in set-transient-keymap

I do like your strategy of partially applying the action to the target in the composed keymap. Much nicer than my strategy of searching for the right target once the action is known.

[jyp]

Those share like 95% of the code.

Functionality is shared, but in reality after I got rid of things that I did not need and simplified the code there is not much in common except the configuration.

Okay, then the dap package by @jyp is mainly useful if you do not need minibuffer actions or already have them due to ivy/helm. Otherwise Embark covers everything or is there some other selling point of the dap package that I missed?

Beside simplifying the code (which can help if you want to configure/reuse parts), as far as I know the differences are:

• dap can match several target types. (This issue)
• arguments need not be strings, they can be any value

[jyp]

Before I forget, another thing that I added (easy when using set-transient-keymap) is that you can configure an action to be repeatable. For example, you can move a column in an org-mode table by several positions in a single run.

[oantolin]

Since I am never using it from the minibuffer, using M-x has not much (any?) use.

I think maybe I didn't explain the M-x thing clearly, @jyp. What I mean is that with dap you only pass the target as an argument to commands explicitly listed in the dap keymaps. I'd like to be able to choose any command I want, whether or not it is listed in some special keymaps, and have it receive the target.

[oantolin]

Before I forget, another thing that I added (easy when using set-transient-keymap) is that you can configure an action to be repeatable. For example, you can move a column in an org-mode table by several positions in a single run.

That is nice! Old versions of Embark used set-transient-keymap but I decided to stop using it because Embark's calling convention is tricky to get right with set-transient-keymap.

EDIT: No, it's not tricky to get right, I think. What I should have said is that I didn't figure out how to correctly implement Embark's calling convention with set-transient-keymap at the time and decided to stop using it so I could implement the calling convention more robustly. Probably now, I could redo it with set-transient-keymap.

[minad]

@oantolin

Probably now, I could redo it with set-transient-keymap.

As you know, I argued before in favor of set-transient-keymap and still think that it is a viable option. There were also issues with a keycast package and maybe with other packages, I read somewhere. Instead of set-transient-keymap I suggested to use your own implementation (pre-command-hook+overriding-terminal-local-map), since in practice I found set-transient-keymap to be too limiting. However I don't think you have to change anything in Embark as of now. It works well as is. Changing the underlying implementation again is only justified, if we would find more serious problems with other packages.