RFC: New embark-org package #502
Comments
|
To be honest, I only use the table cell target to copy the contents of a single cell to the kill-ring, but even just for that it's worth it. 😛 |
|
Footnotes have a few reasonable actions too! |
|
Cool. I look forward to try this out. Org-menu seems also useful for inspiration? The copy as MD is a good ootb addition. I also have sth like this in my config. |
|
Maybe the copy as markdown action should be less trusting of the user and explicitly check that you are using it on a URL. The current version will happily produce "Markdown" links for any link type at all. |
Yes! Thank you for mentioning it, @sheijk had even suggested that his package be used as inspiration, but it had slipped my mind. |
|
Copy as markdown is also useful for the markup. I would trust the user and bind it to the region map. |
|
Oh, you mean use the markdown exporter on a region. Yes, good idea. I currently just have a small function for links. |
|
More generally there are a bunch of org commands that are useful for regions. I think embark-org should add a prefix key to the region map. |
I love this! Having obfuscated solutions in code comments is indeed a good use case for the rot13 action 😆 |
|
I just looked at the code a bit and I have only a few minor comments:
About your keymap design - One alternative is to support merging multiple keymaps via entries like this the ...
(embark-org-file-link embark-org-link-map embark-file-map)
(file . embark-file-map)
(embark-org-link . embark-org-link-map)
...I think this should be very easy to do in embark.el directly and you avoid the inheritance ceremony in embark-org.el. Maybe this proves to be useful elsewhere? Obvious downside is that we don't have an extra dedicated keymap in case the user wants to add more specific actions, e.g., for actions which make only sense for org-file-links. But if that's needed the user could still create an additional dedicated keymap and register it too. |
|
About the loading of embark-org - I would suggest to add a (provide 'embark)
(with-eval-after-load 'org
(require 'embark-org))Alternatively the user puts a nested with-eval-after-load in the configuration (or the equivalent use-package statements): (with-eval-after-load 'org
(with-eval-after-load 'embark
(require 'embark-org))) |
|
I'm glad you liked the use of ro13. 😛 I'm not too keen on allowing multiple keymaps in the |
|
About the FSF assignment, Stefan Monnier already wrote to me about that. 🙂 GNU ELPA has a copyright check feature and having that file with the incorrect copyright keeps GNU ELPA from building the tarball for my package. I told him I was hoping to hold off on publishing, so he told me to create an |
|
I'm still undecided about whether to bundle embark-org with embark, or to make it a separate package, actually. |
|
I understand your point about the names, but |
|
I'm still undecided about whether to bundle embark-org with embark, or to make it a separate package, actually.
Any advantage in making it separate? In Consult I also have consult-org,
consult-flymake and so on.
|
|
I guess I can't think of any advantage. I had just visualized it as separate for no good reason! 😆 |
|
I'm not too keen on allowing multiple keymaps in the `embark-keymap-alist`, mainly because of the issue you mentioned, that there is no obvious place to add new bindings. I don't want to force users to figure out how to make such a keymap and register it themselves, I think you can go a long way in cofniguring your Emacs with just `define-key` or `use-package`'s `:bind` and not know how to create a new keymap that inherits from *two* others. But also, I think it makes it more difficult to examine Embark's keymaps from outside `embark-act` (if your are in `embark-act` it's easy to look at the current keymap, of course). I often use `embark-bindings-in-keymap` to remind myself of soemthing about the actions for a given type, and it's very handy that they are all in a single keymap.
I think the case where we have multiple keymaps and the user wants to
register a special action which only applies to the intersection is
rare. If that case occurs it shouldn't be too hard to figure it out for
the user.
Regarding inspecting the keymap - this argument can be turned around,
since if we have multiple keymaps you have *fewer keymaps* to inspect.
It is more economical that way since you only have to inspect the link
and the file map. ;)
Overall I don't have a strong preference either way. Supporting merging
makes Embark slightly more flexible, but it is also questionable if such
use cases come up often. I am not able to quickly come up with another
good example (besides the org link use case), but if we would find
something, then I would consider this a stronger reason to support
keymap merging.
I am not really fond of the merged keymap macro definitions, but this is
only a minor detail. If I were to decide I would probably implement both
ways and compare the result to see which option leads to less code. I
think that supporting merging could be advantages but maybe it is not if
multiple parts of Embark are affected.
|
|
I tried your suggestion, @minad, of generalizing I've also added a general Org target finder, and actions for source blocks. |
|
Looks good! Now we have a hell of new targets 👀 |
|
There is also a org-table-convert-region in the region map. Would it make sense to move that action to embark-org such that it is merged in only if you are in an org buffer? But it seems also generally useful. Generally I wonder if there are more targets which should be available immediately even outside of Org buffers, e.g., links. But this requires us to write our own target finder since Org is not yet loaded. |
And you'll notice I only enabled 9 out of 54 Org objects and elements! (I have no idea what the difference between an object and an element is, |
|
I think I also wondered about Org links in non-Org buffers. I was thinking of this compromise: keep the link target finder in embark-org, so you only have it active once you've loaded Org, but make the target finder work in all buffers (the initial version did, I'd just have to bring that one back!) and change the One thing that gave me pause is that I'm not sure |
Yes, I noticed. It is a nice implementation and we can easily extend it in the future :)
I don't know. In practice I usually don't use the global command in Org buffers. |
|
@hmelman, you are a big Embark user in text modes: what do you think about the following issue with headings? The org parse includes in the bounds it reports for a headline the entire subtree., while the general outline heading support we have in Embark includes only the line of the heading. For the purposes of navigation or promoting/demoting headings that does not matter, but it does make a difference of course in the "copy to kill-ring" or "delete" actions. Should we settle on just one of the options or should we have both targets, so that people can quickly copy just the heading line, or cycle to the next target and copy the entire subtree? |
|
I'm only an occasional org-mode user, though I use markdown-mode a lot. In markdown-mode I'd expect an embark heading target to just be the heading, not the whole subtree. I'd expect this in other modes with headings too. I think for org-mode embark should do what org users expect. If other org commands typically operate on the sub-tree do that, even if it's different from other text modes. I would think that the ability to copy just the heading would be useful, but I'm unclear on some org usage such as would this include properties, drawers, keywords or other org-isms. |
|
In markdown I believe the defun target is a subtree? Or is just the heading and the body text before the next section or subsection heading? At any rate, in Markdown you have both heading and "heading+some body text" targets, do you use both? |
|
Yes, in a heading the first target is Heading, then Identifier and then Defun (which is the subtree). I honestly don't use embark much in markdown headings. I'd use T for |
|
@oantolin Are you okay with publishing your embark-org package at some point via MELPA? This could happen before an ELPA release, it just requires a small adjustment in the recipe: https://github.com/melpa/melpa/blob/cbaf88bb3789691a2f733369c271d33b8e959f6b/recipes/embark#L3 |
|
The embark and embark-consult packages are specified separately in ELPA. In order to include embark-org.el in the embark package, the ELPA declarations must be adjusted: https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/elpa-packages#n223
|
|
Two minor comments from me: With an org link whose description contains bold text and with point on this This can be fixed with something like (when-let (((derived-mode-p 'org-mode 'org-agenda-mode))
- (element (org-element-context))
- ((memq (car element) embark-org--types))
+ (element (org-element-lineage (org-element-context) embark-org--types t))
(begin (org-element-property :begin element))Another feature request would be for an ability to cycle through ancestors. For This could also be implemented with the help of org-element-lineage, but
And so on for the grand-parent. Not very pretty and efficient, I know. |
It seems to me that this is a sufficient reason to support target finders which return multiple candidates. Such finders are needed for complex markup with nested elements and potentially also for treesitter in the future. The reason why we didn't add multiple-target finders was that the semantics were not clear. The question back then was if these multiple targets have the same priority and are supposed to be acted on at the same time, which would have then muddied the distinction of embark-act and embark-act-all. But in this case we are talking about something else, a single target finder returning nested targets with decreasing priorities. I don't see a reason why we shouldn't add this, since it is an obvious generalization and more of a technical detail which facilitates the target finder implementation. In contrast, adding a bunch of child, parent, grand-parent, grand-grand-parent finders is neither pretty nor efficient as you wrote. |
|
I've been thinking about allowing target finders and target transformers to return multiple targets for a while anyway. It would even simplify some of the default configuration. It's an easy change, I just didn't want to do it if it wasn't necessary. So far I've made do without it somewhat begrudgingly (I'd like for example that if an Elisp symbol names both a variable and a function to get both targets) but this is another great use case, @jakanakaevangeli. |
|
Note that you do currently get both table cell and whole-table targets you can cycle between. But I do agree it would be great to be able to cycle among all ancestors. |
|
If supporting multiple targets simplifies the default implementation, the change is justified. Looking forward to see if this holds indeed or if Embark will get more complicated. As usual I am happy to see simplification and consolidation. :) |
|
In my mind I separate the implementation of Embark from the default configuration of Embark. I only claimed allowing multiple target would simplify (and even improve) the default configuration. I don't think the change will make much of a difference to the implementation, conceptually a mapcar becomes a mapcan, and that's it (it's slightly more complicated since there is no actual mapcar call). |
Okay, I understand. The implementation will get slightly complicated, but this small change will already be amortized when you manage to simplify only two or three target finders. They belong to the configuration, but Embark overall will still get simpler. In the case of embark-org having these child, parent, grand-parent finders wouldn't be really meaningful, while having multi-target finders is a natural generalization. |
|
In org mode there are quite a few things to do with clipboard content:
some of which are only valid for certain types of clipboard content. Would embark be a good choice to dispatch on that? |
|
@oantolin Btw, what about the embark-org package? When will you make this available? |
|
Good question! There are three things I'd like to do to it right now, of which the first two should probably be done before a release:
The third can wait since it adds new functionality rather than changing existing functionality. Any thoughts on link copying? I keep rethinking it and not finding a super-ergonomic solution. There's already a long complicated comment in the source explaining the current design, but I wish I could do slightly better. |
Anyway I just wondered if there are some truly serious issues. Details can always be sorted out step by step. I expect that we are going to add more and more special actions over time inspired by org-menu. |
|
I went with your suggestion for link copying, after trying it for a couple of days. |
|
Could this be removed from .elpaignore eventually, as (I guess several) users currently have to install it manually? Thanks for considering. |
|
Sure, I'm dragging my feet too much. I'll do it now. @minad also sent me a strong hint that I should do that, in the form of a PR... 🙄 |
|
Yeah, finally! ;) |
|
I guess any further discussion of embark-org can happen in specific issues. |
|
@jakanakaevangeli I decided to go ahead with target finders that can return multiple targets and now you can cycle between targets in the entire org-element-lineage. Thanks for suggesting it a while ago and for telling me about the lineage function. |
|
I've forgotten that we had already discussed the multi target idea, so I wrote some critical comment here: #562 (comment). The arguments given here are all good, but it is still unclear to me if there is an actual simplification thanks to that. The whole table finder could be removed in 5f5b005. Additionally we get a generalized behavior since the ancestor elements are all available now. Which actual Org syntax elements are affected by this? |
Chains of length 3 I often have are link < list item < plain list and link < table cell < table. Before this change you could only target the link! |
It's not simpler than the previous code but I believe it is simpler and more general than code I would have had to write to get this functionality without the multi-valued targets. I think it's OK to have multi-valued targets even if the vast majority of target finders are single-valued. Just like I wound up liking the multi-valued keymap change you proposed above. This new change feels very similar in spirit. |
|
To me there is a difference. The multi-valued keymaps seem like a pure technicality to save you from defining all these intermediate keymaps without any deeper conceptual implications. It is not that I don't see the point of the multi-target generalization. It makes sense, also a transformer from symbol to function+variable. Probably my concerns are unfounded. At least we didn't go with the generalization where a target finder could return multiple targets to be acted on at once. 😅 |
|
I agree it the multi-valued target finder extension has a higher cognitive weight than the multi-keymap extension, but I don't feel it's that much higher, the implementation was pretty straightforward and the two uses, for Org and for Emacs Lisp symbols, seem to me to justify it. I may come to regret this, of course, but for now I think it's worth it. |
I decided to write an embark-org package because I imagine many people have had to resort to writing their own target finders and action keymaps, so it's probably worth having a single coordinated effort. ac79525
The current version just has targets for table cells (which do triple duty as standing in for the entire column or row too), entire tables, and org links. I plan to add more stuff before putting this on GNU ELPA and would love some suggestions for both target types and for actions.
Obviously it's a little weird this initial version doesn't have anything for headings, but it's not that bad because Embark already has generic support for outline headings, we'd just need to add org specific actions (like changing todo keyword, priority or tags, I guess).
(The support for links is maybe a little more complex than you'd expect, so I wrote a rationale for it in comments in the source code, in case you're curious.)
So what targets are missing? I'd say headings and dates have useful actions. What about source blocks?
I'm not against writing some custom actions (I threw in a "copy as markdown" action for links requested long ago by @hmelman, for example), but as usual would love to mainly focus on exisitng org commands.
The text was updated successfully, but these errors were encountered: