Tor-björn Claesson <tclaes...@gmail.com> writes:
> Here is my first attempt.
> I have read the commit guidelines, but it is very possible that I have
> misunderstood or just missed something, so I'm grateful for any
> feedback!
Thanks for the patch!
See my comments below.
> * lisp/oc-basic.el (require 'transient): Pull in transient.
Technically, we still support Emacs 27, which does not have
transient. But accounting for this would be a pain and Emacs 30 is
probably going to be released some time around the beginning of the next
year. So, let's not worry about this and accept that Org 9.8-pre (next
release) no longer supports Emacs 27.
If you are a user of Emacs 27 + Org main branch, and have strong
objections, please chime in.
> +*** Menu for choosing how to follow citations
> +If invoked with a prefix of C-- C-u, following citations with
> +the org-cite-basic citation backend will no present a transient menu,
> +offering choices for how to follow citations.
I imagine C-- C-u more as a toggle - if `org-cite-basic-follow-ask' is
t, it disables the menu; enables otherwise.
> +(defcustom org-cite-basic-follow-ask nil
> + "Should `org-cite-basic' ask how to follow citations?
> +
> +When this option is nil, `org-cite-basic-follow' opens the bibliography
> entry.
> +Otherwise, `org-cite-basic-follow' will display a transient menu prompting
> the
> +user for an action. The contents of this menu can be customized in
> +`org-cite-basic-follow-actions'."
Note: our convention is to use double space between sentences.
There are also typos, but that's a minor thing I can fix myself before
merging.
> +(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,
> +!prefix, and !citation-key can be used to access those values.
> +
> +If COMMAND is a lambda form, it can access the citation and prefix like this:
> +
> + (lambda (citation prefix)
> + (interactive (transient-scope))
> + ...)"
Could we make !prefix, and !citation work in lambdas? We should be able
to.
> +(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
> + "Follow citation.
> +
> +This transient is invoked through `org-open-at-point'.
> +When `org-open-at-point' is invoked
It should not matter for this command where it is called from.
You can simply drop references to `org-open-at-point'
We can even make the prefix work as a standalone command. Simply using
interactive spec.
(interactive
(list (let ((obj (org-element-context)))
(pcase (org-element-type obj)
((or citation citation-reference) obj)
(_ (user-error "No citation at point"))))))
> ... with a negative prefix, +or `org-cite-basic-follow-ask' is
> non-nil, it will present +a transient menu prompting the user for an
> action. Otherwise, +it will open the bibliography entry for the
> citation at point.
> +Suffixes can not be added to this transient menu using the ordinary
> +`transient-append-suffix' or `transient-insert-suffix', instead, the
> +contents of the menu are defined in the variable
> +`org-cite-basic-follow-actions'."
Suffixes can be added. Try
(transient-append-suffix 'org-cite-basic-follow nil
'[["Other" ("t" "test" (lambda () (interactive) (message "Hello!")))]])
> +(defun org-cite-basic-follow--parse-suffix-specification (specification)
> + (pcase specification
Please, add the docstring.
> + ((and val `(,key ,desc (,fn . ,fn-args) . ,other))
> + (if (eq fn 'lambda) val
> + `(,key ,desc
> + (lambda ()
> + (interactive)
> + (let ((!citation (car (transient-scope)))
> + (!prefix (cadr (transient-scope)))
> + (!citation-key
> + (org-element-property :key (car (transient-scope)))))
`org-open-at-point' may be called with point at citation rather than
citation reference. Citation object does not have :key property.
I think that we should drop !citation-key spec and instead specify that
the command may be called with citation or citation-reference object in
!citation.
> +(defun org-cite-basic-follow--setup (_)
> + (transient-parse-suffixes
Docstring here as well.
--
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>
