Tor-björn Claesson <tor-bj...@claesson.fi> writes:

> This is the most complicated thing I have written, and though I am happy
> to have gotten this far, since I'm no programmer, I suspect there
> is room for improvement, and have not yet spent much time on the
> docstrings (org-cite-basic-follow stands out as needing one) or
> NEWS. Once the design is finalised, I will be happy to do those:-)

Thanks for your efforts!
The docstrings should indeed be improved. Writing the docstrings often
reveals the edge cases missed in the code itself.

> I have attached a new version of the patch with a working prototype. We
> now have a function org-menu-popup, which can be used for
> org-menu-system. To support this we have org-menu--bind-specification
> (which is also used for the transient) and
> org-menu--specifications-to-menu, which builds a menu style keymap from
> a transient specification.

> +        (complete-arglist (if (member 'prefix-argument arglist)
> +                              arglist
> +                            `(,@arglist prefix-argument))))
> ...
> +       (transient-define-prefix
> +        ,name (,@arglist &optional prefix-argument)

This will fail when (1) arglist already contains prefix-argument or (2)
arglist has &optional/&rest of its own.

We should
(1) Alter
(mapcar
              (lambda (arg)
                `(,(intern (concat "!" (symbol-name arg)))
                  (plist-get (transient-scope)
                             ,(intern (concat ":" (symbol-name arg))))))
              ',complete-arglist)

to filter out &optional and &rest

(2) Probably change the way we introduce prefix argument.
For example, instead of fiddling with (interactive ...) and argument
list, we can make use of current-prefix-arg variable to check
org-menu-switch.

> (defun org-menu--specifications-to-menu (description specifications)
>   "Given SPECIFICATIONS (on the form of `org-cite-basic-follow-actions'), 
> return a menu keymap with those bidings.
> 
> The title of this menu is DESCRIPTION."
>   (let ((new-map (make-sparse-keymap description)))
>     (cl-loop
>      for group across specifications
>      do (cl-loop
>          for specification across group
>          do
>          (pcase specification
>            (`(,key ,desc ,fn . ,_)
>             (define-key new-map key `(menu-item ,desc ,fn))))))
>     new-map))

I think that we should also put group names like "Open" or "Copy" as
non-clickable (menu-item ITEM-NAME), maybe even adding a separator below.

Also, here is a version for menu system using tmm menu:

(defun org-menu-tmm-prompt (description specifications)
  "Show an org-menu using a `tmm-prompt'.

This function is a valid value for `org-menu-system'."
  (let ((menu-keymap (org-menu--specifications-to-menu description 
specifications)))
    (tmm-prompt menu-keymap)))

-- 
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>

Reply via email to