Thanks for looking at this!
Ihor Radchenko <yanta...@posteo.net> writes:
> The setq calls above are evaluated when macro is expanded, not when the
> menu is called.
> Macro expansion may happen during compile time, when loading un-compiled
> library, or when you evaluate the macro expression manually.
> If the expansion happens during compile time, it can only affect the
> Emacs instance that is doing the compilation, but will have 0 effect
> when you use the compiled function later.
> This is likely the reason why you are not seeing the desired effect all
> the time.
>
> Remember that macro is generating the code to be evaluated later. Doing
> computations during expansion is rarely needed, except when you are
> computing some constants to be inlined.
>
> You may probably consider moving setq calls inside `(...), so that it is
> evaluated later, during runtime. Something like `(progn (setq ..)
> (transient-define-prefix ...))
Thanks! I was wondering how to to (begin ...) in elisp=) This still does
not fix the problem for some reason, I will dig some more!
> 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 (...)
Yes, this is much nicer than putting those in org-menu- alists!
> + <menu-name>-menu-system (...)
> + <menu-name>-switch (...)
If we end up with many menus, and I would like to use e.g. embark
everywhere instead of the default, then it would be tedious to change
all of these.
Would it be a good idea to have org-menu-default-menu-system and
org-menu-default-switch, make <menu-name>-menu-system and
<menu-name>-switch be nil by default, and if they are nil use the values
of org-menu-default-menu-system and org-menu-default-switch
respectively?
This way, it would be easy to change the defaults globally, but also
possible to customize them per menu.
> 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.
>
My preliminary plan for this was to keep using transient-define-prefix
to create the <menu-name> command, this way we get the transient "for
free", but change the body to something like
(cond ((<switch logic>) <do default action>)
((<menu system is 'transient>) <set up transient and scope>)
(t (org-menu--show-menu <menu system> <menu-name> args actions)))
Would this be OK?
I am currently a bit swamped with soon upcoming state exams in pediatric
radiology, finishing an article and my doctoral thesis. However, I really enjoy
learning about elisp, appreciate your guidance, and will make sure to
find time for this work (but I will be a bit slow).
Cheers,
Tor-björn
