DEL only deletes an at-point target in certain major modes

[oantolin]

We currently have pre-action hooks that mark the region for two commands often bound to DEL: backward-delete-char and backward-delete-char-untabify. Unfortunately that's only the tip of the iceberg, DEL is very often rebound in different major modes:

major-mode	binding
org-mode	org-delete-backward-char
c-mode	c-electric-backspace
python-mode	python-indent-dedent-line-backspace
markdown-mode	markdown-outdent-or-delete
message-mode	delete-backward-char
cperl-mode	cperl-electric-backspace

There may be others.

I'm trying to decide whether I should add the pre-action hook for all of these, or just delete-backward-char (and let users add pre-action hooks for whatever other ones they need), or maybe try to solve this in a different way (for example add a binding of "DEL" to delete-region in the embark-general-map).

Thankfully C-w seems to be bound to kill-region in all major modes I've tried, so this is only an issue for DEL. On balance maybe the simplest approach is to bind DEL to delete-region in embark-general-map and then only delete-region needs a pre-action hook, I could even remove the pre-action hook for backward-delete-char and backward-delete-char-untabify.

[minad]

Binding DEL seems to be right and removing the other pre action hooks.

I would not add all these pre action hooks. At this point the idea breaks down since it doesn't scale to arbitrary modes.

[oantolin]

The one thing giving me pause about binding DEL is that it would also be available in the minibuffer. It does something logical but I think people might find it surprising: it deletes the region in the target buffer, the buffer in the window selected before the minibuffer was open. And, it deletes the region whether or not it was active.

I think that might be asking for trouble.

[oantolin]

Maybe it's worth having a special check for the minibuffer to unbind the mark and delete-region commands.

[minad]

Alright, you have a point here. If you special case mark and delete we are back at the special casing which I proposed initially. Also replacing the (use-region-p) check you had with a specific check of mark coincides with my initial proposal. Of course I am not against these changes ;)

You may even want to distinguish the embark-general-at-point-map and the embark-general-minibuffer-map. If I recall correctly we also talked about such an approach before, but it would be a bit of a deviation of how Embark works, not distinguishing minibuffer and at-point.

[minad]

Maybe it makes sense to introduce an embark-bounds-map, which is merged in dynamically into the keymap when bounds are present. Then mark and delete-region can go there and we do not touch the embark-general-map. I have to look it up, but something like this was mentioned in the region vs target discussion.

(EDIT: See here https://github.com/oantolin/embark/issues/347#issuecomment-899092291 and following comments.)

[oantolin]

You may even want to distinguish the embark-general-at-point-map and the embark-general-minibuffer-map

We'd have to go back to dynamic inheritance.

Maybe it makes sense to introduce an embark-bounds-map, which is merged in dynamically into the keymap when bounds are present.

I think this is probably better, since I'd expect everything useful in the minibuffer to also be useful at-point. I guess that's really the issue: we'll have to do dynamic tampering with the keymaps either way; if we expect embark-general-at-point-map to contain everything in embark-general-minibuffer-map, then it's probably easier to put the difference in an embark-bounded-target-map; if there are examples of commands we want in one but not the other in both directions, maybe just use separate maps (which can inherit from a common one, of course).

[minad]

I think this is probably better, since I'd expect everything useful in the minibuffer to also be useful at-point.

Yes, this seems to me like a good design paradigm for Embark. Most of the commands should be general and useful in every context, except the ones which lead to a transition to a region or which fundamentally act on regions delete-region. I think it is okay to have a bit of special casing even if it means going back to dynamic inheritance for the region keymaps. And it does not look that we need more special casing in the future?

[oantolin]

Mmh. We only want mark and delete-region for at-point target, and we only want embark-become in the minibuffer.

I guess most people would also only want embark-export, embark-collect-snapshot and embark-collect-live from the minibuffer, but I've been using them from regular buffers --to collect imenu targets.

There's also embark-isearch and the consult search commands that embark-consult puts under a prefix. I suspect most people only want those for at-point targets (though they make perfect sense in the minibuffer and they run the search in the previously selected buffer).

[oantolin]

So here's what I'm leaning towards right now:

• In the embark-general-map only put actions we want both in the minibuffer and in regular buffers.
• Have an embark-minibuffer-map and an embark-at-point-map (name is debatable) for the actions we want only in one spot of the other. These will be dynamically merged in.

Remaining questions:

• Should the at-point map (or whatever we call it) be merged in for all targets outside the minibuffer or only those that have bounds? I think basically at-point targets without bounds should be discouraged, so I'm OK with assuming all at-point targets have bounds.
• What exactly to put in these maps? Certainly mark, delete-region go in the at-point map and embark-become goes in the minibuffer map. The actions I'm unsure about are the ones I already mentioned:
  • the collection commands: embark-export, embark-collect-live, embark-collect-snapshot. I think hardly anyone uses these outside the minibuffer, maybe it makes sense to move them to the minibuffer map? (It is fun to add an imenu or outline candidate collector in your personal configuration and then use embark-collect-live to get an autoupdating table of contents.)
  • the search commands: embark-search and all the consult search commands. Again, probably these almost never get used from the minibuffer though they make perfect sense: they search for the minibuffer candidate in the previously selected buffer. Maybe we should move these to the at-point map.
• Does the dynamic merging in behavior need a way to opt-out of it a la #258? This probably depends on how much goes into the new keymaps. If it is only mark, delete-region and embark-become, then we may not need an opt out mechanism.

[minad]

You could also consider introducing a full split, to create separate at-point/minibuffer keymaps for each target type. I am not sure we would want to go this route, but I am also worried that there is no clear boundary here. It is not clear where to stop with the at-point/minibuffer distinction and I am sure that in the future this problem will come up again.

We need a general way to mark commands as minibuffer- or at-point-only. Then we could canonicalize the keymap and remove the command which don't make sense before prompting.

[hmelman]

the collection commands: embark-export, embark-collect-live, embark-collect-snapshot. I think hardly anyone uses these outside the minibuffer, maybe it makes sense to move them to the minibuffer map? (It is fun to add an imenu or outline candidate collector in your personal configuration and then use embark-collect-live to get an autoupdating table of contents.)

I don't have an opinion on these, but I want so say this... I've only used these from the minibuffer, but if you find them useful from the buffer, then unless there's some big downside, I'd encourage you to enable this by default so others can learn the value of his use case.

[minad]

@hmelman I think @oantolin is using his buffer target here? See https://github.com/oantolin/emacs-config/blob/master/my-lisp/embark-this-buffer.el. If you ask me I do not want to see this in Embark, since a buffer as Embark context is too large. There are better approaches for buffer-local keybindings.

[oantolin]

No, @minad, the export and collect functionality doesn't use target finders at all, it instead uses functions I call candidate collectors. What I was referring to above was this imenu candidate collector. And I was not proposing adding something like this candidate collector to Embark at all.

[oantolin]

The only candidate collectors that come with Embark that are applicable outside the minibuffer are:

• one for dired buffers that collects the files
• one for ibuffer buffer that collects the buffers
• one for the *Completions* buffer that collects the candidates
• one for Embark Collect buffers that collects the candidates

None of them are superuseful, I find. I think I only added them because the functionality is completely free: I need to be able to configure how to get the candidates for collection anyway since every completion UI does it differently, so I just let the user define candidate collectors and then don't force them to have anything to with minibuffers completions.

[minad]

@oantolin I see, thanks for clarifying. About the buffer target - do you use this in practice? Maybe I am wrong about it?

[oantolin]

@oantolin I see, thanks for clarifying. About the buffer target - do you use this in practice? Maybe I am wrong about it?

I do use it a fair bit, actually. The target string is simply the name of the buffer, and many of the most useful actions do not need the target. for those commands I could equally well use a prefix key instead of using C-- <embark-act>. But some of the actions do need the buffer name injected like byte-compile-file, load-file, consult-file-externally (I use this on HTML, PDF and image files), delete-file, rename-file, shell-command. So, since some commands do need the buffer name injected, I decided to write this this-buffer target, and once I had it, its action keymap seemed like a logical place to also put the actions that do not need the buffer name injected ---instead of having a separate prefix map.

The alternative to Embark here involves writing tiny wrappers that use the current file name instead of prompting you for a file name: byte-compile-this-file, load-this-file, consult-this-file-externally, etc.

By the way the delete-file and rename-file are made even more useful with this advice that also kills or renames the buffer visiting the file.

[minad]

Okay, maybe you are indeed right about it that there is a value here thanks to the injection. I am not opposing adding this if you feel it is valuable. My resentment is not backed by hard facts but only because I thought that having such a large context is odd and problematic due to the conflict with other ways to bind buffer-local keybindings. Your approach of actually trying out the target finder is better than only thinking about it and rejecting the idea right away.

[oantolin]

I don't think this should be added to Embark's default configuration, I'd keep it on the wiki, which currently already has @karthink's version of it (it was his idea). The reason I think it shouldn't be added is that even though the injection does give it some value, it's not a lot of value: (a) the "don't prompt; use this file" wrappers are trivial to write, and you may even have some lying around like crux-delete-file-and-buffer and crux-rename-file-and-buffer, and (b) even without the wrappers you could just use a prefix map for all those actions: I think all the commands where you'd want the buffer or file name injected already have it as the first entry of the future history, so you're really only saving yourself a M-n RET.

[minad]

I think all the commands where you'd want the buffer or file name injected already have it as the first entry of the future history, so you're really only saving yourself a M-n RET.

Yes, this is what I thought too. And there are also many commands like kill-current-buffer - "acting" on the current buffer is already present in Emacs ootb.

[oantolin]

You know, currently if you use DEL as the action from the minibuffer, it runs whatever command DEL is bound to in the target buffer (the buffer current before the minibuffer was). In particular, if you have an active region (and use tmm), run a command that opens the minibuffer, run embark-act and then use DEL as action, it will delete your active region. And if the region is not active, DEL used that way will delete a character in the target buffer.

So we actually have a lot of experience with this "danger" and it has proven to not really be a problem. Maybe the best thing for now is to not introduce any notions of minibuffer-only actions and at-point-only actions. We can always do that in the future if it really proves necessary. I think for now the downsides of complicating Embark with dynamically altering keymaps probably outweigh the possible trouble of people pressing DEL by accident when acting in the minibuffer (a danger, which, I repeat, already exists and seems to not cause any serious trouble).

[oantolin]

So, I went with the simple solution of binding DEL to delete-region in the general map, but will still consider a more sophisticated solution such as adding some notion of minibuffer-only actions and at-point-only actions. I just don't want to complicate Embark too much if the design doesn't feel right, and I like that the keymaps are used as-is with no dynamic magic/meddling. I'm definitely open to further discussion on this.

[oantolin]

@hmelman I added a new candidate collector that I think might be useful: if you run embark-collect-snapshot in a customize buffer, you now get a list of all the variable and faces listed in said customize buffer.

I also modified the ones for dired and ibuffer a little. Now if you mark some files in a dired buffer and run embark-export you get a new dired buffer with just the marked files. Same for marking buffers in ibuffer. I don't know if this makes them more useful, but it certainly doesn't make them less useful. 😛

Also, fun one I'm trying out in my personal configuration is

(defun collect-outline ()
  (cons 'consult-location
        (mapcar
         (pcase-lambda (`(,hd ,num "")) (propertize hd 'line-prefix num))
         (mapcar (consult--line-prefix) (consult--outline-candidates)))))
(add-to-list 'embark-candidate-collectors #'collect-outline t)

With that in place, if run embark-export you get an occur buffer that's a table of contents for the current buffer. And a neat thing is that if instead you run embark-collect-live you get an auto updating table of contents!

(If you only care about embark-export or at least you don't care about line numbers in the embark-collect-{live, snapshot} case the code can de simplified as below.)

(defun collect-outline ()
  (cons 'consult-location (consult--outline-candidates))) 
(add-to-list 'embark-candidate-collectors #'collect-outline t)

[hmelman]

Those sound like fun to play with. Maybe put them in the wiki at least?