Commands added to `embark-org-heading-map` may require additional setup which isn't explained
Hi again Omar,
I finally customized embark-org-heading-map by adding my org-notely-here command to it: https://github.com/alphapapa/org-notely/blob/dd71d333dce7bc664250409c7d8a022ac141dbb4/org-notely.el#L127
(use-package embark
;; :quelpa (embark :fetcher github :repo "oantolin/embark")
;; TODO: Install `embark-consult'.
:bind
(("C-." . embark-act)
("C-;" . embark-dwim))
(:map embark-org-heading-map
("N" . #'org-notely-here))
:init
(setq prefix-help-command #'embark-prefix-help-command))
However, when I ran it, it created a heading in the wrong place: when the current buffer is an Org buffer, it created it at point; and when the current buffer is an Org Agenda buffer, it tried to create it in the agenda buffer.
After some digging in embark-org.el, I found this code:
(map-keymap
(lambda (_key cmd)
(unless (or (where-is-internal cmd (list embark-general-map))
(memq cmd embark-org--invisible-jump-to-heading))
(cl-pushnew 'embark-org-goto-heading
(alist-get cmd embark-pre-action-hooks))))
embark-org-heading-map)
I evaluated that again, and sure enough, it fixed it, so now I can call org-notely-here on a heading or org-ql-find result and it works at the correct location in the correct buffer.
So, obviously, a couple of concerns:
- This isn't documented or easily discovered by users.
- Copying this code into the user's config is likely inadvisable, in case it needs to change when Embark is upgraded.
I don't know of the best way to solve this. Maybe there needs to be a wrapper to add commands to these keymaps that need special wrapping of commands.
Thanks as always for your work.
Thanks for reporting this. You're absolutely right that this should be documented. I guess the need for action hooks is sort of left implicit: the documentation tells you how Embark runs an action and it's up to the user to notice when that behavior is not quite what they want and it needs to be tweaked via hooks. The most common hooks used in Embark's default configuration are described in (info "(embark) Running some setup after injecting the target") and (info "(embark) Running hooks before after or around an action").
Do you think documenting the use of embark-org-goto-heading and embark-org--at-heading at the second of those info nodes would be enough? I feel like you'd also want a better answer to "how do I figure out I need an action hook in the first place?". The manual's current answer, as I said above, is to just tell you what happens if you don't have hooks and leave to you to decide if that's what you want to happen or not.
For the record, I don't recommend copying that block of code you re-evaluated to your configuration, since it will make at least one action that didn't jump to the heading now do so: namely embark-org-refile-to. I'd recommend instead adding either this:
:config
(cl-pushnew 'embark-org-goto-heading
(alist-get 'org-notely-here embark-pre-action-hooks))
or this:
:config
(cl-pushnew 'embark-org--at-heading
(alist-get 'org-notely-here embark-around-action-hooks))
The difference is that the first one will visibly jump to the heading before running org-notely-here, using pop-to-buffer and even pulsing the line momentarily, whereas the second will do it invisibly using org-with-point-at.
For the actions in the list embark-org--invisible-jump-to-heading I decided probably the invisible jump would be less annoying (but as always, I am open to suggestions). This includes org-tree-to-indirect-buffer, which I feel is similar enough to org-notely-here that I'd probably recommend the invisible jump approach.
Oh, and if you use both Embark and Consult you should definitely install embark-consult! Just install it and arrange for it to be loaded once both embark and consult are loaded.
Hi Omar,
Sorry, I overlooked your responses until just now when I ran into the problem again and was reminded to go through this process again. :) Thanks for explaining how that works and showing me how to solve it in my config.
Let me first challenge an assumption that you've probably already dealt with but that I don't understand yet: Why shouldn't every command in embark-org-heading-map be run with point at the candidate's heading? It seems like that would solve this problem, but I'm guessing it would cause others that I'm not aware of. (I mean commands that are intended to be run in an Org buffer, not an Agenda buffer.)
Assuming that's so: I do agree that it should be explained completely in the Info manual. But I also think it should be mentioned in the embark-org-heading-map docstring, with a link to the Info node.
I'm trying to decide whether some kind of function or macro to do the helper automatically would be a good idea. Part of me would like it, but OTOH it seems a bit too specialized to be worth defining a symbol for.
But, back on the first hand, I wonder if this is a pattern that is common to other major modes and maps, not just Org-related ones. If so, maybe some kind of a helper function/macro would be justified.
Let me first challenge an assumption that you've probably already dealt with but that I don't understand yet: Why shouldn't every command in embark-org-heading-map be run with point at the candidate's heading?
Well, they should all be run with point at the heading, that's why embark-org installs hooks for all of the actions added to that map in the default configuration! So I guess your question is more along the lines of why doesn't Embark add the hook for you when adding a new action to that keymap? Well, action maps are just normal keymaps, and binding a key just does that, not any other side effects. I think this is the first type of target for which all actions require some hook, it hasn't come up before.
And you can't fully automate adding the hook even for just this case because for some actions we've (well, I've) decided to have the jump to the heading occur invisibly with org-with-point-at and for others to have the jump occur visibly with pop-to-buffer. That decision needs to be made by the user for each new action (although, of course, the tool automating this could have a default).
EDIT: Dealing with #674 I realized this isn't true! Not every action should be run with point at the heading, some need to run with point wherever it was!
It can get a bit confusing, can't it? :) Thanks.