Some improvements to embark-act-all

[minad]

I tested embark-act-all and it seems to work well overall. I've observed a few minor issues and improved them in this PR.

1. Quitting did not work properly with the keymap prompter
2. The indicator can do a bit better, showing Act* instead of Act
3. (Unrelated) we should not use S for elp, since this conflicts with snapshot
4. Maybe we can clean up the functions a little bit better. I applied a few minor refactorings as a start.

[oantolin]

The problem is with default-directory: when you are completing file names in the minibuffer you can naviagte to a different directory, when you do that the candidates are just the last component of the path, and the directory you navigated too is not the default-directory of the minibuffer or of the target buffer. This causes problems in several places in Embark.

Maybe the best solution would be to switch the candidate collectors for the various UIs to return full paths, and then just relativize them again in embark-collect and embark-export. The target finder for the minibuffer does use full paths so embark-act doesn't have to worry about this, it's just the candidate collectors that use, well, completion candidates instead of making them "full candidates".

[minad]

Makes sense. But what I find confusing is that for embark-export you somehow fix the default-directory such that it reflects the modified path? Couldn't we use the same default-directory fix for embark-act-all? Of course the alternative would be to switch to full candidates (and abbreviating them again in dired). Probably this would be my preferred solution but I am not sure about potential edge cases.

[oantolin]

Fantastic, this all looks great, thank you!

I did notice the problem with quitting yesterday but was near the end of my allotted time for this.

I can't believe I didn't notice S conflicted with snapshot!

Thanks for the refactorings, I was definitely in bang-it-out-quickly mode last night and intended to clean up later.

[minad]

I can't believe I didn't notice S conflicted with snapshot!

Me too 😆

Thanks for the refactorings, I was definitely in bang-it-out-quickly mode last night and intended to clean up later.

You are welcome! Of course, this is a perfectly valid approach and it is good to get the ball rolling!

One thing I wondered - do we really need this confirmation in the current setup? I am generally leaning a bit towards the safer side, but since embark-act-all is started via A there is already one additional step in C- . A x. I like this A binding btw, originally I've considered to give embark-act-all another toplevel binding, but A is much better.

[oantolin]

I added the confirmation because you mentioned it somewhere, but I think it's a little annoying. How about this: only ask for confirmation if the action was on embark-allow-edit-actions? As you probably noticed I let-bind that to nil to avoid having to press RET for every single file deletion, for example.

[oantolin]

The A binding is upside down, I really wanted to bind it to ∀...

[minad]

Hmm. Indeed we could ask for confirmation only for the problematic (allow edit) commands. However since you can start arbitrary commands via M-x or other keybindings I find this all a bit dangerous. Probably better to confirm always at least by default and add a confirm customization option which can have the values t, nil or 'allow-edit.