Re: Fwd: Org-cite: Replace basic follow-processor with transient menu?

[Ihor Radchenko <yantar92@posteo.net>]

Tor-björn Claesson <tclaesson@gmail.com> writes:

> Ah, then it has to go. Here comes a fixed patch. ...

Thanks.
See my comments below.

> +*** Menu for choosing how to follow citations

Maybe "New transient menu when following citations"

> +Following citations with the org-cite-basic citation backend can now present a
> +transient menu. To show this menu, set ~org-cite-basic-follow-ask~ to non-nil. 
> +This behaviour can be reversed with a negativ prefix.

"-4" prefix. Not negative.

> +This behaviour can be reversed with a negative prefix argument.

Same. "-4".

> +(defcustom org-cite-basic-follow-actions
> +  '[["Open"
> +     ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]]
> +  "Actions in the `org-cite-basic-follow' transient menu.
> +
> +This option uses the same syntax as `transient-define-prefix', see Info node
> +`(transient)Binding Suffix and Infix Commands'.  In addition, it is possible 
> +to specify a function call for the COMMAND part, where !citation and 
> +!prefix can be used to access those values."
> +  :group 'org-cite
> +  :package-version '(Org . "9.8")
> +  :type 'sexp)

1. Ideally, we want at least one more menu entry here. Otherwise, the
   menu is not very useful.
2. It would be nice to provide some examples on using !citation and
   !prefix in the docstring. Also, lambdas.
3. We need to explain what !citation and !prefix refer to.

> +(transient-define-prefix org-cite-basic-follow (citation-object &optional prefix)
> ...
> +This behaviour is inverted when the transient is called with a negative prefix
> +argument.

-4 prefix

> +(defun org-cite-basic-follow--parse-suffix-specification (specification)
> +  "Handle special syntax for `org-cite-basic-follow-actions'."
> +  (pcase specification
> +    (`(,key ,desc (lambda ,args . ,fn-args) . ,other)
> +     `(,key ,desc
> +            (lambda ,args
> +              ,(unless (and (listp (car fn-args))
> +                            (equal (caar fn-args)
> +                                   'interactive))
> +                 '(interactive))
> +              (let ((!citation (car (transient-scope)))
> +                    (!prefix (cadr (transient-scope))))

This can be improved a bit.
Rather than storing transient scope as (list citation prefix), we can
use plist: (list :citation citation :prefix prefix). Then, we can do
(let ((!citation (plist-get (transient-scope) :citation))
      (!prefix (plist-get (transient-scope) :prefix))) ...)

It will be more readable.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>