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

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

[Tor-björn Claesson]

Hello!

I recently switched from org-ref to org-cite, and would like to thank
eveyone who has worked on citation handling in org-mode!
Your work is of incredible value to my research productivity!

Since I use org-roam-ref, I initially went with citar and installed
vertico, marginalia and embark, but this felt a bit invasive,
so I went back to the built in basic processors,
which fill all my needs except for the follow-processor.

To improve following, I made a transient which offers options other than
opening the bibliography entry. This works really well, and can easily
be extended by adding new suffixes.

In order to make the basic follow processor more useful, would you be
interested in replacing it with a transient menu?

As an example, I attach my transient, including examples on extensions.
It would obviously need some work on wording and thought as to what
commands should be made available by default. Also I am not used to
elisp, and the code can probably be improved!

I hope that this example demonstrates how more useful and extensible
the basic citation follower would be in form of a transient menu,
and would be happy to work this into something fit for inclusion
in org-mode, in case you would be interested.

Best regards,
Tor-björn Claesson

(transient-define-prefix tbc/follow-reference (datum &optional _)
  "How should we follow references?"
  [["Open"
    ("b" "bibliography entry"
     (lambda ()
       (interactive)
       (org-cite-basic-goto
        (car  (oref (transient-prefix-object) scope))
        (cadr (oref (transient-prefix-object) scope)))))]
   ["Copy"
    ("c" "citation key"
     (lambda ()
       (interactive)
       (kill-new
        (org-element-property :key (car (oref (transient-prefix-object) scope))))))]]
  (interactive)
  (transient-setup 'tbc/follow-reference nil nil :scope (list 
                                                         datum
                                                         _)))
(org-cite-register-processor 'tbc
  :follow #'tbc/follow-reference)
(setq org-cite-follow-processor 'tbc)

(transient-append-suffix 'tbc/follow-reference "b"
  '("p" "pdf"
    (lambda ()
      (interactive)
      (find-file-other-window
       (concat
        tbc/projektet ; path to my research files
        "Referensartiklar/"
        (org-element-property :key (car (oref (transient-prefix-object) scope)))
        ".pdf")))))

(transient-append-suffix 'tbc/follow-reference "p"
  '("n" "note"
    (lambda ()
      (interactive)
      (orb-edit-note
       (org-element-property
        :key (car  (oref (transient-prefix-object) scope)))))))

;; Adapted from org-ref
(transient-append-suffix 'tbc/follow-reference "c"
  '("d" "DOI"
     (lambda ()
       (interactive)
       (kill-new 
        (save-excursion 
          (with-temp-buffer
            (mapc #'insert-file-contents org-cite-global-bibliography)
            (bibtex-set-dialect (parsebib-find-bibtex-dialect) t)
            (bibtex-search-entry (org-element-property :key (car (oref (transient-prefix-object) scope))))
            (setq doi (bibtex-autokey-get-field "doi"))
            (replace-regexp-in-string "^http://dx.doi.org/" "" doi)))))))

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

[Ihor Radchenko]

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

> Since I use org-roam-ref, I initially went with citar and installed
> vertico, marginalia and embark, but this felt a bit invasive,
> so I went back to the built in basic processors,
> which fill all my needs except for the follow-processor.
>
> To improve following, I made a transient which offers options other than
> opening the bibliography entry. This works really well, and can easily
> be extended by adding new suffixes.
>
> In order to make the basic follow processor more useful, would you be
> interested in replacing it with a transient menu?

I do think that having extended menus for org-open-at-point could be
useful. Not by default, but, for example, with a prefix argument.

> As an example, I attach my transient, including examples on extensions.
> It would obviously need some work on wording and thought as to what
> commands should be made available by default. Also I am not used to
> elisp, and the code can probably be improved!

Your example demonstrates the following options:
1. Plain old opening bibtex entry
2. Copying citation key
3. Opening DOI-derived link in browser
4. Opening PDF (but I am not sure how you want to find the PDF name from
   bibtex record)

I am not sure how useful is copying the citation key, but various extra
menus like opening DOI/ISBN/URL links might be of use.
PDFs might be useful, but it is not clear how to know where such PDF is
located for arbitrary user.

Any other suggestions?
Maybe from
https://github.com/jkitchin/org-ref/blob/fd178abf12a85f8e12005d1df683564bdc534124/org-ref-citation-links.el#L525 ?

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

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

[Tor-björn Claesson]

Hi and thanks for replying!

Ihor Radchenko <yantar92@posteo.net> writes:
>
> I do think that having extended menus for org-open-at-point could be
> useful. Not by default, but, for example, with a prefix argument.
>

This is a good point, but of much larger scope than just replacing the
follower of the basic citation-processor.

>
> Your example demonstrates the following options:
> 1. Plain old opening bibtex entry
> 2. Copying citation key
> 3. Opening DOI-derived link in browser
> 4. Opening PDF (but I am not sure how you want to find the PDF name from
>    bibtex record)
>
> I am not sure how useful is copying the citation key, but various extra
> menus like opening DOI/ISBN/URL links might be of use.
> PDFs might be useful, but it is not clear how to know where such PDF is
> located for arbitrary user.
>
> Any other suggestions?
> Maybe from
> https://github.com/jkitchin/org-ref/blob/fd178abf12a85f8e12005d1df683564bdc534124/org-ref-citation-links.el#L525 ?

Notes and PDF depends heavily on user preferences - and should maybe be
left out for now? org-ref allows customizing org-ref-open-pdf-function,
with a default one using bibtex-completion-find-pdf, which finds a pdf in
bibtex-completion-library-path called citekey with one of the extension
listed in bibtex-completion-pdf-extension.

Maybe it is a good idea to start small, for example provide
1. Open bibtex-entry
2. Copy DOI
3. Opening DOI/ISBN/URL links in browser

Further functionality can easily be added per user, and good solutions
incorporated by default in the future?

I have played some more with this - would it be a good idea to include
macros to get citekey, datum and _? I would be happy to clean this up a
bit, add DOI/ISBN/URL-functionality, documentation and prepare a bug report/patch.

(defmacro org-cite-basic-follow--citekey ()
  '(org-element-property :key (car (oref (transient-prefix-object) scope))))

(defmacro org-cite-basic-follow--datum ()
  '(car  (oref (transient-prefix-object) scope)))

(defmacro org-cite-basic-follow--_ ()
  '(cadr (oref (transient-prefix-object) scope)))

(transient-define-prefix org-cite-basic-follow (datum _)
  "How should we follow references?"
  [["Open"
    ("b" "bibliography entry"
     (lambda ()
       (interactive)
       (org-cite-basic-goto
        (org-cite-basic-follow--datum)
        (org-cite-basic-follow--_))))]
   ["Copy"
    ("d" "DOI"
     (lambda ()
       (interactive)
       (kill-new
        (save-excursion
          (with-temp-buffer
            (mapc #'insert-file-contents org-cite-global-bibliography)
            (bibtex-set-dialect (parsebib-find-bibtex-dialect) t)
            (bibtex-search-entry (org-cite-basic-follow--citekey))
            (setq doi (bibtex-autokey-get-field "doi"))
            (replace-regexp-in-string "^http://dx.doi.org/" "" doi))))))]]
  (interactive)
  (transient-setup 'org-cite-basic-follow nil nil :scope (list 
                                                         datum
                                                         _)))

Best regards,
Tor-björn Claesson

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

[Ihor Radchenko]

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

>> I do think that having extended menus for org-open-at-point could be
>> useful. Not by default, but, for example, with a prefix argument.
>>
> This is a good point, but of much larger scope than just replacing the
> follower of the basic citation-processor.

No problem. But my idea may still be used - without prefix argument,
just move to citation record; with prefix argument - invoke the menu.

Follow processors are provided with prefix argument passed to
`org-open-at-point':

(defun org-cite-basic-goto (datum _)...

The "_" argument is the currently ignored prefix argument.

> Maybe it is a good idea to start small, for example provide
> 1. Open bibtex-entry
> 2. Copy DOI
> 3. Opening DOI/ISBN/URL links in browser
>
> Further functionality can easily be added per user, and good solutions
> incorporated by default in the future?

+1.
Ideally, these options should simply be a customization.

> I have played some more with this - would it be a good idea to include
> macros to get citekey, datum and _? I would be happy to clean this up a
> bit, add DOI/ISBN/URL-functionality, documentation and prepare a bug report/patch.
>
> (defmacro org-cite-basic-follow--citekey ()
>   '(org-element-property :key (car (oref (transient-prefix-object) scope))))
>
> (defmacro org-cite-basic-follow--datum ()
>   '(car  (oref (transient-prefix-object) scope)))
>
> (defmacro org-cite-basic-follow--_ ()
>   '(cadr (oref (transient-prefix-object) scope)))
>
> (transient-define-prefix org-cite-basic-follow (datum _)
>   "How should we follow references?"
>   [["Open"
>     ("b" "bibliography entry"
>      (lambda ()
>        (interactive)
>        (org-cite-basic-goto
>         (org-cite-basic-follow--datum)
>         (org-cite-basic-follow--_))))]

This looks way too complicated.
Is there an easier way to access transient prefix command arguments from
suffixes? Maybe something provided by transient itself?

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

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

[Tor-björn Claesson]

Ihor Radchenko <yantar92@posteo.net> writes:

> Tor-björn Claesson <tclaesson@gmail.com> writes:
>
>>> I do think that having extended menus for org-open-at-point could be
>>> useful. Not by default, but, for example, with a prefix argument.
>>>
>> This is a good point, but of much larger scope than just replacing the
>> follower of the basic citation-processor.
>
> No problem. But my idea may still be used - without prefix argument,
> just move to citation record; with prefix argument - invoke the menu.
>
> Follow processors are provided with prefix argument passed to
> `org-open-at-point':
>
> (defun org-cite-basic-goto (datum _)...
>
> The "_" argument is the currently ignored prefix argument.
>

Ah, I did not understand. Thanks for explaining! This has the nice
effect of not changing the previous behaviour. I think this should be
customizeable.

>> I have played some more with this - would it be a good idea to include
>> macros to get citekey, datum and _? I would be happy to clean this up a
>> bit, add DOI/ISBN/URL-functionality, documentation and prepare a bug report/patch.
>>
>> (defmacro org-cite-basic-follow--citekey ()
>>   '(org-element-property :key (car (oref (transient-prefix-object) scope))))
>>
>> (defmacro org-cite-basic-follow--datum ()
>>   '(car  (oref (transient-prefix-object) scope)))
>>
>> (defmacro org-cite-basic-follow--_ ()
>>   '(cadr (oref (transient-prefix-object) scope)))
>>
>> (transient-define-prefix org-cite-basic-follow (datum _)
>>   "How should we follow references?"
>>   [["Open"
>>     ("b" "bibliography entry"
>>      (lambda ()
>>        (interactive)
>>        (org-cite-basic-goto
>>         (org-cite-basic-follow--datum)
>>         (org-cite-basic-follow--_))))]
>
> This looks way too complicated.
> Is there an easier way to access transient prefix command arguments from
> suffixes? Maybe something provided by transient itself?

Yes it was way to complicated, thanks! A better way can be found reading
transient.el and magit sources. Together with your other feedback, I now have:

(defcustom org-cite-basic-follow-ask nil
  "Should org-cite-basic ask how to follow citations?"
  :group 'org-cite
  :type 'boolean)

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
  "Follow a citation reference.

New actions can be added using transient-append-suffix. 
The body of such new actions should have the form:

(lambda (citation prefix) (interactive (oref (transient-prefix-object) scope)) ...)"
  [["Open"
    ("b" "bibliography entry" org-cite-basic-follow.open-bibliography)]
   ["Copy"
    ("d" "DOI" org-cite-basic-follow.copy-doi)]
   ["Browse"
    ("u" "url" org-cite-basic-follow.browse-url)]]
  (interactive)
  (if (or org-cite-basic-follow-ask prefix)
      (transient-setup 'org-cite-basic-follow nil nil :scope (list citation prefix))
    (org-cite-basic-goto citation prefix)))

(transient-define-suffix org-cite-basic-follow.open-bibliography (citation prefix)
  "Find bibliography entry for citation"
  (interactive (oref (transient-prefix-object) scope))
  (org-cite-basic-goto citation prefix))
  
...

And I can then for example add my own pdf-action like this:

(transient-append-suffix 'org-cite-basic-follow "b"
  '("p" "pdf"
    (lambda (citation prefix)
      (interactive (oref (transient-prefix-object) scope))
      (find-file-other-window
       (concat
        tbc/projektet
        "Referensartiklar"
        "/"
        (org-element-property :key citation)
        ".pdf")))))


Does this start to look ok, and is there something else I should
address if this was to be included in org-mode?

Cheers,
Tor-björn

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

[Ihor Radchenko]

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

>> Is there an easier way to access transient prefix command arguments from
>> suffixes? Maybe something provided by transient itself?
>
> Yes it was way to complicated, thanks! A better way can be found reading
> transient.el and magit sources. Together with your other feedback, I now have:
>
> (defcustom org-cite-basic-follow-ask nil
>   "Should org-cite-basic ask how to follow citations?"
>   :group 'org-cite
>   :type 'boolean)
>
> (transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>   "Follow a citation reference.
>
> New actions can be added using transient-append-suffix. 
> The body of such new actions should have the form:
>
> (lambda (citation prefix) (interactive (oref (transient-prefix-object) scope)) ...)"
>   [["Open"
>     ("b" "bibliography entry" org-cite-basic-follow.open-bibliography)]
>    ["Copy"
>     ("d" "DOI" org-cite-basic-follow.copy-doi)]
>    ["Browse"
>     ("u" "url" org-cite-basic-follow.browse-url)]]
>   (interactive)
>   (if (or org-cite-basic-follow-ask prefix)
>       (transient-setup 'org-cite-basic-follow nil nil :scope (list citation prefix))
>     (org-cite-basic-goto citation prefix)))

Thanks! This looks much more clean.
Even better would be having a defcustom that defines the transient
layout. The idea is to avoid hard-coding [["Open" ... ] ["Copy" ...]
...] and instead make it defcustom.

> (transient-define-suffix org-cite-basic-follow.open-bibliography (citation prefix)
>   "Find bibliography entry for citation"
>   (interactive (oref (transient-prefix-object) scope))
>   (org-cite-basic-goto citation prefix))
>   
> ...

(oref (transient-prefix-object) scope) is equivalent to (transient-scope)

> And I can then for example add my own pdf-action like this:
>
> (transient-append-suffix 'org-cite-basic-follow "b"
>   '("p" "pdf"
>     (lambda (citation prefix)
>       (interactive (oref (transient-prefix-object) scope))
>       (find-file-other-window
>        (concat
>         tbc/projektet
>         "Referensartiklar"
>         "/"
>         (org-element-property :key citation)
>         ".pdf")))))

It feels a bit too complex to demand knowledge of these transient
details (how to get the arglist) from users.

I am wondering if we can somehow plug the existing commands passing the
arguments without any extra setup on the user side.

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

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

[Tor-björn Claesson]

Ihor Radchenko <yantar92@posteo.net> writes:
> Thanks! This looks much more clean.
> Even better would be having a defcustom that defines the transient
> layout. The idea is to avoid hard-coding [["Open" ... ] ["Copy" ...]
> ...] and instead make it defcustom.

Here is a solution that works for me. Is this an OK use of eval, or is
there a better way of doing this?

(defcustom org-cite-basic-follow-actions
  '[["Open"
    ("b" "bibliography entry" org-cite-basic-follow.open-bibliography)]
   ["Copy"
    ("d" "DOI" org-cite-basic-follow.copy-doi)]
   ["Browse"
    ("u" "url" org-cite-basic-follow.browse-url)]]
  "Contents of the org-cite-basic-follow transient menu. 

This can be customized directly using the customization 
interface. Use setopt instead of setq if you change this option 
in elisp, to ensure that the transient is rebuilt.

Further  actions can be added using transient-define-suffix."
  :group 'org-cite
  :type 'sexp
  :set (lambda (option-name new-value)
         (eval
          `(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
             "Follow a citation reference.

The contents of this transient menu is set in org-cite-basic-follow-actions."
             ,new-value
             (interactive)
             (if (or org-cite-basic-follow-ask prefix)
                 (transient-setup 'org-cite-basic-follow nil nil :scope (list citation prefix))
               (org-cite-basic-goto citation prefix))))
         (set-default-toplevel-value option-name new-value)))

>> (transient-define-suffix org-cite-basic-follow.open-bibliography (citation prefix)
>>   "Find bibliography entry for citation"
>>   (interactive (oref (transient-prefix-object) scope))
>>   (org-cite-basic-goto citation prefix))
>>   
>> ...
>
> (oref (transient-prefix-object) scope) is equivalent to (transient-scope)
>

Ah, thanks!

>> And I can then for example add my own pdf-action like this:
>>
>> (transient-append-suffix 'org-cite-basic-follow "b"
>>   '("p" "pdf"
>>     (lambda (citation prefix)
>>       (interactive (oref (transient-prefix-object) scope))
>>       (find-file-other-window
>>        (concat
>>         tbc/projektet
>>         "Referensartiklar"
>>         "/"
>>         (org-element-property :key citation)
>>         ".pdf")))))
>
> It feels a bit too complex to demand knowledge of these transient
> details (how to get the arglist) from users.
>
> I am wondering if we can somehow plug the existing commands passing the
> arguments without any extra setup on the user side.

The lambda form is much neater with your (transient-scope) suggestion:
(lambda (citation prefix)
        (interactive (transient-scope))
        ...)

Is this simple enough? I don't feel a macro would improve the
situation.

Best regards,
Tor-björn

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

[Ihor Radchenko]

[ CCing emacs-devel and the author of transient; maybe we can have some
  more suggestions this way ]

For some context, we are trying to create a customizeable transient menu
with items configured via user option.

We are also trying to pass additional arguments from prefix to suffix
commands in a way that there is no need to write suffix commands
specially just for transient.

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

> Ihor Radchenko <yantar92@posteo.net> writes:
>> Thanks! This looks much more clean.
>> Even better would be having a defcustom that defines the transient
>> layout. The idea is to avoid hard-coding [["Open" ... ] ["Copy" ...]
>> ...] and instead make it defcustom.
>
> Here is a solution that works for me. Is this an OK use of eval, or is
> there a better way of doing this?
>
> (defcustom org-cite-basic-follow-actions
>   '[["Open"
>     ("b" "bibliography entry" org-cite-basic-follow.open-bibliography)]
>    ["Copy"
>     ("d" "DOI" org-cite-basic-follow.copy-doi)]
>    ["Browse"
>     ("u" "url" org-cite-basic-follow.browse-url)]]
>   "Contents of the org-cite-basic-follow transient menu. 
>
> This can be customized directly using the customization 
> interface. Use setopt instead of setq if you change this option 
> in elisp, to ensure that the transient is rebuilt.

+1

> Further  actions can be added using transient-define-suffix."
>   :group 'org-cite
>   :type 'sexp
>   :set (lambda (option-name new-value)
>          (eval
>           `(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>              "Follow a citation reference.
>
> The contents of this transient menu is set in org-cite-basic-follow-actions."
>              ,new-value
>              (interactive)
>              (if (or org-cite-basic-follow-ask prefix)
>                  (transient-setup 'org-cite-basic-follow nil nil :scope (list citation prefix))
>                (org-cite-basic-goto citation prefix))))
>          (set-default-toplevel-value option-name new-value)))

This should work, but maybe Jonas can provide better ideas.

>>> And I can then for example add my own pdf-action like this:
>>>
>>> (transient-append-suffix 'org-cite-basic-follow "b"
>>>   '("p" "pdf"
>>>     (lambda (citation prefix)
>>>       (interactive (oref (transient-prefix-object) scope))
>>>       (find-file-other-window
>>>        (concat
>>>         tbc/projektet
>>>         "Referensartiklar"
>>>         "/"
>>>         (org-element-property :key citation)
>>>         ".pdf")))))
>>
>> It feels a bit too complex to demand knowledge of these transient
>> details (how to get the arglist) from users.
>>
>> I am wondering if we can somehow plug the existing commands passing the
>> arguments without any extra setup on the user side.
>
> The lambda form is much neater with your (transient-scope) suggestion:
> (lambda (citation prefix)
>         (interactive (transient-scope))
>         ...)
>
> Is this simple enough? I don't feel a macro would improve the
> situation.

It is not too bad, but what I really wanted is to reuse an existing
command/function without having to write a tailored interactive
statement. Again, I am hoping to get some insight from emacs-devel.

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

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

[Jonas Bernoulli]

> (defcustom org-cite-basic-follow-actions
>   '[["Open"
>     ("b" "bibliography entry" org-cite-basic-follow.open-bibliography)]
>    ["Copy"
>     ("d" "DOI" org-cite-basic-follow.copy-doi)]
>    ["Browse"
>     ("u" "url" org-cite-basic-follow.browse-url)]]
>   "Contents of the org-cite-basic-follow transient menu. 
>
> This can be customized directly using the customization 
> interface. Use setopt instead of setq if you change this option 
> in elisp, to ensure that the transient is rebuilt.
>
> Further  actions can be added using transient-define-suffix."
>   :group 'org-cite
>   :type 'sexp
>   :set (lambda (option-name new-value)
>          (eval
>           `(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>              "Follow a citation reference.
>
> The contents of this transient menu is set in org-cite-basic-follow-actions."
>              ,new-value
>              (interactive)
>              (if (or org-cite-basic-follow-ask prefix)
>                  (transient-setup 'org-cite-basic-follow nil nil :scope (list citation prefix))
>                (org-cite-basic-goto citation prefix))))
>          (set-default-toplevel-value option-name new-value)))

(defcustom org-cite-basic-follow-actions
  '[["Open"
    ("b" "bibliography entry" org-cite-basic-follow.open-bibliography)]
    ["Copy"
     ("d" "DOI" org-cite-basic-follow.copy-doi)]
    ["Browse"
     ("u" "url" org-cite-basic-follow.browse-url)]]
  ...)

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
  [org-cite-basic-follow-actions]
  (interactive)
  (if (or org-cite-basic-follow-ask prefix)
      (transient-setup 'org-cite-basic-follow nil nil
                       ;; (off-topic) Add \n here ---^
                       :scope (list citation prefix))
    (org-cite-basic-goto citation prefix))))


I use something similar in Forge (forge--lists-group et al.), but
there the purpose is to share groups between different prefixes, not
to make them customizable.

To let users choose what commands to offer in the menu, I would
recommend directing users towards Transient's "layer" mechanism instead
of adding an option.

See [[info:transient#Enabling and Disabling Suffixes]].  To try it enter
any prefix (magit-diff would do) and type "C-x l".  Usage information is
displayed after that.

> :scope (list citation prefix)

Shouldn't that be just be
  :scope citation
and then
  (interactive (list (magit-scope)))
to access it?

>> It feels a bit too complex to demand knowledge of these transient
>> details (how to get the arglist) from users.
>>
>> I am wondering if we can somehow plug the existing commands passing the
>> arguments without any extra setup on the user side.
>
> The lambda form is much neater with your (transient-scope) suggestion:
> (lambda (citation prefix)
>         (interactive (transient-scope))
>         ...)
>
> Is this simple enough? I don't feel a macro would improve the
> situation.

Yes, obviously you have to call transient-scope somewhere.

I haven't seen enough of the commands you want to add as suffixes to
know whether it would make sense, and is even possible, to add an
abstraction, and the few examples I have seen already contain
non-commands and "find as pdf" doesn't even exist as a named function.

That being said, if there are multiple commands like

  (defun do-something-with-citation (citation)
    (interactive (list (read-citation "Do something with citation: ")))
    ...)

it might make sense to change read-citation like this

  (defun read-citation (prompt)
    (if (eq transient-current-prefix 'org-cite-basic-follow)
        (org-element-property :key (transient-scope))
      ...old body here...))

Cheers,
Jonas

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

[Ihor Radchenko]

Jonas Bernoulli <jonas@bernoul.li> writes:

>>   :set (lambda (option-name new-value)
>>          (eval
>>           `(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>>              "Follow a citation reference.
> ...
>
> (transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>   [org-cite-basic-follow-actions]
> ...

This is more compact indeed. Thanks!

> I use something similar in Forge (forge--lists-group et al.), but
> there the purpose is to share groups between different prefixes, not
> to make them customizable.

And this is the problem. See the above :set we have to add in order to
re-evaluate the prefix definition.

It would be nice if the layout could be calculated dynamically rather
than frozen in place in the "defun".

> To let users choose what commands to offer in the menu, I would
> recommend directing users towards Transient's "layer" mechanism instead
> of adding an option.
>
> See [[info:transient#Enabling and Disabling Suffixes]].  To try it enter
> any prefix (magit-diff would do) and type "C-x l".  Usage information is
> displayed after that.

This will indeed help with customizing *pre-existing suffixes*. However,
what we really want here is different - we want a minimal set of
suffixes to be provided by Org mode, and more suffixes to be contributed
by individual packages or by users themselves.

In other words, we need some way to add new suffixes to the
org-cite-basic-follow prefix menu.

>> :scope (list citation prefix)
>
> Shouldn't that be just be
>   :scope citation
> and then
>   (interactive (list (magit-scope)))
> to access it?

We want the [suffix] commands to have information about the prefix argument used
to call `org-cite-basic-follow'.

> Yes, obviously you have to call transient-scope somewhere.
>
> I haven't seen enough of the commands you want to add as suffixes to
> know whether it would make sense, and is even possible, to add an
> abstraction, and the few examples I have seen already contain
> non-commands and "find as pdf" doesn't even exist as a named function.
>
> That being said, if there are multiple commands like
>
>   (defun do-something-with-citation (citation)
>     (interactive (list (read-citation "Do something with citation: ")))
>     ...)
>
> it might make sense to change read-citation like this
>
>   (defun read-citation (prompt)
>     (if (eq transient-current-prefix 'org-cite-basic-follow)
>         (org-element-property :key (transient-scope))
>       ...old body here...))

This is precisely what I want to avoid.

What we want is having normal commands/functions and then allowing to
add them as suffixes without having to change their interactive spec or
source code in general.

Currently, if we want suffix that is calling a function not specially
designed with transient support in mind, we need to do the ugly juggle
like

    (transient-append-suffix 'org-cite-basic-follow "b"
      '("g" "goto"
	(lambda (citation prefix)
	  (interactive (transient-scope))
          (org-cite-basic-goto citation prefix))))

It would be so much nicer to write something simpler like

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
		   "Follow a citation reference.
      The contents of this transient menu is set in org-cite-basic-follow-actions."
		   [["Open"
                     ("b" "bibliography entry" org-cite-basic-goto :args (transient-args))]]
		   (interactive)
		   (if (or org-cite-basic-follow-ask prefix)
                       ;; Imaginary :args slot
		       (transient-setup 'org-cite-basic-follow nil nil :args (list citation prefix))
		     (org-cite-basic-goto citation prefix)))

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

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

[Jonas Bernoulli]

Ihor Radchenko <yantar92@posteo.net> writes:

> Jonas Bernoulli <jonas@bernoul.li> writes:
>
>>>   :set (lambda (option-name new-value)
>>>          (eval
>>>           `(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>>>              "Follow a citation reference.
>> ...
>>
>> (transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>>   [org-cite-basic-follow-actions]
>> ...
>
> This is more compact indeed. Thanks!
>
>> I use something similar in Forge (forge--lists-group et al.), but
>> there the purpose is to share groups between different prefixes, not
>> to make them customizable.
>
> And this is the problem. See the above :set we have to add in order to
> re-evaluate the prefix definition.
>
> It would be nice if the layout could be calculated dynamically rather
> than frozen in place in the "defun".

Look at notmuch-tag-transient from notmuch-transient.el [1].  It does
exactly that.  I.e., the :setup-children slot is what you are looking
for.

[1] https://github.com/tarsius/notmuch-transient

>> To let users choose what commands to offer in the menu, I would
>> recommend directing users towards Transient's "layer" mechanism instead
>> of adding an option.
>>
>> See [[info:transient#Enabling and Disabling Suffixes]].  To try it enter
>> any prefix (magit-diff would do) and type "C-x l".  Usage information is
>> displayed after that.
>
> This will indeed help with customizing *pre-existing suffixes*. However,
> what we really want here is different - we want a minimal set of
> suffixes to be provided by Org mode, and more suffixes to be contributed
> by individual packages or by users themselves.
>
> In other words, we need some way to add new suffixes to the
> org-cite-basic-follow prefix menu.

The recommended way of adding bindings is transient-insert-suffix, but
that won't work here because of the "we need to wrap the command" issue.

>>> :scope (list citation prefix)
>>
>> Shouldn't that be just be
>>   :scope citation
>> and then
>>   (interactive (list (magit-scope)))
>> to access it?
>
> We want the [suffix] commands to have information about the prefix argument used
> to call `org-cite-basic-follow'.

I asked because none of the provided examples actually used that
information (or maybe I just overlooked that).  But sure, if that
information is needed, then that's the way to provide it.

>> Yes, obviously you have to call transient-scope somewhere.
>>
>> I haven't seen enough of the commands you want to add as suffixes to
>> know whether it would make sense, and is even possible, to add an
>> abstraction, and the few examples I have seen already contain
>> non-commands and "find as pdf" doesn't even exist as a named function.
>>
>> That being said, if there are multiple commands like
>>
>>   (defun do-something-with-citation (citation)
>>     (interactive (list (read-citation "Do something with citation: ")))
>>     ...)
>>
>> it might make sense to change read-citation like this
>>
>>   (defun read-citation (prompt)
>>     (if (eq transient-current-prefix 'org-cite-basic-follow)
>>         (org-element-property :key (transient-scope))
>>       ...old body here...))
>
> This is precisely what I want to avoid.

I don't think I got my point across.

I am not saying that you should modify the interactive form of every
command, or each command's individual reader function.

Instead I am saying that IFF "all" of these commands already happen to
use the same reader function, then it would make sense to trivially
address the issue by adding the following two lines to that reader, and
be done with it.

 (defun read-citation (prompt)
+  (if (eq transient-current-prefix 'org-cite-basic-follow)
+      (org-element-property :key (transient-scope)
     ...old body here...))

But that doesn't seem to be the case, so...

> What we want is having normal commands/functions and then allowing to
> add them as suffixes without having to change their interactive spec or
> source code in general.
>
> Currently, if we want suffix that is calling a function not specially
> designed with transient support in mind, we need to do the ugly juggle
> like

(That code was unreadable in my mua, so I am trimming it to the relevant
part.)

> [["Open"
>  ("b" "bibliography entry" org-cite-basic-goto
>   :args (transient-args))]]

I was planning to suggest something vaguely along those lines, after
confirming that the "slightly modify the universally used reader"
approach above, turns out to not be applicable.

Adding a new slot for that makes sense but would need a new class.  See
the `forge--topic-set-state-command' class and the commands that use it,
for an example where I felt that approach makes sense.

But the approach I used for `notmuch-tag-transient' is more applicable
here.  Hm, that actually also adds a new class, which we *might* be able
to avoid here, let's look at the simpler `notmuch-search-transient'
for inspiration.

[Obviously completely untested:]

(defcustom org-cite-basic-follow-actions
  '[["Open"
     ("b" "bibliography entry" org-cite-basic-follow.open-bibliography (...))]
    ["Copy"
     ("d" "DOI" org-cite-basic-follow.copy-doi (...))]
    ["Browse"
     ("u" "url" org-cite-basic-follow.browse-url (...))]]
  ...)

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
  [:class transient-column
   :setup-children org-cite-basic-follow--setup
   :pad-keys t]
  (interactive)
  (if (or org-cite-basic-follow-ask prefix)
      (transient-setup 'org-cite-basic-follow nil nil
                       :scope (list citation prefix))
    (org-cite-basic-goto citation prefix)))

(defun org-cite-basic-follow--setup (_)
  (transient-parse-suffixes
   'notmuch-search-transient
   (mapcar (pcase-lambda (`(,key ,desc ,fn ,transform))
               (list ,key ,desc
                     (lambda ()
                       (interactive)
                       (apply fn (eval transform)))))
           org-cite-basic-follow-actions)))

     Cheers,
     Jonas

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

[Ihor Radchenko]

Jonas Bernoulli <jonas@bernoul.li> writes:

> ...
> But the approach I used for `notmuch-tag-transient' is more applicable
> here.  Hm, that actually also adds a new class, which we *might* be able
> to avoid here, let's look at the simpler `notmuch-search-transient'
> for inspiration.
>
> [Obviously completely untested:]
> ...
> (transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>   [:class transient-column
>    :setup-children org-cite-basic-follow--setup

Thanks!
This seems to be good enough approach.
Tor-björn, does the proposed idea make sense to you?

> (That code was unreadable in my mua, so I am trimming it to the relevant
> part.)

Side node: If your mua is able to display raw message as plain text, it
will have the original formatting without newlines being removed/added.

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

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

[Tor-björn Claesson]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

Hi and thanks for these suggestions! I have some time away from the family
tomorrow evening, and will try to make a new prototype along those lines:)

Getting back!

Cheers/Tor-björn

Den sön 27 okt. 2024 10.07Ihor Radchenko <yantar92@posteo.net> skrev:

> Jonas Bernoulli <jonas@bernoul.li> writes:
>
> > ...
> > But the approach I used for `notmuch-tag-transient' is more applicable
> > here.  Hm, that actually also adds a new class, which we *might* be able
> > to avoid here, let's look at the simpler `notmuch-search-transient'
> > for inspiration.
> >
> > [Obviously completely untested:]
> > ...
> > (transient-define-prefix org-cite-basic-follow (citation &optional
> prefix)
> >   [:class transient-column
> >    :setup-children org-cite-basic-follow--setup
>
> Thanks!
> This seems to be good enough approach.
> Tor-björn, does the proposed idea make sense to you?
>

[-- Attachment #2: Type: text/html, Size: 1471 bytes --]

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

[Tor-björn Claesson]

Hi again!

Den lör 26 okt. 2024 kl 14:45 skrev Jonas Bernoulli <jonas@bernoul.li>:
> [Obviously completely untested:]
>
> (defcustom org-cite-basic-follow-actions
>   '[["Open"
>      ("b" "bibliography entry" org-cite-basic-follow.open-bibliography (...))]
>     ["Copy"
>      ("d" "DOI" org-cite-basic-follow.copy-doi (...))]
>     ["Browse"
>      ("u" "url" org-cite-basic-follow.browse-url (...))]]
>   ...)
>
> (transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>   [:class transient-column
>    :setup-children org-cite-basic-follow--setup
>    :pad-keys t]
>   (interactive)
>   (if (or org-cite-basic-follow-ask prefix)
>       (transient-setup 'org-cite-basic-follow nil nil
>                        :scope (list citation prefix))
>     (org-cite-basic-goto citation prefix)))
>
> (defun org-cite-basic-follow--setup (_)
>   (transient-parse-suffixes
>    'notmuch-search-transient
>    (mapcar (pcase-lambda (`(,key ,desc ,fn ,transform))
>                (list ,key ,desc
>                      (lambda ()
>                        (interactive)
>                        (apply fn (eval transform)))))
>            org-cite-basic-follow-actions)))
>
>      Cheers,
>      Jonas

I didn't get this working yesterday, but it looks really nice from my
non-technical user point of view! Thanks!

I will keep trying, but I must find the spare time to learn more about
mapping and pattern matching in elisp, so this might take a while. In
case Ihor wants to just fix it, please go ahead :-)

It would be good if we could match against the case of '(key desc
suffix) as well, so that we could include otherwhere defined suffixes.

Cheers,
Tor-björn

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

[Ihor Radchenko]

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

> ...
>>    (mapcar (pcase-lambda (`(,key ,desc ,fn ,transform))
>>                (list ,key ,desc
>>                      (lambda ()
>>                        (interactive)
>>                        (apply fn (eval transform)))))
>>            org-cite-basic-follow-actions)))
>
> ...
> I will keep trying, but I must find the spare time to learn more about
> mapping and pattern matching in elisp, so this might take a while. In
> case Ihor wants to just fix it, please go ahead :-)

Check "11.4.4 Destructuring with ‘pcase’ Patterns" section of Elisp
manual. It has examples explaining how backquote pattern works.

> It would be good if we could match against the case of '(key desc
> suffix) as well, so that we could include otherwhere defined suffixes.

To not complicate things, I suggest something simple along the lines of

(lambda (&rest suffix-spec)
 (pcase suffix-spec
  (`(,key ,desc ,fn ,transform) ...)
  (`(,key ,desc ,suffix) ...)))

Remember that you can always play around with things in scratch buffer,
in IELM, or even just using C-x C-e from anywhere:

(pcase '(1 2 3 4)
  (`(,key ,desc ,fn ,transform)
    (format "key: %s; desc: %s; fn:%s; transform: %s"
       key  desc fn transform))
  (`(,key ,desc ,suffix)
    (format "key: %s; desc: %s; suffix:%s"
       key desc suffix)))
;; => "key: 1; desc: 2; fn:3; transform: 4"

(pcase '(1 2 3)
  (`(,key ,desc ,fn ,transform)
    (format "key: %s; desc: %s; fn:%s; transform: %s"
       key  desc fn transform))
  (`(,key ,desc ,suffix)
    (format "key: %s; desc: %s; suffix:%s"
       key desc suffix)))
;; => "key: 1; desc: 2; suffix:3"

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

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

[Tor-björn Claesson]

This is a great learning experience, thank you Ihor and Jonas for the
friendly feedback!

(defcustom org-cite-basic-follow-actions
  '[["Open"
     ("b" "bibliography entry" org-cite-basic-goto (list (transient-scope) 0))]    
    ["Copy"
     ("d" "DOI" org-cite-basic-follow.copy-doi)]
    ["Browse"
     ("u" "url" org-cite-basic-follow.browse-url)]]
  "Hepp"
  :group 'org-cite
  :type 'sexp)

The bibliography entry demonstrates using any function (a bit ugly since
org-cite-basic-goto insists on the prefix argument).

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
  [:class transient-columns
   :setup-children org-cite-basic-follow--setup
   :pad-keys t]
  (interactive)
  (if org-cite-basic-follow-ask
      (transient-setup 'org-cite-basic-follow nil nil
                       :scope (list citation))
    (org-cite-basic-goto citation prefix)))

Here I removed activating the transient menu if a prefix argument is
present. If org-cite-basic-follow-ask is nil (the default) and I want
to call org-cite-basic-goto with a prefix argument, this would cause the
transient menu to activate, which seems wrong.

(defun org-cite-basic-follow--setup (_)
  (transient-parse-suffixes
   'org-cite-basic-follow
   (cl-map 'vector (lambda (group)
          (cl-map 'vector (lambda (entry)
                    (pcase entry
                      ((and (pred stringp) label)
                       label)
                      (`(,key ,desc ,fn ,transform)
                       (list key desc `(lambda ()
                                         (interactive)
                                         (apply ',fn ,transform))))
                      (`(,key ,desc ,suffix)
                       (list key desc suffix))))
                  group))
        org-cite-basic-follow-actions)))

Prefixes of :class transient-columns expect a vector of vectors of a string and
lists, so we need to use a map that produces vectors.

This works. Do the code look OK to you? I'll prepare a patch if that is the
case.

Cheers,
Tor-björn

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

[Ihor Radchenko]

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

> (defcustom org-cite-basic-follow-actions
>   '[["Open"
>      ("b" "bibliography entry" org-cite-basic-goto (list (transient-scope) 0))]    
>     ["Copy"
>      ("d" "DOI" org-cite-basic-follow.copy-doi)]
>     ["Browse"
>      ("u" "url" org-cite-basic-follow.browse-url)]]
>   "Hepp"
>   :group 'org-cite
>   :type 'sexp)

Since we can now allow pretty much a custom suffix definition, what
about adding a bit of syntax sugar to the format.

Rather than doing

   ("b" "bibliography entry" org-cite-basic-goto (list (transient-scope) 0))

we can allow

   ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))

where !citation and !prefix can be pattern-matched and automatically
replaced with the appropriate values.

> (transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>   [:class transient-columns
>    :setup-children org-cite-basic-follow--setup
>    :pad-keys t]
>   (interactive)
>   (if org-cite-basic-follow-ask
>       (transient-setup 'org-cite-basic-follow nil nil
>                        :scope (list citation))
>     (org-cite-basic-goto citation prefix)))
>
> Here I removed activating the transient menu if a prefix argument is
> present. If org-cite-basic-follow-ask is nil (the default) and I want
> to call org-cite-basic-goto with a prefix argument, this would cause the
> transient menu to activate, which seems wrong.

I would still prefer some way to force transient version of the command
in some way. I see it very useful for some people to set the proposed
`org-cite-basic-follow-ask' to nil _most of the time_, but occasionally
switch to transient version for non-standard operations.

Maybe we can utilize some other rarely used prefix argument to force
transient menu.

Say, something like C-- C-u <key sequence>. This will make prefix
argument have a value of '(-4).

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

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

[Tor-björn Claesson]

Good ideas!

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
  [:class transient-columns
   :setup-children org-cite-basic-follow--setup
   :pad-keys t]
  (interactive)
  (if (or org-cite-basic-follow-ask
          (eq prefix '(-4)))
      (transient-setup 'org-cite-basic-follow nil nil
                       :scope (list citation prefix))
    (org-cite-basic-goto citation prefix)))

This works nicely!

(defun org-cite-basic-follow--setup (_)
  (transient-parse-suffixes
   'org-cite-basic-follow
   (cl-map 'vector (lambda (group)
          (cl-map 'vector (lambda (suffix-spec)
                    (pcase suffix-spec
                      ((and (pred stringp) label)
                       label)
                      (`(,key ,desc (,fn !citation !prefix))
                       (list key desc `(lambda ()
                                         (interactive)
                                         (letrec ((scope (transient-scope))
                                                  (citation (car scope))
                                                  (prefix (cadr scope)))
                                             (,fn citation prefix)))))
                      (`(,key ,desc ,fn ,transform)
                       (list key desc `(lambda ()
                                         (interactive)
                                         (apply ',fn ,transform))))
                      (`(,key ,desc ,suffix)
                       (list key desc suffix))))
                  group))
        org-cite-basic-follow-actions)))

This too works really well! I think it would make sence to match (,fn
!citation), (,fn !citation-key) and (,fn !citation-key !prefix), does
this sound reasonable?

Thanks again for taking the time to help me with this=)

Cheers,
Tor-björn

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

[Ihor Radchenko]

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

> (defun org-cite-basic-follow--setup (_)
>   (transient-parse-suffixes
>    'org-cite-basic-follow
>    (cl-map 'vector (lambda (group)
>           (cl-map 'vector (lambda (suffix-spec)

This is getting complex enough that you can split out this lambda into a
dedicated private function.

>                     (pcase suffix-spec
>                       ((and (pred stringp) label)
>                        label)
>                       (`(,key ,desc (,fn !citation !prefix))

I think we can do yet better:

  `(,key ,desc (,fn . ,fn-args) . ,other)

Here, `(,a . ,b) is matching a list with 1+ elements, with b
collecting the cdr of the list.

Once we have fn-args, we can simply map over it and search for
'!citation or '!prefix symbols, replacing them with appropriate
values.

The final return value will be (using backquote-style list syntax:
10.4 Backquote in Elisp manual):

  `(,key ,desc (,fn ,@processed-args) ,@other)

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

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

[Tor-björn Claesson]

Thanks!

Here is another take=)

(defcustom org-cite-basic-follow-actions
  '[["Open"
     ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]    
    ["Copy"
     ("d" "DOI" org-cite-basic-follow.copy-doi)]
    ["Browse"
     ("u" "url" org-cite-basic-follow.browse-url)]]
  "Hepp"
  :group 'org-cite
  :type 'sexp)

(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
  [:class transient-columns
          :setup-children org-cite-basic-follow--setup
          :pad-keys t]
  (interactive)
  (if (or org-cite-basic-follow-ask
          (eq prefix '(-4)))
      (transient-setup 'org-cite-basic-follow nil nil
                       :scope (list citation prefix))
    (org-cite-basic-goto citation prefix)))

(defun org-cite-basic-follow--parse-suffix-specification (specification)
  (pcase specification
    ((and (pred stringp) label)
     label)
    (`(,key ,desc (,fn . ,fn-args) . ,other)
     (let ((function-args
            (mapcar
             (lambda (arg)
               (pcase arg
                 ('!citation
                  '(car (transient-scope)))
                 ('!prefix
                  '(cadr (transient-scope)))
                 ('!citation-key
                  '(org-element-property :key (car (transient-scope))))))
             fn-args)))
       `(,key ,desc
              (lambda ()
                (interactive)
                (,fn ,@function-args))
              ,other)))
    (`(,key ,desc ,suffix)
     (list key desc suffix))))

(defun org-cite-basic-follow--setup (_)
  (transient-parse-suffixes
   'org-cite-basic-follow
   (cl-map 'vector
           (lambda (group)
             (cl-map 'vector
                     #'org-cite-basic-follow--parse-suffix-specification
                     group))
           org-cite-basic-follow-actions)))

Cheers,
Tor-björn

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

[Tor-björn Claesson]

But that was not good enough - we do not cover the case of specifying
a lambda in org-cite-basic-follow-actions,
or passing other arguments to the function than citation, prefix or
citation key.

This updated version fixes this, so the action can be either
1. a suffix (as in transient-define-suffix)
2. a lambda form (as in (lambda (citation prefix) (interactive
(transient-scope)) ...))
3. a function call, which will be wrapped in the ugly dance-lambda and
where !citation, !prefix, and !citation-key
    will be (recursively) substituted but other arguments preserved.

(defun org-cite-basic-follow--process-function-arguments (arguments)
  (cond ((null arguments)
         '())
        ((atom (car arguments))
         (cons
          (pcase (car arguments)
            ('!citation
             '(car (transient-scope)))
            ('!prefix
             '(cadr (transient-scope)))
            ('!citation-key
             '(org-element-property :key (car (transient-scope))))
            (argument
             argument))
          (org-cite-basic-follow--process-function-arguments (cdr arguments))))
        (t
         (cons
          (org-cite-basic-follow--process-function-arguments (car arguments))
          (org-cite-basic-follow--process-function-arguments (cdr
arguments))))))

(defun org-cite-basic-follow--parse-suffix-specification (specification)
  (pcase specification
    ((and (pred stringp) label)
     label)
    (`(,key ,desc (lambda . ,fn-args) . ,other)
     (list key desc `(lambda ,@fn-args) ,other))
    (`(,key ,desc (,fn . ,fn-args) . ,other)
     (let ((function-args
            (org-cite-basic-follow--process-function-arguments
             fn-args)))
       `(,key ,desc
              (lambda ()
                (interactive)
                (,fn ,@function-args))
              ,other)))
    (`(,key ,desc ,suffix)
     (list key desc suffix))))

(defun org-cite-basic-follow--setup (_)
  (transient-parse-suffixes
   'org-cite-basic-follow
   (cl-map 'vector
           (lambda (group)
             (cl-map 'vector #'org-cite-basic-follow--parse-suffix-specification
                     group))
           org-cite-basic-follow-actions)))

Cheers!
Tor-björn

Den tors 31 okt. 2024 kl 22:48 skrev Tor-björn Claesson <tclaesson@gmail.com>:
>
> Thanks!
>
> Here is another take=)
>
> (defcustom org-cite-basic-follow-actions
>   '[["Open"
>      ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]
>     ["Copy"
>      ("d" "DOI" org-cite-basic-follow.copy-doi)]
>     ["Browse"
>      ("u" "url" org-cite-basic-follow.browse-url)]]
>   "Hepp"
>   :group 'org-cite
>   :type 'sexp)
>
> (transient-define-prefix org-cite-basic-follow (citation &optional prefix)
>   [:class transient-columns
>           :setup-children org-cite-basic-follow--setup
>           :pad-keys t]
>   (interactive)
>   (if (or org-cite-basic-follow-ask
>           (eq prefix '(-4)))
>       (transient-setup 'org-cite-basic-follow nil nil
>                        :scope (list citation prefix))
>     (org-cite-basic-goto citation prefix)))
>
> (defun org-cite-basic-follow--parse-suffix-specification (specification)
>   (pcase specification
>     ((and (pred stringp) label)
>      label)
>     (`(,key ,desc (,fn . ,fn-args) . ,other)
>      (let ((function-args
>             (mapcar
>              (lambda (arg)
>                (pcase arg
>                  ('!citation
>                   '(car (transient-scope)))
>                  ('!prefix
>                   '(cadr (transient-scope)))
>                  ('!citation-key
>                   '(org-element-property :key (car (transient-scope))))))
>              fn-args)))
>        `(,key ,desc
>               (lambda ()
>                 (interactive)
>                 (,fn ,@function-args))
>               ,other)))
>     (`(,key ,desc ,suffix)
>      (list key desc suffix))))
>
> (defun org-cite-basic-follow--setup (_)
>   (transient-parse-suffixes
>    'org-cite-basic-follow
>    (cl-map 'vector
>            (lambda (group)
>              (cl-map 'vector
>                      #'org-cite-basic-follow--parse-suffix-specification
>                      group))
>            org-cite-basic-follow-actions)))
>
> Cheers,
> Tor-björn

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

[Ihor Radchenko]

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

> But that was not good enough - we do not cover the case of specifying
> a lambda in org-cite-basic-follow-actions,
> or passing other arguments to the function than citation, prefix or
> citation key.
>
> This updated version fixes this, so the action can be either
> 1. a suffix (as in transient-define-suffix)
> 2. a lambda form (as in (lambda (citation prefix) (interactive
> (transient-scope)) ...))
> 3. a function call, which will be wrapped in the ugly dance-lambda and
> where !citation, !prefix, and !citation-key
>     will be (recursively) substituted but other arguments preserved.

Rather than going into recursive replacements, simply let-bind
!citation, !prefix, and anything else we may want to provide around the
lambda/function call.

>   (pcase specification
>     ((and (pred stringp) label)
>      label)
>     (`(,key ,desc (lambda . ,fn-args) . ,other)
>      (list key desc `(lambda ,@fn-args) ,other))
> ...
>     (`(,key ,desc ,suffix)
>      (list key desc suffix))))

All these special cases can be simply handled using

 (other other) ; if not a pattern we care about, just retain it unchanged

pcase clause.

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

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

[Tor-björn Claesson]

Ihor Radchenko <yantar92@posteo.net> writes:
> Rather than going into recursive replacements, simply let-bind
> !citation, !prefix, and anything else we may want to provide around the
> lambda/function call.

Clever! I had to put the let inside the lambda for it to work.

(defun org-cite-basic-follow--parse-suffix-specification (specification)
  (pcase specification
    (`(,key ,desc (lambda . ,fn-args) . ,other)
     (list key desc `(lambda ,@fn-args) ,other))
    (`(,key ,desc (,fn . ,fn-args) . ,other)
     `(,key ,desc
            (lambda ()
              (interactive)
              (let ((!citation (car (transient-scope)))
                    (!prefix (cadr (transient-scope)))
                    (!citation-key (org-element-property :key (car (transient-scope)))))
                (,fn ,@fn-args)))
            ,other))
    (other other)))

Does it make sense to keep matching the lambda separately? This just
keeps getting better=)

Thanks and cheers!
Tor-björn

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

[Ihor Radchenko]

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

> Clever! I had to put the let inside the lambda for it to work.

You probably do not have to once you use lexical binding (that is - not
C-x C-e ad-hoc, but put things into actual byte-compiled file)

But let inside the lambda body is perfectly fine.

> (defun org-cite-basic-follow--parse-suffix-specification (specification)
>   (pcase specification
>     (`(,key ,desc (lambda . ,fn-args) . ,other)
>      (list key desc `(lambda ,@fn-args) ,other))
>     (`(,key ,desc (,fn . ,fn-args) . ,other)
>      `(,key ,desc
>             (lambda ()
>               (interactive)
>               (let ((!citation (car (transient-scope)))
>                     (!prefix (cadr (transient-scope)))
>                     (!citation-key (org-element-property :key (car (transient-scope)))))
>                 (,fn ,@fn-args)))
>             ,other))
>     (other other)))
>
> Does it make sense to keep matching the lambda separately? This just
> keeps getting better=)

AFIU, you need to match against lambda simply to avoid the next clause
matching it. If so, you can change the clause to match all ,fn, except
lambda like the following:

`(,key
  ,desc
  (,(and fn (guard (not (eq fn 'lambda))))
   . ,fn-args)
  . ,other)

This is getting ugly though.
An alternative would be simply

(defun org-cite-basic-follow--parse-suffix-specification (specification)
  (pcase specification
    ((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)))))
                  (,fn ,@fn-args)))
              ,@other)))
    (other other)))

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

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

[Tor-björn Claesson]

Ihor Radchenko <yantar92@posteo.net> writes:
> AFIU, you need to match against lambda simply to avoid the next clause
> matching it.

Exactly.

> If so, you can change the clause to match all ,fn, except
> lambda like the following:
>
> `(,key
>   ,desc
>   (,(and fn (guard (not (eq fn 'lambda))))
>    . ,fn-args)
>   . ,other)
>
> This is getting ugly though.
> An alternative would be simply
>
> (defun org-cite-basic-follow--parse-suffix-specification (specification)
>   (pcase specification
>     ((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)))))
>                   (,fn ,@fn-args)))
>               ,@other)))
>     (other other)))

I feel that the guard option does the right thing by directly fixing
the pattern matching - but what approach do you prefer?

Is this starting to be a good time for me to produce a patch?

Cheers,
Tor-björn

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

[Ihor Radchenko]

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

> I feel that the guard option does the right thing by directly fixing
> the pattern matching - but what approach do you prefer?

I provided the guard example just for your reference.
The preference for actual code is more readable code.
IMHO, my second variant with (and val ...) is more readable.

> Is this starting to be a good time for me to produce a patch?

Yes. Thanks in advance!

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

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

[Tor-björn Claesson]

Ihor Radchenko <yantar92@posteo.net> writes:

> Tor-björn Claesson <tclaesson@gmail.com> writes:
>
>> I feel that the guard option does the right thing by directly fixing
>> the pattern matching - but what approach do you prefer?
>
> I provided the guard example just for your reference.
> The preference for actual code is more readable code.
> IMHO, my second variant with (and val ...) is more readable.
>

A, thanks, I will use that then!

>> Is this starting to be a good time for me to produce a patch?
>
> Yes. Thanks in advance!

Would it make sense to add two functions for getting the doi and url
from a citation? That would be generally useful, and simplify the
transient menu specification.

Also, is this change big enough that I should apply for copyright
assignment?

Cheers,
Tor-björn

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

[Ihor Radchenko]

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

>>> Is this starting to be a good time for me to produce a patch?
>>
>> Yes. Thanks in advance!
>
> Would it make sense to add two functions for getting the doi and url
> from a citation? That would be generally useful, and simplify the
> transient menu specification.

Do you mean opening citation URL derived from URL/DOI fields in browser?

> Also, is this change big enough that I should apply for copyright
> assignment?

FSF threshold for "tiny" changes is 15 lines of code.
I doubt that you can fit the changes within 15 lines, so yes, you do
need copyright assignment. See
https://orgmode.org/worg/org-contribute.html#copyright

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

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

[Tor-björn Claesson]

Den lör 9 nov. 2024 kl 16:07 skrev Ihor Radchenko <yantar92@posteo.net>:
> > Would it make sense to add two functions for getting the doi and url
> > from a citation? That would be generally useful, and simplify the
> > transient menu specification.
>
> Do you mean opening citation URL derived from URL/DOI fields in browser?

No, that I think belongs in the transient suffix. Rather I meant a
function to get the
DOI from a citation. I thought about this, and would like to only
include the open
bibliography entry in the patch, and look into other options later.
Would this be ok?

>
> FSF threshold for "tiny" changes is 15 lines of code.
> I doubt that you can fit the changes within 15 lines, so yes, you do
> need copyright assignment. See
> https://orgmode.org/worg/org-contribute.html#copyright

Ok, thanks!

Cheers
Tor-björn

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

[Ihor Radchenko]

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

> ... I thought about this, and would like to only
> include the open
> bibliography entry in the patch, and look into other options later.
> Would this be ok?

Yup.

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

[Tor-björn Claesson]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

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!

Cheers,
Tor-björn

Den sön 10 nov. 2024 kl 18:40 skrev Ihor Radchenko <yantar92@posteo.net>:
>
> Tor-björn Claesson <tclaesson@gmail.com> writes:
>
> > ... I thought about this, and would like to only
> > include the open
> > bibliography entry in the patch, and look into other options later.
> > Would this be ok?
>
> Yup.

[-- Attachment #2: 0001-lisp-oc-basic.el-Transient-menu-for-following-citati.patch --]
[-- Type: text/x-patch, Size: 7625 bytes --]

From c8731ecd5db2beecc434a65448a7302212aab95a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tor-bj=C3=B6rn=20Claesson?= <tclaesson@gmail.com>
>
Date: Mon, 11 Nov 2024 11:46:32 +0200
Subject: [PATCH] lisp/oc-basic.el: Transient menu for following citations

* lisp/oc-basic.el (require 'transient): Pull in transient.
(org-cite-basic-follow-ask): New customization option. should
`org-cite-basic-follow' should prompt the user for an action?
(org-cite-basic-follow-actions): New customization option, that
specifies the contents of the transient menu.
(org-cite-basic-follow): New function.  Displays a menu asking how to
follow a citation if called with a negative prefix, or
`org-cite-basic-follow-ask' is non-nil. Otherwise, it retains the
default behaviour of opening the bibliography entry.
(org-cite-basic-follow--parse-suffix-specification,
org-cite-basic-follow--setup): Helper functions for
`org-cite-basic-follow'.
(org-cite-register-processor 'basic): Update the basic citation
processor to follow citations using `org-cite-basic-follow'.

* etc/ORG_NEWS (Menu for choosing how to follow citations): Describe
the new feature
(New option ~org-cite-basic-follow-ask~): Describe this new
customization option.
(New option ~org-cite-basic-follow-actions~): Describe this new
customization option, which specifies the layaout of the
`org-cite-basic-follow' transient menu.

This change was co-authored with much support from Ihor Radchenko and
Jonas Bernoulli, thanks!
---
 etc/ORG-NEWS     | 22 +++++++++++++
 lisp/oc-basic.el | 84 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index de4f11b25..cf87e11bf 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -114,6 +114,17 @@ The keybindings in the repeat-maps can be changed by customizing
 
 See the new [[info:org#Repeating commands]["Repeating commands"]] section in Org mode manual.
 
+*** 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.
+
+The contents of this menu can be customized in
+~org-cite-basic-follow-actions~.
+
+In order to always show this menu, set ~org-cite-basic-follow-ask~
+to non-nil.
+
 ** New and changed options
 
 # Chanes deadling with changing default values of customizations,
@@ -158,6 +169,17 @@ English.  The default value is ~t~ as the CSL standard assumes that
 English titles are specified in sentence-case but the bibtex
 bibliography format requires them to be written in title-case.
 
+*** New option ~org-cite-basic-follow-ask~
+When this option is non-nil, following a citation with the basic citation
+backend will present a transient menu with choices for how to follow the
+citation.
+If nil, following a citation will open its bibliography entry.
+
+*** New option ~org-cite-basic-follow-actions~
+This option specifies the options presented by ~org-cite-basic-follow~
+if it is invoked with a C-- C-u prefix or ~org-cite-basic-follow-ask~
+is non-nil.
+
 ** New functions and changes in function arguments
 
 # This also includes changes in function behavior from Elisp perspective.
diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
index e207a1997..8add329b7 100644
--- a/lisp/oc-basic.el
+++ b/lisp/oc-basic.el
@@ -74,6 +74,7 @@
 (require 'map)
 (require 'oc)
 (require 'seq)
+(require 'transient)
 
 (declare-function org-open-at-point "org" (&optional arg))
 (declare-function org-open-file "org" (path &optional in-emacs line search))
@@ -140,6 +141,36 @@
   :type 'face
   :safe #'facep)
 
+(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'."
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'boolean)
+
+(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))
+     ...)"
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'sexp)
+
 \f
 ;;; Internal variables
 (defvar org-cite-basic--bibliography-cache nil
@@ -832,6 +863,53 @@ present in the citation."
        (bibtex-set-dialect)
        (bibtex-search-entry key)))))
 
+(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 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'."
+  [:class transient-columns
+          :setup-children org-cite-basic-follow--setup
+          :pad-keys t]
+  (interactive)
+  (if (or org-cite-basic-follow-ask
+          (equal prefix '(-4)))
+      (transient-setup 'org-cite-basic-follow nil nil
+                       :scope (list citation prefix))
+    (org-cite-basic-goto citation prefix)))
+
+(defun org-cite-basic-follow--parse-suffix-specification (specification)
+  (pcase specification
+    ((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)))))
+                  (,fn ,@fn-args)))
+              ,@other)))
+    (other other)))
+
+(defun org-cite-basic-follow--setup (_)
+  (transient-parse-suffixes
+   'org-cite-basic-follow
+   (cl-map 'vector
+           (lambda (group)
+             (cl-map 'vector #'org-cite-basic-follow--parse-suffix-specification
+                     group))
+           org-cite-basic-follow-actions)))
+
 \f
 ;;; "Insert" capability
 (defun org-cite-basic--complete-style (_)
@@ -920,9 +998,9 @@ Raise an error when no bibliography is set in the buffer."
   :activate #'org-cite-basic-activate
   :export-citation #'org-cite-basic-export-citation
   :export-bibliography #'org-cite-basic-export-bibliography
-  :follow #'org-cite-basic-goto
-  :insert (org-cite-make-insert-processor #'org-cite-basic--complete-key
-                                          #'org-cite-basic--complete-style)
+  :follow #'org-cite-basic-follow
+  :insert  (org-cite-make-insert-processor #'org-cite-basic--complete-key
+                                           #'org-cite-basic--complete-style)
   :cite-styles
   '((("author" "a") ("caps" "c"))
     (("noauthor" "na") ("bare" "b"))
-- 
2.45.2

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

[Ihor Radchenko]

Tor-björn Claesson <tclaesson@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>

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

[Tor-björn Claesson]

[-- Attachment #1: Type: text/plain, Size: 4756 bytes --]

Ihor Radchenko <yantar92@posteo.net> writes:
> See my comments below.
Thanks for taking the time to look at this!

>> +*** 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.
That is more useful, thanks!

>
>> +(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.

Ah, bummer. I have fixed (at least some) of those typos.

>> +(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.

Sure! It took me some trial and error to get the list splicing right but
now it works in lambdas to. I added another helper to do let binding.

>> +(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"))))))

I was not able to get this to work.

>> ... 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!")))]])

Ah, I didn't know you could nil the position.

>> +(defun org-cite-basic-follow--parse-suffix-specification (specification)
>> +  (pcase specification
>
> Please, add the docstring.
>

Ok.

>> +    ((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.

`org-cite-basic-goto' handles this by prompting the user for a key, if
it is called with a citation object. I adopted this approach. (I find the
!citation-key useful for a lot of things I like to do to citations, and
would like to keep it.)

>> +(defun org-cite-basic-follow--setup (_)
>> +  (transient-parse-suffixes
>
> Docstring here as well.

Ok.

Version 2 of the patch is attached.

Cheers,
Tor-björn


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: transient-follower-version2 --]
[-- Type: text/x-diff, Size: 8075 bytes --]

From e91fff9f6c03e2c766dee410f46382398f883997 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tor-bj=C3=B6rn=20Claesson?= <tclaesson@gmail.com>
Date: Tue, 12 Nov 2024 11:09:16 +0200
Subject: [PATCH] lisp/oc-basic.el: Transient menu for following citations

* lisp/oc-basic.el (require 'transient): Pull in transient.
(org-cite-basic-follow-ask): New customization option. should
`org-cite-basic-follow' prompt the user for an action?
(org-cite-basic-follow-actions): New customization option, that
specifies the contents of the transient menu.
(org-cite-basic-follow): New function.  Displays a menu asking how to
follow a citation if `org-cite-basic-follow-ask' is
non-nil. Otherwise, it retains the default behaviour of opening the
bibliography entry. This can be inversed with a negative prefix argument.
(org-cite-basic-follow--parse-suffix-specification,
org-cite-basic-follow--wrap-function, and
org-cite-basic-follow--setup): Helper functions for
`org-cite-basic-follow'.
(org-cite-register-processor 'basic): Update the basic citation
processor to follow citations using `org-cite-basic-follow'.

* etc/ORG_NEWS (Menu for choosing how to follow citations): Describe
the new feature
(New option ~org-cite-basic-follow-ask~): Describe this new
customization option.
(New option ~org-cite-basic-follow-actions~): Describe this new
customization option, which specifies the layout of the
`org-cite-basic-follow' transient menu.

This change was co-authored with much support from Ihor Radchenko and
Jonas Bernoulli, thanks!
---
 etc/ORG-NEWS     | 22 +++++++++++
 lisp/oc-basic.el | 95 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index de4f11b25..b859b0ada 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -114,6 +114,15 @@ The keybindings in the repeat-maps can be changed by customizing
 
 See the new [[info:org#Repeating commands]["Repeating commands"]] section in Org mode manual.
 
+*** Menu for choosing how to follow 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.
+
+The contents of this menu can be customized in
+~org-cite-basic-follow-actions~.
+
 ** New and changed options
 
 # Chanes deadling with changing default values of customizations,
@@ -158,6 +167,19 @@ English.  The default value is ~t~ as the CSL standard assumes that
 English titles are specified in sentence-case but the bibtex
 bibliography format requires them to be written in title-case.
 
+*** New option ~org-cite-basic-follow-ask~
+
+When this option is non-nil, following a citation with the basic citation
+backend will present a transient menu with choices for how to follow the
+citation.
+If nil, following a citation will open its bibliography entry.
+
+This behaviour can be reversed with a negative prefix argument.
+
+*** New option ~org-cite-basic-follow-actions~
+
+This option specifies the options presented by ~org-cite-basic-follow~.
+
 ** New functions and changes in function arguments
 
 # This also includes changes in function behavior from Elisp perspective.
diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
index e207a1997..c3fc3f34d 100644
--- a/lisp/oc-basic.el
+++ b/lisp/oc-basic.el
@@ -74,6 +74,7 @@
 (require 'map)
 (require 'oc)
 (require 'seq)
+(require 'transient)
 
 (declare-function org-open-at-point "org" (&optional arg))
 (declare-function org-open-file "org" (path &optional in-emacs line search))
@@ -140,6 +141,30 @@
   :type 'face
   :safe #'facep)
 
+(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'."
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'boolean)
+
+(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."
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'sexp)
+
 \f
 ;;; Internal variables
 (defvar org-cite-basic--bibliography-cache nil
@@ -832,6 +857,74 @@ present in the citation."
        (bibtex-set-dialect)
        (bibtex-search-entry key)))))
 
+(transient-define-prefix org-cite-basic-follow (citation &optional prefix)
+  "Follow citation.
+
+If `org-cite-basic-follow-ask' is non-nil, this transient will present
+a menu prompting the user for an action. 
+Otherwise, it will open the bibliography entry for the citation at point.  
+This behaviour is inverted when the transient is called with a negative prefix
+argument.
+
+The contents of the menu are defined in the variable
+`org-cite-basic-follow-actions'."
+  [:class transient-columns
+          :setup-children org-cite-basic-follow--setup
+          :pad-keys t]
+  (interactive)
+  (if (xor org-cite-basic-follow-ask
+           (equal prefix '(-4)))
+      (transient-setup 'org-cite-basic-follow nil nil
+                       :scope (list citation prefix))
+    (org-cite-basic-goto citation prefix)))
+
+(defun org-cite-basic-follow--wrap-function (fn-args &optional fn)
+  "Provide !citation, !prefix, and !citation-key symbols."
+  (append 
+   `(let ((!citation (car (transient-scope)))
+          (!prefix (cadr (transient-scope)))
+          (!citation-key
+           (if (org-element-type-p (car (transient-scope)) 'citation-reference)
+               (org-element-property :key (car (transient-scope)))
+             (pcase (org-cite-get-references (car (transient-scope)) t)
+               (`(,key) key)
+               (keys
+                (or (completing-read "Select citation key: " keys nil t)
+                    (user-error "Aborted"))))))))
+   (if fn
+       `((,fn ,@fn-args))
+     fn-args)))
+
+(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))
+              ,(org-cite-basic-follow--wrap-function fn-args))
+            ,@other))
+    (`(,key ,desc (,fn . ,fn-args) . ,other)
+     `(,key ,desc
+            (lambda ()
+	      (interactive)
+	      ,(org-cite-basic-follow--wrap-function fn-args fn))
+            ,@other))
+    (other other)))
+
+(defun org-cite-basic-follow--setup (_)
+  "Update `org-cite-basic-follow' when `org-cite-basic-follow-actions' changes."
+  (transient-parse-suffixes
+   'org-cite-basic-follow
+   (cl-map 'vector
+           (lambda (group)
+             (cl-map 'vector #'org-cite-basic-follow--parse-suffix-specification
+                     group))
+           org-cite-basic-follow-actions)))
+
 \f
 ;;; "Insert" capability
 (defun org-cite-basic--complete-style (_)
@@ -920,7 +1013,7 @@ Raise an error when no bibliography is set in the buffer."
   :activate #'org-cite-basic-activate
   :export-citation #'org-cite-basic-export-citation
   :export-bibliography #'org-cite-basic-export-bibliography
-  :follow #'org-cite-basic-goto
+  :follow #'org-cite-basic-follow
   :insert (org-cite-make-insert-processor #'org-cite-basic--complete-key
                                           #'org-cite-basic--complete-style)
   :cite-styles
-- 
2.46.0

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

[Ihor Radchenko]

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

>> Could we make !prefix, and !citation work in lambdas? We should be able
>> to.
>
> Sure! It took me some trial and error to get the list splicing right but
> now it works in lambdas to. I added another helper to do let binding.

I think that a simple

(let ((!prefix ...) ...)
 (lambda ...))

will work.

But you should test without C-x C-e. Need to reload the whole library.

>> 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"))))))
>
> I was not able to get this to work.

Just replace the interactive spec with my version and then
M-x org-cite-basic-follow with point on citation in Org document.

>> `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.
>
> `org-cite-basic-goto' handles this by prompting the user for a key, if
> it is called with a citation object. I adopted this approach. (I find the
> !citation-key useful for a lot of things I like to do to citations, and
> would like to keep it.)

I am not sure if it is a good idea.
Commands in org-cite-basic-follow-actions may or may not need it, while
your code will _aways_ prompt user about citation key; even when the
citation key is never used.

If you realy, really want it, we can go into `cl-symbol-macrolet' and
lazy evaluation, but will be tricky (especially arranging for
(setq !citation-key ...) to work.

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

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

[Tor-björn Claesson]

[-- Attachment #1.1: Type: text/plain, Size: 1513 bytes --]

Ihor Radchenko <yantar92@posteo.net> writes:
> Tor-björn Claesson <tclaesson@gmail.com> writes:
>
>> Den tis 12 nov. 2024 kl 20:02 skrev Ihor Radchenko <yantar92@posteo.net>:
>>>
>>> I am not sure if it is a good idea.
>>> Commands in org-cite-basic-follow-actions may or may not need it, while
>>> your code will _aways_ prompt user about citation key; even when the
>>> citation key is never used.
>>>
>>> If you realy, really want it, we can go into `cl-symbol-macrolet' and
>>> lazy evaluation, but will be tricky (especially arranging for
>>> (setq !citation-key ...) to work.
>>>
>>
>> Wouldn't my beginner approach with recursive replacement fix this
problem?
>
> Nope. Mindlessly replacing instances of !citation-key with value may
> break the code. Consider, for example,
>
> (lambda ()
>   (let ((citation! (concat citation! "-foo"))) ...))
>
> There will be more complex cases as well.
>
> `cl-symbol-macrolet' it trying to handle what you tried with recursive
> replacement, but more carefully. But even `cl-symbol-macrolet' fails in
> certain edge cases.

Ah, then it has to go. Here comes a fixed patch. The code is much
simpler like this, but i kept the let under the lambda because for some
reason it did not work for me. Also, the interactive clause in the
transient,
while working, upset make test, but this could be fixed by requiring
org-element and quoting citation and citation-reference.

Thanks for taking the time to explain!

Cheers,
Tor-björn

[-- Attachment #1.2: Type: text/html, Size: 2088 bytes --]

[-- Attachment #2: 0001-lisp-oc-basic.el-Transient-menu-for-following-v3.patch --]
[-- Type: application/x-patch, Size: 7955 bytes --]

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

[Ihor Radchenko]

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>

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

[Tor-björn Claesson]

[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]

Thank you for taking the time to look at this!

Ihor Radchenko <yantar92@posteo.net> writes:
>> +(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.

1. The infrastructure is very useful in itself, but I have added an
option to add the DOI to the kill ring, it seems innocent enough. I had
first thought that I would like to, as a next step, add a helper
function to try really hard to get a citation key from a citation, but
lets go with this.

2 and 3. I tried to describe this in the new version of the patch.

>> +(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.
Neat! Thanks!

Please find attached version 4 of the patch.

Cheers,
Tor-björn


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: oc-basic transient follower version 4 --]
[-- Type: text/x-diff, Size: 8452 bytes --]

From c66af1e878f90e7b2cd89a613fc72da177cc6529 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tor-bj=C3=B6rn=20Claesson?= <torb@barbar.claesson.fi>
Date: Tue, 12 Nov 2024 11:09:16 +0200
Subject: [PATCH] lisp/oc-basic.el: Transient menu for following citations

* lisp/oc-basic.el (require 'transient): Pull in transient.
(require 'org-element): Pull in org-element.
(org-cite-basic-follow-ask): New customization option. should
`org-cite-basic-follow' prompt the user for an action?
(org-cite-basic-follow-actions): New customization option, that
specifies the contents of the transient menu.
(org-cite-basic-follow): New function.  Displays a menu asking how to
follow a citation if `org-cite-basic-follow-ask' is
non-nil. Otherwise, it retains the default behaviour of opening the
bibliography entry. This can be inversed with a negative prefix argument.
(org-cite-basic-follow--parse-suffix-specification and
org-cite-basic-follow--setup): Helper functions for
`org-cite-basic-follow'.
(org-cite-register-processor 'basic): Update the basic citation
processor to follow citations using `org-cite-basic-follow'.

* etc/ORG_NEWS (Menu for choosing how to follow citations): Describe
the new feature
(New option ~org-cite-basic-follow-ask~): Describe this new
customization option.
(New option ~org-cite-basic-follow-actions~): Describe this new
customization option, which specifies the layout of the
`org-cite-basic-follow' transient menu.

This change was co-authored with much support from Ihor Radchenko and
Jonas Bernoulli, thanks!
---
 etc/ORG-NEWS     |  22 +++++++++++
 lisp/oc-basic.el | 100 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index de4f11b25..bacc38be2 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -114,6 +114,15 @@ The keybindings in the repeat-maps can be changed by customizing
 
 See the new [[info:org#Repeating commands]["Repeating commands"]] section in Org mode manual.
 
+*** 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 -4 prefix.
+
+The contents of this menu can be customized in
+~org-cite-basic-follow-actions~.
+
 ** New and changed options
 
 # Chanes deadling with changing default values of customizations,
@@ -158,6 +167,19 @@ English.  The default value is ~t~ as the CSL standard assumes that
 English titles are specified in sentence-case but the bibtex
 bibliography format requires them to be written in title-case.
 
+*** New option ~org-cite-basic-follow-ask~
+
+When this option is non-nil, following a citation with the basic citation
+backend will present a transient menu with choices for how to follow the
+citation.
+If nil, following a citation will open its bibliography entry.
+
+This behaviour can be reversed with a -4 prefix argument.
+
+*** New option ~org-cite-basic-follow-actions~
+
+This option specifies the options presented by ~org-cite-basic-follow~.
+
 ** New functions and changes in function arguments
 
 # This also includes changes in function behavior from Elisp perspective.
diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
index e207a1997..c42f95489 100644
--- a/lisp/oc-basic.el
+++ b/lisp/oc-basic.el
@@ -74,6 +74,8 @@
 (require 'map)
 (require 'oc)
 (require 'seq)
+(require 'transient)
+(require 'org-element)
 
 (declare-function org-open-at-point "org" (&optional arg))
 (declare-function org-open-file "org" (path &optional in-emacs line search))
@@ -140,6 +142,43 @@
   :type 'face
   :safe #'facep)
 
+(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'."
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'boolean)
+
+(defcustom org-cite-basic-follow-actions
+  '[["Open"
+     ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]
+    ["Copy"
+     ("d" "DOI"
+      (kill-new
+       (save-excursion
+         (with-temp-buffer
+           (mapc #'insert-file-contents org-cite-global-bibliography)
+           (bibtex-set-dialect (parsebib-find-bibtex-dialect) t)
+           (bibtex-search-entry (org-element-property :key !citation))
+           (setq doi (bibtex-autokey-get-field "doi"))
+           (replace-regexp-in-string "^http://dx.doi.org/" "" doi)))))]]
+  "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 (the citation
+object to be followed) and !prefix (any prefix argument to the follower) can be
+used to access those values. For example:
+(org-cite-basic-goto !citation !prefix) or
+(lambda () (message (org-element-property :key !citation)))"
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'sexp)
+
 \f
 ;;; Internal variables
 (defvar org-cite-basic--bibliography-cache nil
@@ -832,6 +871,65 @@ present in the citation."
        (bibtex-set-dialect)
        (bibtex-search-entry key)))))
 
+(transient-define-prefix org-cite-basic-follow (citation-object &optional prefix)
+  "Follow citation.
+
+If `org-cite-basic-follow-ask' is non-nil, this transient will present
+a menu prompting the user for an action. 
+Otherwise, it will open the bibliography entry for the citation at point.  
+This behaviour is inverted when the transient is called with a -4 prefix
+argument.
+
+The contents of the menu are defined in the variable
+`org-cite-basic-follow-actions'."
+  [:class transient-columns
+          :setup-children org-cite-basic-follow--setup
+          :pad-keys t]
+  (interactive
+    (list (let ((obj (org-element-context)))
+           (pcase (org-element-type obj)
+             ((or 'citation 'citation-reference) obj)
+             (_ (user-error "No citation at point"))))))
+  (if (xor org-cite-basic-follow-ask
+           (equal prefix '(-4)))
+      (transient-setup 'org-cite-basic-follow nil nil
+                       :scope (list :citation citation-object :prefix prefix))
+    (org-cite-basic-goto citation-object 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 (plist-get (transient-scope) :citation))
+                    (!prefix (plist-get (transient-scope) :prefix)))
+                ,@fn-args))
+            ,@other))
+    (`(,key ,desc (,fn . ,fn-args) . ,other)
+     `(,key ,desc
+            (lambda ()
+	      (interactive)
+              (let ((!citation (plist-get (transient-scope) :citation))
+                    (!prefix (plist-get (transient-scope) :prefix)))
+	        (,fn ,@fn-args)))
+            ,@other))
+    (other other)))
+
+(defun org-cite-basic-follow--setup (_)
+  "Update `org-cite-basic-follow' when `org-cite-basic-follow-actions' changes."
+  (transient-parse-suffixes
+   'org-cite-basic-follow
+   (cl-map 'vector
+           (lambda (group)
+             (cl-map 'vector #'org-cite-basic-follow--parse-suffix-specification
+                     group))
+           org-cite-basic-follow-actions)))
+
 \f
 ;;; "Insert" capability
 (defun org-cite-basic--complete-style (_)
@@ -920,7 +1018,7 @@ Raise an error when no bibliography is set in the buffer."
   :activate #'org-cite-basic-activate
   :export-citation #'org-cite-basic-export-citation
   :export-bibliography #'org-cite-basic-export-bibliography
-  :follow #'org-cite-basic-goto
+  :follow #'org-cite-basic-follow
   :insert (org-cite-make-insert-processor #'org-cite-basic--complete-key
                                           #'org-cite-basic--complete-style)
   :cite-styles
-- 
2.46.0

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

[Ihor Radchenko]

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

>> 1. Ideally, we want at least one more menu entry here. Otherwise, the
>>    menu is not very useful.
> ...
> 1. The infrastructure is very useful in itself, but I have added an
> option to add the DOI to the kill ring, it seems innocent enough. I had
> first thought that I would like to, as a next step, add a helper
> function to try really hard to get a citation key from a citation, but
> lets go with this.

Thanks!

> +(defcustom org-cite-basic-follow-actions
> +  '[["Open"
> +     ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]
> +    ["Copy"
> +     ("d" "DOI"
> +      (kill-new
> +       (save-excursion
> +         (with-temp-buffer
> +           (mapc #'insert-file-contents org-cite-global-bibliography)
> +           (bibtex-set-dialect (parsebib-find-bibtex-dialect) t)
> +           (bibtex-search-entry (org-element-property :key !citation))
> +           (setq doi (bibtex-autokey-get-field "doi"))
> +           (replace-regexp-in-string "^http://dx.doi.org/" "" doi)))))]]
> +  "Actions in the `org-cite-basic-follow' transient menu.

Two comments here:
1. It should not be a lambda. Better have a function or even command
2. The logic is not right. You should better follow
   `org-cite-basic--key-completion-table':
   `org-cite-basic--parse-bibliography', `org-cite-basic--get-entry',
   and `org-cite-basic--get-field'.

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

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

[Tor-björn Claesson]

Ihor Radchenko <yantar92@posteo.net> writes:
>> +(defcustom org-cite-basic-follow-actions
>> +  '[["Open"
>> +     ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]
>> +    ["Copy"
>> +     ("d" "DOI"
>> +      (kill-new
>> +       (save-excursion
>> +         (with-temp-buffer
>> +           (mapc #'insert-file-contents org-cite-global-bibliography)
>> +           (bibtex-set-dialect (parsebib-find-bibtex-dialect) t)
>> +           (bibtex-search-entry (org-element-property :key !citation))
>> +           (setq doi (bibtex-autokey-get-field "doi"))
>> +           (replace-regexp-in-string "^http://dx.doi.org/" "" doi)))))]]
>> +  "Actions in the `org-cite-basic-follow' transient menu.
>
> Two comments here:
> 1. It should not be a lambda. Better have a function or even command
> 2. The logic is not right. You should better follow
>    `org-cite-basic--key-completion-table':
>    `org-cite-basic--parse-bibliography', `org-cite-basic--get-entry',
>    and `org-cite-basic--get-field'.

Thanks for taking the time to look at this. This was originally stolen
from org-ref, but can be greatly simplified using
org-cite-basic--get-field.

I am not sure how to go about getting the citation key from the
citation. In the attached patch I introduce a org-cite-basic--get-key
helper function, which takes the behaviour of org-cite-basic-goto to
extract citation keys from a citation-or-citation-reference.

The other option is of course to just use
(org-element-property :key !citation).

Which approach would be better?

Cheers,
Tor-björn

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

[Tor-björn Claesson]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

> Two comments here:
> 1. It should not be a lambda. Better have a function or even command
> 2. The logic is not right. You should better follow
>    `org-cite-basic--key-completion-table':
>    `org-cite-basic--parse-bibliography', `org-cite-basic--get-entry',
>    and `org-cite-basic--get-field'.

And this time with a patch, apologies.

/Tor-björn


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch version 5 --]
[-- Type: text/x-diff, Size: 9808 bytes --]

From 7e9e0c64fbda2dcb67d8c8421d1c9923ca93e8b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tor-bj=C3=B6rn=20Claesson?= <torb@barbar.claesson.fi>
Date: Tue, 12 Nov 2024 11:09:16 +0200
Subject: [PATCH] lisp/oc-basic.el: Transient menu for following citations

* lisp/oc-basic.el (require 'transient): Pull in transient.
(require 'org-element): Pull in org-element.
(org-cite-basic-follow-ask): New customization option. should
`org-cite-basic-follow' prompt the user for an action?
(org-cite-basic-follow-actions): New customization option, that
specifies the contents of the transient menu.
(org-cite-basic--get-key): New function. Get citation key from
citation or citation reference.
(org-cite-basic-follow): New function.  Displays a menu asking how to
follow a citation if `org-cite-basic-follow-ask' is
non-nil. Otherwise, it retains the default behaviour of opening the
bibliography entry. This can be inversed with a negative prefix argument.
(org-cite-basic-follow--parse-suffix-specification and
org-cite-basic-follow--setup): Helper functions for
`org-cite-basic-follow'.
(org-cite-register-processor 'basic): Update the basic citation
processor to follow citations using `org-cite-basic-follow'.

* etc/ORG_NEWS (Menu for choosing how to follow citations): Describe
the new feature
(New option ~org-cite-basic-follow-ask~): Describe this new
customization option.
(New option ~org-cite-basic-follow-actions~): Describe this new
customization option, which specifies the layout of the
`org-cite-basic-follow' transient menu.

This change was co-authored with much support from Ihor Radchenko and
Jonas Bernoulli, thanks!
---
 etc/ORG-NEWS     |  22 +++++++++
 lisp/oc-basic.el | 115 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 128 insertions(+), 9 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index de4f11b25..bacc38be2 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -114,6 +114,15 @@ The keybindings in the repeat-maps can be changed by customizing
 
 See the new [[info:org#Repeating commands]["Repeating commands"]] section in Org mode manual.
 
+*** 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 -4 prefix.
+
+The contents of this menu can be customized in
+~org-cite-basic-follow-actions~.
+
 ** New and changed options
 
 # Chanes deadling with changing default values of customizations,
@@ -158,6 +167,19 @@ English.  The default value is ~t~ as the CSL standard assumes that
 English titles are specified in sentence-case but the bibtex
 bibliography format requires them to be written in title-case.
 
+*** New option ~org-cite-basic-follow-ask~
+
+When this option is non-nil, following a citation with the basic citation
+backend will present a transient menu with choices for how to follow the
+citation.
+If nil, following a citation will open its bibliography entry.
+
+This behaviour can be reversed with a -4 prefix argument.
+
+*** New option ~org-cite-basic-follow-actions~
+
+This option specifies the options presented by ~org-cite-basic-follow~.
+
 ** New functions and changes in function arguments
 
 # This also includes changes in function behavior from Elisp perspective.
diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
index e207a1997..fc55d3a32 100644
--- a/lisp/oc-basic.el
+++ b/lisp/oc-basic.el
@@ -74,6 +74,8 @@
 (require 'map)
 (require 'oc)
 (require 'seq)
+(require 'transient)
+(require 'org-element)
 
 (declare-function org-open-at-point "org" (&optional arg))
 (declare-function org-open-file "org" (path &optional in-emacs line search))
@@ -140,6 +142,39 @@
   :type 'face
   :safe #'facep)
 
+(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'."
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'boolean)
+
+(defcustom org-cite-basic-follow-actions
+  '[["Open"
+     ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]
+    ["Copy"
+     ("d" "DOI"
+      (kill-new
+       (org-cite-basic--get-field
+        'doi
+        (org-cite-basic--get-key !citation))))]]
+  "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 (the citation
+object to be followed) and !prefix (any prefix argument to the follower) can be
+used to access those values. For example:
+(org-cite-basic-goto !citation !prefix) or
+(lambda () (message (org-element-property :key !citation)))"
+  :group 'org-cite
+  :package-version '(Org . "9.8")
+  :type 'sexp)
+
 \f
 ;;; Internal variables
 (defvar org-cite-basic--bibliography-cache nil
@@ -326,6 +361,16 @@ INFO is the export state, as a property list."
                 (map-keys entries))
               (org-cite-basic--parse-bibliography)))
 
+(defun org-cite-basic--get-key (citation-or-citation-reference)
+  "Return citation key for CITATION."
+  (if (org-element-type-p citation-or-citation-reference 'citation-reference)
+      (org-element-property :key citation-or-citation-reference)
+    (pcase (org-cite-get-references citation-or-citation-reference t)
+      (`(,key) key)
+      (keys
+       (or (completing-read "Select citation key: " keys nil t)
+           (user-error "Aborted"))))))
+
 (defun org-cite-basic--get-entry (key &optional info)
   "Return BibTeX entry for KEY, as an association list.
 When non-nil, INFO is the export state, as a property list."
@@ -805,14 +850,7 @@ export state, as a property list."
 When DATUM is a citation reference, open bibliography entry referencing
 the citation key.  Otherwise, select which key to follow among all keys
 present in the citation."
-  (let* ((key
-          (if (org-element-type-p datum 'citation-reference)
-              (org-element-property :key datum)
-            (pcase (org-cite-get-references datum t)
-              (`(,key) key)
-              (keys
-               (or (completing-read "Select citation key: " keys nil t)
-                   (user-error "Aborted"))))))
+  (let* ((key (org-cite-basic--get-key datum))
          (file
           (pcase (seq-find (pcase-lambda (`(,_ . ,entries))
                              (gethash key entries))
@@ -832,6 +870,65 @@ present in the citation."
        (bibtex-set-dialect)
        (bibtex-search-entry key)))))
 
+(transient-define-prefix org-cite-basic-follow (citation-object &optional prefix)
+  "Follow citation.
+
+If `org-cite-basic-follow-ask' is non-nil, this transient will present
+a menu prompting the user for an action. 
+Otherwise, it will open the bibliography entry for the citation at point.  
+This behaviour is inverted when the transient is called with a -4 prefix
+argument.
+
+The contents of the menu are defined in the variable
+`org-cite-basic-follow-actions'."
+  [:class transient-columns
+          :setup-children org-cite-basic-follow--setup
+          :pad-keys t]
+  (interactive
+    (list (let ((obj (org-element-context)))
+           (pcase (org-element-type obj)
+             ((or 'citation 'citation-reference) obj)
+             (_ (user-error "No citation at point"))))))
+  (if (xor org-cite-basic-follow-ask
+           (equal prefix '(-4)))
+      (transient-setup 'org-cite-basic-follow nil nil
+                       :scope (list :citation citation-object :prefix prefix))
+    (org-cite-basic-goto citation-object 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 (plist-get (transient-scope) :citation))
+                    (!prefix (plist-get (transient-scope) :prefix)))
+                ,@fn-args))
+            ,@other))
+    (`(,key ,desc (,fn . ,fn-args) . ,other)
+     `(,key ,desc
+            (lambda ()
+	      (interactive)
+              (let ((!citation (plist-get (transient-scope) :citation))
+                    (!prefix (plist-get (transient-scope) :prefix)))
+	        (,fn ,@fn-args)))
+            ,@other))
+    (other other)))
+
+(defun org-cite-basic-follow--setup (_)
+  "Update `org-cite-basic-follow' when `org-cite-basic-follow-actions' changes."
+  (transient-parse-suffixes
+   'org-cite-basic-follow
+   (cl-map 'vector
+           (lambda (group)
+             (cl-map 'vector #'org-cite-basic-follow--parse-suffix-specification
+                     group))
+           org-cite-basic-follow-actions)))
+
 \f
 ;;; "Insert" capability
 (defun org-cite-basic--complete-style (_)
@@ -920,7 +1017,7 @@ Raise an error when no bibliography is set in the buffer."
   :activate #'org-cite-basic-activate
   :export-citation #'org-cite-basic-export-citation
   :export-bibliography #'org-cite-basic-export-bibliography
-  :follow #'org-cite-basic-goto
+  :follow #'org-cite-basic-follow
   :insert (org-cite-make-insert-processor #'org-cite-basic--complete-key
                                           #'org-cite-basic--complete-style)
   :cite-styles
-- 
2.46.0

[RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark? (was: Fwd: Org-cite: Replace basic follow-processor with transient menu?)

[Ihor Radchenko]

TL;DR: We are in the process of designing a more unified selection
interface for Org mode and want to see if there is some way to unify
context-menu-mode, transient, which-key and embark together. The idea is
to (1) avoid too many customizations; (2) allow users to decide how to
choose between multiple options - by mouse, keyboard, and using
customizable UIs.

Dear all,

I have raised the topic of refactoring Org mode menu systems during
EmacsConf in https://emacsconf.org/2024/talks/org-update/
The initial idea was replacing the self-written menu code in Org with
built-in transient.el.

Later, during OrgMeetup a number of people raised concerns that
transient may sometimes be an overkill, and that some people may prefer
alternative UIs. In particular, embark and context-menu-mode were
mentioned.

(I am CCing the discussion participants and potentially interested
maintainers)

In Org mode (although not only in Org mode, looking at the success of
embark.el), we often have a UI model where users call an "action"
command (like org-ctrl-c-ctrl-c or org-open-at-point) followed by
interactive selection among multiple actions.

For example, org-open-at-point on a heading with multiple links inside
will raise a section buffer listing all the links - pressing a number
will open the corresponding link.

Another example (see the example patch below), which is a
work-in-progress patch for Org citation system, is "following" a
citation. To "follow" citation may mean multiple things including but
not limited to: (1) going to citation record in the bibliography file;
(2) following URL; (3) downloading .pdf file for a citation; etc.
 **The list of "follow" actions may be customized by users**

The general UI flow in these scenarios will be:

1. User calls "action" with cursor at certain syntax element
2. Action menu is displayed, showing the available actions/targets (dynamically built)
3. User selects the action/target

This UI flow can be implemented using context menus, which-key popups,
transient menus, and also using embark (where the way menu is displayed
can be customized).

All the 4 approaches represent different UI models with various
strengths and weaknesses:

- transient has a very flexible layout builder where the menu items can
  be arranged granularly, but intercepts the main loop disrupting
  certain keyboard-based workflows
- which-key does not stand on the way and integrates well into Emacs'
  key binding model, but provides little flexibility for menu layout
- embark stays in the middle between which-key and transient, making use
  of transient keymaps and allowing a custom menu renderer
- context-menu-mode provides mouse experience

I am wondering if we can work out some universal API to plug the
described action->menu->selection model into the UI that user prefers.

Tentatively, I am thinking about the following:

For a given Emacs "prefix" command (e.g. org-open-at-point), we define a
set of customizations:

1. List of possible actions: ((name1 . action1 props) (name2 . action2 ...) ...)
   PROPS is a plist defining extra properties like key-binding, display
   string, maybe something else to be used in the future.
2. Menu interface to use (transient, context-menu, embark, which-key)
3. Layout settings for the specific interfaces. For example, transient
   layout definition.

WDYT?

Best,
Ihor

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

> From 7e9e0c64fbda2dcb67d8c8421d1c9923ca93e8b4 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tor-bj=C3=B6rn=20Claesson?= <torb@barbar.claesson.fi>
> Date: Tue, 12 Nov 2024 11:09:16 +0200
> Subject: [PATCH] lisp/oc-basic.el: Transient menu for following citations
>
> * lisp/oc-basic.el (require 'transient): Pull in transient.
> (require 'org-element): Pull in org-element.
> (org-cite-basic-follow-ask): New customization option. should
> `org-cite-basic-follow' prompt the user for an action?
> (org-cite-basic-follow-actions): New customization option, that
> specifies the contents of the transient menu.
> (org-cite-basic--get-key): New function. Get citation key from
> citation or citation reference.
> (org-cite-basic-follow): New function.  Displays a menu asking how to
> follow a citation if `org-cite-basic-follow-ask' is
> non-nil. Otherwise, it retains the default behaviour of opening the
> bibliography entry. This can be inversed with a negative prefix argument.
> (org-cite-basic-follow--parse-suffix-specification and
> org-cite-basic-follow--setup): Helper functions for
> `org-cite-basic-follow'.
> (org-cite-register-processor 'basic): Update the basic citation
> processor to follow citations using `org-cite-basic-follow'.
>
> * etc/ORG_NEWS (Menu for choosing how to follow citations): Describe
> the new feature
> (New option ~org-cite-basic-follow-ask~): Describe this new
> customization option.
> (New option ~org-cite-basic-follow-actions~): Describe this new
> customization option, which specifies the layout of the
> `org-cite-basic-follow' transient menu.
>
> This change was co-authored with much support from Ihor Radchenko and
> Jonas Bernoulli, thanks!
> ---
>  etc/ORG-NEWS     |  22 +++++++++
>  lisp/oc-basic.el | 115 +++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 128 insertions(+), 9 deletions(-)
>
> diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
> index de4f11b25..bacc38be2 100644
> --- a/etc/ORG-NEWS
> +++ b/etc/ORG-NEWS
> @@ -114,6 +114,15 @@ The keybindings in the repeat-maps can be changed by customizing
>  
>  See the new [[info:org#Repeating commands]["Repeating commands"]] section in Org mode manual.
>  
> +*** 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 -4 prefix.
> +
> +The contents of this menu can be customized in
> +~org-cite-basic-follow-actions~.
> +
>  ** New and changed options
>  
>  # Chanes deadling with changing default values of customizations,
> @@ -158,6 +167,19 @@ English.  The default value is ~t~ as the CSL standard assumes that
>  English titles are specified in sentence-case but the bibtex
>  bibliography format requires them to be written in title-case.
>  
> +*** New option ~org-cite-basic-follow-ask~
> +
> +When this option is non-nil, following a citation with the basic citation
> +backend will present a transient menu with choices for how to follow the
> +citation.
> +If nil, following a citation will open its bibliography entry.
> +
> +This behaviour can be reversed with a -4 prefix argument.
> +
> +*** New option ~org-cite-basic-follow-actions~
> +
> +This option specifies the options presented by ~org-cite-basic-follow~.
> +
>  ** New functions and changes in function arguments
>  
>  # This also includes changes in function behavior from Elisp perspective.
> diff --git a/lisp/oc-basic.el b/lisp/oc-basic.el
> index e207a1997..fc55d3a32 100644
> --- a/lisp/oc-basic.el
> +++ b/lisp/oc-basic.el
> @@ -74,6 +74,8 @@
>  (require 'map)
>  (require 'oc)
>  (require 'seq)
> +(require 'transient)
> +(require 'org-element)
>  
>  (declare-function org-open-at-point "org" (&optional arg))
>  (declare-function org-open-file "org" (path &optional in-emacs line search))
> @@ -140,6 +142,39 @@
>    :type 'face
>    :safe #'facep)
>  
> +(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'."
> +  :group 'org-cite
> +  :package-version '(Org . "9.8")
> +  :type 'boolean)
> +
> +(defcustom org-cite-basic-follow-actions
> +  '[["Open"
> +     ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]
> +    ["Copy"
> +     ("d" "DOI"
> +      (kill-new
> +       (org-cite-basic--get-field
> +        'doi
> +        (org-cite-basic--get-key !citation))))]]
> +  "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 (the citation
> +object to be followed) and !prefix (any prefix argument to the follower) can be
> +used to access those values. For example:
> +(org-cite-basic-goto !citation !prefix) or
> +(lambda () (message (org-element-property :key !citation)))"
> +  :group 'org-cite
> +  :package-version '(Org . "9.8")
> +  :type 'sexp)
> +
>  \f
>  ;;; Internal variables
>  (defvar org-cite-basic--bibliography-cache nil
> @@ -326,6 +361,16 @@ INFO is the export state, as a property list."
>                  (map-keys entries))
>                (org-cite-basic--parse-bibliography)))
>  
> +(defun org-cite-basic--get-key (citation-or-citation-reference)
> +  "Return citation key for CITATION."
> +  (if (org-element-type-p citation-or-citation-reference 'citation-reference)
> +      (org-element-property :key citation-or-citation-reference)
> +    (pcase (org-cite-get-references citation-or-citation-reference t)
> +      (`(,key) key)
> +      (keys
> +       (or (completing-read "Select citation key: " keys nil t)
> +           (user-error "Aborted"))))))
> +
>  (defun org-cite-basic--get-entry (key &optional info)
>    "Return BibTeX entry for KEY, as an association list.
>  When non-nil, INFO is the export state, as a property list."
> @@ -805,14 +850,7 @@ export state, as a property list."
>  When DATUM is a citation reference, open bibliography entry referencing
>  the citation key.  Otherwise, select which key to follow among all keys
>  present in the citation."
> -  (let* ((key
> -          (if (org-element-type-p datum 'citation-reference)
> -              (org-element-property :key datum)
> -            (pcase (org-cite-get-references datum t)
> -              (`(,key) key)
> -              (keys
> -               (or (completing-read "Select citation key: " keys nil t)
> -                   (user-error "Aborted"))))))
> +  (let* ((key (org-cite-basic--get-key datum))
>           (file
>            (pcase (seq-find (pcase-lambda (`(,_ . ,entries))
>                               (gethash key entries))
> @@ -832,6 +870,65 @@ present in the citation."
>         (bibtex-set-dialect)
>         (bibtex-search-entry key)))))
>  
> +(transient-define-prefix org-cite-basic-follow (citation-object &optional prefix)
> +  "Follow citation.
> +
> +If `org-cite-basic-follow-ask' is non-nil, this transient will present
> +a menu prompting the user for an action. 
> +Otherwise, it will open the bibliography entry for the citation at point.  
> +This behaviour is inverted when the transient is called with a -4 prefix
> +argument.
> +
> +The contents of the menu are defined in the variable
> +`org-cite-basic-follow-actions'."
> +  [:class transient-columns
> +          :setup-children org-cite-basic-follow--setup
> +          :pad-keys t]
> +  (interactive
> +    (list (let ((obj (org-element-context)))
> +           (pcase (org-element-type obj)
> +             ((or 'citation 'citation-reference) obj)
> +             (_ (user-error "No citation at point"))))))
> +  (if (xor org-cite-basic-follow-ask
> +           (equal prefix '(-4)))
> +      (transient-setup 'org-cite-basic-follow nil nil
> +                       :scope (list :citation citation-object :prefix prefix))
> +    (org-cite-basic-goto citation-object 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 (plist-get (transient-scope) :citation))
> +                    (!prefix (plist-get (transient-scope) :prefix)))
> +                ,@fn-args))
> +            ,@other))
> +    (`(,key ,desc (,fn . ,fn-args) . ,other)
> +     `(,key ,desc
> +            (lambda ()
> +	      (interactive)
> +              (let ((!citation (plist-get (transient-scope) :citation))
> +                    (!prefix (plist-get (transient-scope) :prefix)))
> +	        (,fn ,@fn-args)))
> +            ,@other))
> +    (other other)))
> +
> +(defun org-cite-basic-follow--setup (_)
> +  "Update `org-cite-basic-follow' when `org-cite-basic-follow-actions' changes."
> +  (transient-parse-suffixes
> +   'org-cite-basic-follow
> +   (cl-map 'vector
> +           (lambda (group)
> +             (cl-map 'vector #'org-cite-basic-follow--parse-suffix-specification
> +                     group))
> +           org-cite-basic-follow-actions)))
> +
>  \f
>  ;;; "Insert" capability
>  (defun org-cite-basic--complete-style (_)
> @@ -920,7 +1017,7 @@ Raise an error when no bibliography is set in the buffer."
>    :activate #'org-cite-basic-activate
>    :export-citation #'org-cite-basic-export-citation
>    :export-bibliography #'org-cite-basic-export-bibliography
> -  :follow #'org-cite-basic-goto
> +  :follow #'org-cite-basic-follow
>    :insert (org-cite-make-insert-processor #'org-cite-basic--complete-key
>                                            #'org-cite-basic--complete-style)
>    :cite-styles
> -- 
> 2.46.0
>

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

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Gabriel Santos]

[-- Attachment #1: Type: text/plain, Size: 6022 bytes --]

Greetings,

I'll go through the examples found on this e-mail and
suggest the menu command I find best for the proposed
scenarios.

Below is how I think these options could be used.

context-menu-mode
      No particular target, should mostly be used for actions on
      the whole buffer or on a region, or all particular elements
      defined.

      Example: Org -> Headings -> Demote Headings

transient
      Also no particular target, but should be used for commands that
      would require a menu for interaction, such as exporting and capturing.

      Example 1: org-export-*dispatch* -> transient menu
      Example 2: org-capture -> transient menu
      Example 3: org-attach -> transient menu

which-key
      On a target, list actions that could be performed on it.

embark
      Can be used to replace which-key in customization.

      But, despite saying this, I have it configured to use which-key as its menu.

Ihor Radchenko <yantar92@posteo.net> writes:

> I have raised the topic of refactoring Org mode menu systems during
> EmacsConf in <https://emacsconf.org/2024/talks/org-update/>

I saw the talk live, really excited for the future of Org!

> The initial idea was replacing the self-written menu code in Org with
> built-in transient.el.
>
> Later, during OrgMeetup a number of people raised concerns that
> transient may sometimes be an overkill, and that some people may prefer
> alternative UIs. In particular, embark and context-menu-mode were
> mentioned.

Personally, I'd prefer for built-in packages/functionality to be considered first.

The consideration for context-menu for me is particularly intriguing, as there's
a lot of functionality already included in Org's context menu.

> (I am CCing the discussion participants and potentially interested
> maintainers)

Hope to see their responses to this.

I'm just a common user, so my opinions should be taken with a grain of salt.

> In Org mode (although not only in Org mode, looking at the success of
> embark.el), we often have a UI model where users call an "action"
> command (like org-ctrl-c-ctrl-c or org-open-at-point) followed by
> interactive selection among multiple actions.

I don't often use org-ctrl-c-ctrl-c, but now that I've seen the interaction
menu for properties as an example, I'd say the best option for it would be
which-key, as it's a simpler menu.

> For example, org-open-at-point on a heading with multiple links inside
> will raise a section buffer listing all the links - pressing a number
> will open the corresponding link.

I'd consider which-key again for this.

> Another example (see the example patch below), which is a
> work-in-progress patch for Org citation system, is "following" a
> citation. To "follow" citation may mean multiple things including but
> not limited to: (1) going to citation record in the bibliography file;
> (2) following URL; (3) downloading .pdf file for a citation; etc.
>  **The list of "follow" actions may be customized by users**

This is similar to the functions available in the following package:

<https://github.com/emacs-citar/citar>

It also allows for opening the bibliography, links, notes, and files
connected to the citation.

I use it with embark:

<https://github.com/emacs-citar/citar/blob/main/citar-embark.el>

> The general UI flow in these scenarios will be:
>
> 1. User calls "action" with cursor at certain syntax element
> 2. Action menu is displayed, showing the available actions/targets (dynamically built)
> 3. User selects the action/target

I'm not sure what the best option would be for displaying targets, but which-key
should be able to cover most cases that would require "simpler" menus for actions.

I'd also add in that it could be considered over transient if its dynamically built.
I tend to associate transient with "static" options.

> This UI flow can be implemented using context menus, which-key popups,
> transient menus, and also using embark (where the way menu is displayed
> can be customized).
>
> All the 4 approaches represent different UI models with various
> strengths and weaknesses:
>
> - transient has a very flexible layout builder where the menu items can
>   be arranged granularly, but intercepts the main loop disrupting
>   certain keyboard-based workflows
> - which-key does not stand on the way and integrates well into Emacs'
>   key binding model, but provides little flexibility for menu layout

It has options for setting the pop-up type and position. Could this help with
flexibility?

> - embark stays in the middle between which-key and transient, making use
>   of transient keymaps and allowing a custom menu renderer
> - context-menu-mode provides mouse experience
>
> I am wondering if we can work out some universal API to plug the
> described action->menu->selection model into the UI that user prefers.

I'd say that this is the best options out of all of them, but, as you said:

> "I am wondering if we can work out [...]"

This would require considerable work.

> Tentatively, I am thinking about the following:
>
> For a given Emacs "prefix" command (e.g. org-open-at-point), we define a
> set of customizations:
>
> 1. List of possible actions: ((name1 . action1 props) (name2 . action2 ...) ...)
>    PROPS is a plist defining extra properties like key-binding, display
>    string, maybe something else to be used in the future.
> 2. Menu interface to use (transient, context-menu, embark, which-key)
> 3. Layout settings for the specific interfaces. For example, transient
>    layout definition.
>
> WDYT?
>
> Best,
> Ihor

On this described state (list of actions), which-key would be the
best option according to my definition.

But, on the current state of org-open-at-point (shows more targets),
as I commented previously, there's no menu that I associate with:

"Act on target, display a list of other targets."

Maybe context-menu would be the closest one, but I wouldn't consider
which-key or embark, these are more related to functions.

Regards,

--
*Gabriel Santos*

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Ihor Radchenko]

Gabriel Santos <gabrielsantosdesouza@disroot.org> writes:

> context-menu-mode
>       No particular target, should mostly be used for actions on
>       the whole buffer or on a region, or all particular elements
>       defined.
>
>       Example: Org -> Headings -> Demote Headings
> ...
> Personally, I'd prefer for built-in packages/functionality to be considered first.

We will definitely try to support built-ins first and foremost.
I mentioned embark as an example of alternative UI.
Also, embark might be a candidate for upstreaming.

> The consideration for context-menu for me is particularly intriguing, as there's
> a lot of functionality already included in Org's context menu.

This is not right. I think you are confusing ordinary menu bar and
context menu. Context menu is "right click" menu that will display
different items depending on where you click. Org mode currently does
not have context-menu-mode integration (we should fix this deficiency)

> I don't often use org-ctrl-c-ctrl-c, but now that I've seen the interaction
> menu for properties as an example, I'd say the best option for it would be
> which-key, as it's a simpler menu.

My conclusion so far is that there is no "best" for every user. We
should ideally support user-customized menu UI. The main question is how
to do it.

>> This UI flow can be implemented using context menus, which-key popups,
>> transient menus, and also using embark (where the way menu is displayed
>> can be customized).
>>
>> All the 4 approaches represent different UI models with various
>> strengths and weaknesses:
> ...
> It has options for setting the pop-up type and position. Could this help with
> flexibility?

May you elaborate what "it" refers to?

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

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Gabriel Santos]

[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]

>> The consideration for context-menu for me is particularly intriguing, as there's
>> a lot of functionality already included in Org's context menu.
>
> This is not right. I think you are confusing ordinary menu bar and
> context menu. Context menu is "right click" menu that will display
> different items depending on where you click. Org mode currently does
> not have context-menu-mode integration (we should fix this deficiency)

Maybe it's some third-party package I'm using, but right-cliking on an Org
buffer gives me a lot of options:

<https://0x0.st/XF1y.png>

>> I don't often use org-ctrl-c-ctrl-c, but now that I've seen the interaction
>> menu for properties as an example, I'd say the best option for it would be
>> which-key, as it's a simpler menu.
>
> My conclusion so far is that there is no "best" for every user. We
> should ideally support user-customized menu UI. The main question is how
> to do it.

I also agree with this conclusion. Hope others can contribute with
suggestions of how to go about it.

>>> This UI flow can be implemented using context menus, which-key popups,
>>> transient menus, and also using embark (where the way menu is displayed
>>> can be customized).
>>>
>>> All the 4 approaches represent different UI models with various
>>> strengths and weaknesses:
>> ...
>> It has options for setting the pop-up type and position. Could this help with
>> flexibility?
>
> May you elaborate what "it" refers to?

Sorry, seems I forgot to clarify. "It" in this context was referring to which-key.

It has the following variables for altering (window) display:

- which-key-popup-type
- which-key-side-window-location

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Ihor Radchenko]

Gabriel Santos <gabrielsantosdesouza@disroot.org> writes:

> Maybe it's some third-party package I'm using, but right-cliking on an Org
> buffer gives me a lot of options:
>
> <https://0x0.st/XF1y.png>

This is simply a copy of the top-level menu. You would see the same if
you enable the menu bar. No "context" is considered in this case.

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

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Suhail Singh]

Ihor Radchenko <yantar92@posteo.net> writes:

> Tentatively, I am thinking about the following:
>
> For a given Emacs "prefix" command (e.g. org-open-at-point), we define a
> set of customizations:
>
> 1. List of possible actions: ((name1 . action1 props) (name2 . action2 ...) ...)
>    PROPS is a plist defining extra properties like key-binding, display
>    string, maybe something else to be used in the future.
> 2. Menu interface to use (transient, context-menu, embark, which-key)
> 3. Layout settings for the specific interfaces. For example, transient
>    layout definition.
>
> WDYT?

By "display string" do you mean a description of the action?  Or would
that be additional?

Hopefully, the description of each action will be a first-class member
of the "List of possible actions".  Or is the intent that the
description be taken from the form representing an action (e.g. a defun,
a lambda etc).

-- 
Suhail

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Ihor Radchenko]

Suhail Singh <suhailsingh247@gmail.com> writes:

>> For a given Emacs "prefix" command (e.g. org-open-at-point), we define a
>> set of customizations:
>>
>> 1. List of possible actions: ((name1 . action1 props) (name2 . action2 ...) ...)
>>    PROPS is a plist defining extra properties like key-binding, display
>>    string, maybe something else to be used in the future.
>> 2. Menu interface to use (transient, context-menu, embark, which-key)
>> 3. Layout settings for the specific interfaces. For example, transient
>>    layout definition.
>
> By "display string" do you mean a description of the action?  Or would
> that be additional?

I mean some way to define how the action should be displayed in the
menu. It may be a full string or just a description to be appended to
the action name, or something else.

> Hopefully, the description of each action will be a first-class member
> of the "List of possible actions".  Or is the intent that the
> description be taken from the form representing an action (e.g. a defun,
> a lambda etc).

Using a docstring sounds like a good idea. But it is a bit early to
decide these details. I'd like to discuss the more general design first.

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

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Suhail Singh]

Ihor Radchenko <yantar92@posteo.net> writes:

>> Hopefully, the description of each action will be a first-class member
>> of the "List of possible actions".  Or is the intent that the
>> description be taken from the form representing an action (e.g. a defun,
>> a lambda etc).
>
> Using a docstring sounds like a good idea. But it is a bit early to
> decide these details. I'd like to discuss the more general design first.

I agree that it may be premature to think about implementation details
of how docstrings are stored etc.  However, I would like to ensure that
any discussion of the design of ways-to-choose-an-action-at-point
include self-documentation and self-discovery in the desiderata.

Perhaps this goes without saying, in which case please ignore this
message.  However, it's too important to be left to chance.

-- 
Suhail

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark? (was: Fwd: Org-cite: Replace basic follow-processor with transient menu?)

[Panayotis Manganaris]

Hello. I'm very much an elisp amateur, but one of my main motives for
learning has been to make use of emacs' many beautiful UI systems.

Therefore, I'll provide my comments from a designer perspective and
avoid implementation detail.

Ihor Radchenko <yantar92@posteo.net> writes:

> TL;DR: We are in the process of designing a more unified selection
> interface for Org mode

Thanks to Tor-björn, Jonas, Ihor, and everyone else for this work. I've
learned much by following this thread.

> a number of people raised concerns that transient may sometimes be an
> overkill

I agree with others that transient is overkill for most of the menus in
org-mode. A counter argument, going by the patches exchanged here, might
be that transient menus are a clean and scalable approach.

I've not tried the in-dev transient follow processor, but I imagine it
is similar to the old org-ref (John Kitchin) hydra menu. I used org-ref
for years, and enjoyed how its menu united many features, including
- a system analogous to org-cite follow processors for navigating to sources/notes
- a system akin to reftex-mode for org
- a system for downloading new references and making new bibliography entries

With this level of complexity, a transient menu seems like the modern
weapon of choice for a similar feature set. Only, wait, I stopped using
org-ref.

Long story short, I concluded that treating org-mode like a nexus for
these intertwined features was, at least, encouraging my bad habit of
treating org-mode as an "everything app."

I have since delegated the task of populating my personal library to a
dedicated reference manager. I started using latex-mode and reftex to
write papers, thus gaining more typesetting control and making version
management much easier. I still use org to take notes, but now enjoy a
much more stable and focused configuration.

I tell this story as an (anecdotal) case study for how a powerful menu
can be less productive in the long run. Org-ref's hydra menu is not to
blame for my bad decisions but, at least in-part, I used it (and even
extended it) because a greater man put it there.

Scalable design is an invitation to scale. Perhaps the wise know better.
Anyway, the simple tasks of navigating from a citation link to a copy of
the original document, its URL, or an associated note is achievable with
a completing-read (helm, counsel, vertico, etc.) interface.

Transient is overkill.

> This UI flow can be implemented using context menus [...] embark

I like org's context menus, but I like embark better. I do think it
would be very generous of org to delegate its own contextual actions to
embark if so configured.

I'll elaborate:

As is, I like to run org-babel code block functions through embark, I
only wish there were more of them to run. I find this to be far superior
in every way to binding equivalent key chords for the mode.

Also, for me, embark-bindings and embark-prefix-help-command have
completely replaced which-key for discovering key bindings on-the-fly.

> I am wondering if we can work out some universal API to plug the
> described action->menu->selection model into the UI that user prefers.

I support this idea in principle.

> 1. List of possible actions: ((name1 . action1 props) (name2 . action2 ...) ...)
>    PROPS is a plist defining extra properties like key-binding, display
>    string, maybe something else to be used in the future.

I would focus on associating just enough metadata with functions to
enable using them with embark. I really have no idea what the best way
to do this is. This seems to be the biggest missing piece.

Maybe in the short term we just crowd-source some embark configuration
on WORG.

Menus should of course be useful without embark. If the goal here is to
deprecate many of org's ad-hoc interfaces, I think the natural default
replacement for most is completing-read. I may be completely wrong.

> 2. Menu interface to use (transient, context-menu, embark, which-key)

It seems like Tor-björn and Ihor have already laid the groundwork for
using transient in this way. So, I'll just say bravo. Regardless of my
opinion on transient overuse, it looks like great work.

On the other hand, e.g. if the org export dispatch menu isn't transient,
it should be. Seems obvious to me.

> 3. Layout settings for the specific interfaces. For example, transient
>    layout definition.

It is important to avoid making more org-mode-specific UI abstractions.
Whatever the solution, it is important that the UI implementations
tapped to take over help share the maintenance burden.

In this way, I reckon most interface customization can be done in hooks.

> WDYT?

Ideally org-mode uses established APIs rather than making another.

Thanks for reading, and thanks for the chance to contribute.

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark? (was: Fwd: Org-cite: Replace basic follow-processor with transient menu?)

[Ihor Radchenko]

Panayotis Manganaris <panos.manganaris@gmail.com> writes:

> Menus should of course be useful without embark. If the goal here is to
> deprecate many of org's ad-hoc interfaces, I think the natural default
> replacement for most is completing-read. I may be completely wrong.

For the existing interfaces, we are not going to change things in
incompatible ways. The idea is to keep the existing menus working as
similarly as possible compared to the current ad-hoc menus.
So, no, we are not going to replace things with completing-read.
Not by default.

But what I hope to see is a way for users to customize the UI to their
preference. Without touching the defaults.

> On the other hand, e.g. if the org export dispatch menu isn't transient,
> it should be. Seems obvious to me.

Yes. That's where we will have to use transient.
This discussion mostly concerns simpler cases when user needs to choose
from a list.

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

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[indieterminacy]

Hello Ihor,

On 2024-12-13 19:41, Ihor Radchenko wrote:
> TL;DR: We are in the process of designing a more unified selection
> interface for Org mode and want to see if there is some way to unify
> context-menu-mode, transient, which-key and embark together. The idea 
> is
> to (1) avoid too many customizations; (2) allow users to decide how to
> choose between multiple options - by mouse, keyboard, and using
> customizable UIs.
> ...

I would consider an `actions -> menu` functionality to be something 
which should be a distinct tool,
albeit heavily configured to suit Orgmode functionality.
I think its great how Transient was able to emerge from Magit's 
activities and its clearly providing opportunities for scaling the 
utility.

If I may widen the topic a little, your RFC could be an opportunity to 
examine the overlaps between Orgmode and Hyperbole.

For instance, the use of implicit buttons could be examined:
https://www.gnu.org/software/hyperbole/man/hyperbole.html#Implicit-Buttons

I reckon what you are proposing (greater fluency and flow for menus 
dependent on context) could benefit Hyperbole's functionality too (the 
action utility for that environment seems more focused on one action 
rather than prompting a selection of actions).

Ive CC'd Robert Weiner (who leads Hyperbole), incase that is of use.

Kind regards,


Jonathan McHugh

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark? (was: Fwd: Org-cite: Replace basic follow-processor with transient menu?)

[Psionic K]

[-- Attachment #1: Type: text/plain, Size: 4497 bytes --]

> intercepts the main loop
This is optional, per transient menu (prefix) and the commands within it.
A common technique, used by magit and others, is to have entry point
commands in the regular keymap so that many commands can be started without
traversing several menus.  If you want normal un-shadowed bindings active
at the same time, the prefix has a slot called `transient-non-suffix' that
is similar to the `:suppress' option in keymaps or setting a `t'
`undefined' binding in a keymap.  However the results of mixing self-insert
and modal or modal and another modal are generally bad.

The biggest drawbacks of transient are well known in Github issues:
- Which-key discovery of Transient keymaps
- Transient binding generation based on keymaps
- Normalizing how to obtain arguments when being called independently as an
interactive command versus being called as a Transient suffix

In the short term, to punch the first two problems in the face, override
the `:setup-children' method.  If you know what keymap you are borrowing
bindings from, you can synchronize it at display time.

Regarding the normalization with interactive, if you are not using
transient infixes and instead lean on the :info class and dynamic
:descriptions, you can display state and store it using normal buffer-local
and defvar techniques, providing visual feedback for what might be hidden
states after the user gets more familiar.  The commands work with or
without a prefix currently active.  In this usage model, you only use
Transient for its flow control, display, and layout.  I find the infix
system to be somewhat of a distraction if you are not actually building a
CLI wrapper, but you can definitely make suffixes and descriptions "smart"
by reading a scope from the prefix and making custom infixes that also
obtain more state when displayed.  A custom infix for storing org elements
or objects could certainly be a thing.

I think deeper user customization is an area that is weak with transient,
but only because the user actually needs to have a vision for how they want
to build up stateful commands.  If you're just doing prefix maps, transient
and hydra are equivalent concepts.  Transient becomes differentiated when
you consider commands that build up state for other commands.  Executing
slightly modified command sentences in rapid succession is not something
the user customizes casually.  Complex commands only make sense when the
context they depend on is populated, which naturally decides the menu flow.

> I am wondering if we can work out some universal API to plug the
> described action->menu->selection model into the UI that user prefers.
>
> Tentatively, I am thinking about the following:
>
> For a given Emacs "prefix" command (e.g. org-open-at-point), we define a
> set of customizations:
>
> 1. List of possible actions: ((name1 . action1 props) (name2 . action2
...) ...)
>    PROPS is a plist defining extra properties like key-binding, display
>    string, maybe something else to be used in the future.
> 2. Menu interface to use (transient, context-menu, embark, which-key)
> 3. Layout settings for the specific interfaces. For example, transient
>    layout definition.

Well, I'm sure you know that transient has more decisions encoded in the
layout than the other options.  If the data going in is a least common
denominator, you need supplementary data elsewhere to achieve a good result.

What I fear is a system like org-speed-keys which relies on an override of
`org-self-insert' and is yet another orthogonal system.  I much prefer the
Lispy style of integration, which uses a keymap.  Using keymaps, even if
they are not active, to generate transient key bindings via :setup-children
is the best way to have certain integration with other Emacs tools.

How people can collaborate with me on general questions of design is to
open issues on the Transient Showcase.  Either I can point to an existing
example or make a new one.  I've been giving some thought to how to
demonstrate an idea more generally of composing multiple commands and
manipulating the composition to dispatch complex commands in rapid
succession with minor differences.  I personally have my own org speed keys
solution that I've been developing for yet another more complex package I
call Afterburner.  These projects can become stuck in design hell when I
don't have the prodding from other problem analysis, so please, bother me.

https://github.com/positron-solutions/transient-showcase

[-- Attachment #2: Type: text/html, Size: 4850 bytes --]

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark? (was: Fwd: Org-cite: Replace basic follow-processor with transient menu?)

[Ihor Radchenko]

Psionic K <psionik@positron.solutions> writes:

>> intercepts the main loop

> This is optional, per transient menu (prefix) and the commands within it.
> A common technique, used by magit and others, is to have entry point
> commands in the regular keymap so that many commands can be started without
> traversing several menus.  If you want normal un-shadowed bindings active
> at the same time, the prefix has a slot called `transient-non-suffix' that
> is similar to the `:suppress' option in keymaps or setting a `t'
> `undefined' binding in a keymap.  However the results of mixing self-insert
> and modal or modal and another modal are generally bad.

Thanks for the info!
So, we can have something like

:transient-non-suffix 'leave

and then pressing something that is not bound to a suffix or infix will
run the parent keymap command, automatically leaving transient state.

> - Normalizing how to obtain arguments when being called independently as an
> interactive command versus being called as a Transient suffix

I think it is addressed in the example patch I shared. There, we pass
around the original function arguments via macro expansion (!arg-name)

  ["Open" ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]

> In the short term, to punch the first two problems in the face, override
> the `:setup-children' method.  If you know what keymap you are borrowing
> bindings from, you can synchronize it at display time.

This is also partially solved. We do use :setup-children, although the
initial implementation simply reads user customization into menu layout.

I believe that we can read a keymap in similar way and generate
transient layout automatically.

> What I fear is a system like org-speed-keys which relies on an override of
> `org-self-insert' and is yet another orthogonal system.  I much prefer the
> Lispy style of integration, which uses a keymap.  Using keymaps, even if
> they are not active, to generate transient key bindings via :setup-children
> is the best way to have certain integration with other Emacs tools.

May you please elaborate?

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

Re: [RFC] The best way to choose an "action" at point: context-menu-mode, transient, which-key or embark?

[Philip Kaludercic]

Can I please be removed from the CCs in this thread?

Ihor Radchenko <yantar92@posteo.net> writes:

> Psionic K <psionik@positron.solutions> writes:
>
>>> intercepts the main loop
>
>> This is optional, per transient menu (prefix) and the commands within it.
>> A common technique, used by magit and others, is to have entry point
>> commands in the regular keymap so that many commands can be started without
>> traversing several menus.  If you want normal un-shadowed bindings active
>> at the same time, the prefix has a slot called `transient-non-suffix' that
>> is similar to the `:suppress' option in keymaps or setting a `t'
>> `undefined' binding in a keymap.  However the results of mixing self-insert
>> and modal or modal and another modal are generally bad.
>
> Thanks for the info!
> So, we can have something like
>
> :transient-non-suffix 'leave
>
> and then pressing something that is not bound to a suffix or infix will
> run the parent keymap command, automatically leaving transient state.
>
>> - Normalizing how to obtain arguments when being called independently as an
>> interactive command versus being called as a Transient suffix
>
> I think it is addressed in the example patch I shared. There, we pass
> around the original function arguments via macro expansion (!arg-name)
>
>   ["Open" ("b" "bibliography entry" (org-cite-basic-goto !citation !prefix))]
>
>> In the short term, to punch the first two problems in the face, override
>> the `:setup-children' method.  If you know what keymap you are borrowing
>> bindings from, you can synchronize it at display time.
>
> This is also partially solved. We do use :setup-children, although the
> initial implementation simply reads user customization into menu layout.
>
> I believe that we can read a keymap in similar way and generate
> transient layout automatically.
>
>> What I fear is a system like org-speed-keys which relies on an override of
>> `org-self-insert' and is yet another orthogonal system.  I much prefer the
>> Lispy style of integration, which uses a keymap.  Using keymaps, even if
>> they are not active, to generate transient key bindings via :setup-children
>> is the best way to have certain integration with other Emacs tools.
>
> May you please elaborate?