Tor-björn Claesson <tclaes...@gmail.com> writes:
> It adds om.el, which contains the start of a simple menu system, and uses
> this to define org-cite-basic-follow.
> Most of the work is done in org-menu-define.
Now a proper high-level review.
> +(org-menu-define org-cite-basic-follow
> + "Follow citation"
> + org-cite-basic-follow-actions
> + :display t
> + :default-action 'org-cite-basic-goto
> + :parameter-types ('citation 'citation-reference))
> ...
> +;;; Main macro definition
> +(cl-defmacro org-menu-define (name
> + docstring
> + contents
> + &key ((:display display) nil)
> + &key ((:default-action default-action) nil)
> + &key ((:parameter-types types) nil))
> + "Define an org menu."
> + (setq org-menu-display (cons `(,name . ,display) org-menu-display))
> + (setq org-menu-default-actions (cons `(,name . ,default-action)
> org-menu-default-actions))
> +
> + `(transient-define-prefix ,name (object &optional prefix)
> + ,docstring
> + [:class transient-columns
> + :setup-children
> ...
> + (interactive
> ...
> + (list (let ((obj (org-element-context)))
I think that we need to step back and define what we want to see in this
new library, because your current version of the code limits the overall
scope of where the org-menu can be used.
I envision the general idea of org-menu commands to:
1. Either display a menu or just call a normal command depending on
prefix argument, org-menu-mode, and user customizations
2. Provide infrastructure to define menus regardless of the display
backend (transient or something else) and without limiting the
command arguments.
I believe that specifically tying things to transient is limiting.
Similarly, hard-coding (object &optional prefix) arguments is limiting -
this will prevent menus from being used outside Org mode and will
severely limit the argument passed.
I imagine the following semantics of org-menu-define:
(org-menu-define menu-name (prefix argument1 argument2 ...) ;; PREFIX argument
mandatory
"DOCSTRING"
:actions <(mandatory) actions or variable holding actions>
:default-action <(optional) action or variable holding default action>
:display <(optional) whether to display full menu by default, may be
customized>
:menu-system <(optional) menu system to be used, may be customized>
:switch <(optional) switch to be used, may be customized>
<(optional) body that may pre-process arguments arbitrarily; the body may
contain
interactive spec as needed (if not, the macro will define one)
)
Then, org-menu-define will:
1. Define a set of variables:
+ <menu-name>-actions (if actions is not already a variable)
+ <menu-name>-default-action (if not already a variable)
+ <menu-name>-display (...)
+ <menu-name>-menu-system (...)
+ <menu-name>-switch (...)
2. Define a command named <menu-name> that will honor
org-menu-mode, keyword options passed, and generally provide unified UI
3. The <menu-name> command will roughly look like
(defun menu-name (args)
"DOCSTRING modified to indicate that is it a menu command"
<either (interactive "P") or interactive spec provided in menu definition>
<menu body if any>
<process actions>
(if ...
<call menu provider>
<call default action>))
4. In the menu specification, rather than hard-coding
!object and !prefix, we allow any argument from the argument list to
be referenced.
We should also not rely upon `transient-scope'. Instead, we should
set things up to pass the argument values to a menu provider and only
use `transient-scope' when the menu provider is transient.
The details in these last 4 points are mostly my brainstorming. We may
discuss specific design further. But I hope that you got my idea about
more generic design less tied to transient and the specifics of
`org-cite-basic-follow'.
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
