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?

[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? (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?

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

[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? (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?

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

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

[Juri Linkov]

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

This looks like the best design.  Any part of the org buffer could have
text properties with a list of its available actions.  Such a property
could be similar to 'context-menu-functions' handled by 'context-menu-map'.
But since it will be a plain generic list, it could be transformed to any
menu interface such as transient, context-menu, etc.

To transform it to context-menu, org-mode should provide a function
like 'context-menu-minor' that will create a corresponding menu
that will be added as a submenu of the default context menu.

Such integration with existing menus would be better than the current
implementation of context menus in org-mouse-context-menu that completely
replaces the context menu with its own.

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

[Ihor Radchenko]

Juri Linkov <juri@linkov.net> writes:

>> 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)
>
> This looks like the best design.  Any part of the org buffer could have
> text properties with a list of its available actions.  Such a property
> could be similar to 'context-menu-functions' handled by 'context-menu-map'.
> But since it will be a plain generic list, it could be transformed to any
> menu interface such as transient, context-menu, etc.

I am a bit lost.
Maybe I did not describe the use cases I had in mind well.

What I have in mind is a menu UI for various commands:
1. org-open-at-point (one set of actions)
2. org-ctrl-c-ctrl-c (another set of action)
3. some other command
4. ...

Then, "actions" will be various options a given command can do.

In such scenario, the usefulness of text properties is elusive to me.
I'd rather link the menu items to a command, not to place in buffer.

> To transform it to context-menu, org-mode should provide a function
> like 'context-menu-minor' that will create a corresponding menu
> that will be added as a submenu of the default context menu.
>
> Such integration with existing menus would be better than the current
> implementation of context menus in org-mouse-context-menu that completely
> replaces the context menu with its own.

What do you mean by "default context menu"?

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

[Kierin Bell]

Hi Ihor,

Ihor Radchenko <yantar92@posteo.net> writes:

> 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 think that the built-in Emacs thingatpt.el should not be overlooked
here.

Instead of implementing an entire system specific to Org, imagine a
generic action-at-point interface that works on "things" from
thingatpt.el. For the various targets, Org could add new "providers" to
`thing-at-point-provider-alist', `forward-thing-provider-alist', and
`bounds-of-thing-at-point-provider-alist'. [ Org actually does already
register its own 'url' provider for links. ]

Then, Org could implement a number of action selection interfaces that
act on the various classes of "thing". An exemplary package would be
Philip Kaludercic's great =do-at-point= package, which provides a simple
action selection menu for the thing-at-point using
`read-multiple-choice', which I find elegant and intuitive.[1]

I have gone as far as implementing a 'heading' provider for Org and
`outline-mode' (for use with =do-at-point.el=). I don't see any reason
why Org couldn't define a 'citation' provider, a 'source-block'
provider, etc.

The only downside that I have found with adding lots of thingatpt.el
providers is that it can be difficult to write providers efficient
enough for `forward-thing' in particular
(`forward-thing-provider-alist').

I may also be misunderstanding the proposed interface. For example,
instead of a generic interface for acting on a single thing at point,
maybe you are describing more of an interface for associating commands
with multiple potential targets that must be located (e.g., in a
subtree), which are then each associated with actions.

Even if that's the case, there is a good case for implementing
thingatpt.el providers for the targets, so that users could bring our
own action-at-point packages/interfaces. [ I would be willing to help
write some of those providers. ] And if thingatpt.el isn't generalized
or fast enough, then there is a case for creating a new, more flexible
/de facto/ library like this for Emacs.


[1] https://codeberg.org/pkal/do-at-point

Thanks,
Kierin

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

[Samuel Wales]

a couple of quick and somewhat obvious comments.  i use VERY large
fonts with a maximized gui emacs [vt / maximized urxvt  when needed],
still resulting in, throughout emacs, often, a smaller number of
screen lines or columns than content, even with my usual one window
per frame.  most org menus respect this and can be scrolled or
tweaked.  just wanted compatibility to be in the specs.  especially,
to make sure no functionality is unreachable, but also where possible
to modify column numbers / fill to look ok.  i am slowly trying
transient in gptel and still getting used to it.

next, i use org-mouse and think it's wonderful.  generally, when using
a computer, i use mouse-only or kb-only, but not the combination.
mouseability was mentioned, but just wanted, where possible, the
ability to do most things using only mouse in addition to only kb.
and, where possible, org-mouse compatibility or replacement, whichever
is deemed best.

On Sat, Dec 14, 2024 at 3:08 AM Ihor Radchenko <yantar92@posteo.net> wrote:
>
> 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>
>


-- 
The Kafka Pandemic

A blog about science, health, human rights, and misopathy:
https://thekafkapandemic.blogspot.com

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

[Juri Linkov]

>>> 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)
>>
>> This looks like the best design.  Any part of the org buffer could have
>> text properties with a list of its available actions.  Such a property
>> could be similar to 'context-menu-functions' handled by 'context-menu-map'.
>> But since it will be a plain generic list, it could be transformed to any
>> menu interface such as transient, context-menu, etc.
>
> I am a bit lost.
> Maybe I did not describe the use cases I had in mind well.
>
> What I have in mind is a menu UI for various commands:
> 1. org-open-at-point (one set of actions)
> 2. org-ctrl-c-ctrl-c (another set of action)
> 3. some other command
> 4. ...
>
> Then, "actions" will be various options a given command can do.
>
> In such scenario, the usefulness of text properties is elusive to me.
> I'd rather link the menu items to a command, not to place in buffer.

Indeed, in this case text properties are not needed.
Then you can use something like the buffer-local variable with the
same name 'context-menu-functions' (handled in 'context-menu-map').
The main point is that these functions return a menu.

But instead of a menu, an org function could return
a more high-level data structure like
((name1 . action1 props) (name2 . action2 ...) ...)

IOW, this means adding an abstraction layer on top of the existing
user interfaces such as transient and context-menu.

>> To transform it to context-menu, org-mode should provide a function
>> like 'context-menu-minor' that will create a corresponding menu
>> that will be added as a submenu of the default context menu.
>>
>> Such integration with existing menus would be better than the current
>> implementation of context menus in org-mouse-context-menu that completely
>> replaces the context menu with its own.
>
> What do you mean by "default context menu"?

The default context menu is the menu constructed from many different
context-menu functions.  Some of them add Undo/Redo entries,
some add Select/Copy/Paste, some add Global submenus, etc.

I meant to keep all these existing menu items, and also append
the submenu items returned by Org-mode.

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]

Samuel Wales <samologist@gmail.com> writes:

> a couple of quick and somewhat obvious comments.  i use VERY large
> fonts with a maximized gui emacs [vt / maximized urxvt  when needed],
> still resulting in, throughout emacs, often, a smaller number of
> screen lines or columns than content, even with my usual one window
> per frame.  most org menus respect this and can be scrolled or
> tweaked.  just wanted compatibility to be in the specs.  especially,
> to make sure no functionality is unreachable, but also where possible
> to modify column numbers / fill to look ok.  i am slowly trying
> transient in gptel and still getting used to it.

Thanks for the important input!
Does transient look fine on your screen? If yes, we should not have a
problem.

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

[Ihor Radchenko]

Juri Linkov <juri@linkov.net> writes:

> Then you can use something like the buffer-local variable with the
> same name 'context-menu-functions' (handled in 'context-menu-map').
> The main point is that these functions return a menu.
>
> But instead of a menu, an org function could return
> a more high-level data structure like
> ((name1 . action1 props) (name2 . action2 ...) ...)
>
> IOW, this means adding an abstraction layer on top of the existing
> user interfaces such as transient and context-menu.

Yeah. I am not 100% sure if adding an abstraction layer is a great idea.
Another idea I was considering is similar to what you propose: have some
kind of hook like `context-menu-functions', but accepting an extra
argument - menu type (context menu, transient, which-key, etc.). Then, it
should produce appropriate menu spec.

However, this will require Elisp to customize things. Also not ideal.

-- 
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? (was: Fwd: Org-cite: Replace basic follow-processor with transient menu?)

[Samuel Wales]

thanks for interest.

the transient menu i am trying uses more character columns and
lines than window (c-u m-x gptel-send).  transient's solution
does not work well for me.

1) wrapping
   - 3 text columns.  rightmost text column (header: "Response
     to") has lines that wrap at rhs (not word) to lhs (not
     beginning of that text column), putting lines between items
     in first text column.
   - (for clarity: by rhs and lhs, i mean where text goes, at
     smallest usable font, maximized gui frame, no wm
     decoration, one window in frame, 2 fringes)
   - BEST SOLUTION FOR ME: wrap at word boundaries, /within each
     text column/, as is done when one exports org tables to
     html
2) vertical scrolling
   - BEST SOLUTION FOR ME: SPC DEL, as is done in org export
     menu
     - in transient, SPC and DEL make echo area say ? for help.
       ? makes the message go away but nothing else changes.
       another ?  brings up the help for function gptel-send.
       i'd want scrolling.
     - transient is vertically scrollable using up down arrows
       and c-v m-v, but those are harder on rsi and harder to
       locate on my kb than SPC DEL
   - i did not notice that there are lines below window.  if
     there isn't an indicator, BEST LOCATION FOR ME: left fringe
     in cases where fringe exists.
3) transient does not allow changing font size with
   text-scale-increase/decrease (but a smaller font would not be
   legible)


- transient:
    - Archive: gnu
    - Version: 0.8.1
    - dependency issue before explicitly installing this version.

- emacs    29.4
- org    9.7.16
- gptel    ~/.emacs.d/elpa/gptel-0.9.6.0.20241115.83706


some current org menus, although REALLY GOOD, have possibly
regressed in recent years.  in any case, for example, export is
scrollable; todo kw is not, so i cannot access some todo kw.

i put a lot of effort into this response, hope it makes sense.


On Mon, Dec 16, 2024 at 10:52 AM Ihor Radchenko <yantar92@posteo.net> wrote:
>
> Samuel Wales <samologist@gmail.com> writes:
>
> > a couple of quick and somewhat obvious comments.  i use VERY large
> > fonts with a maximized gui emacs [vt / maximized urxvt  when needed],
> > still resulting in, throughout emacs, often, a smaller number of
> > screen lines or columns than content, even with my usual one window
> > per frame.  most org menus respect this and can be scrolled or
> > tweaked.  just wanted compatibility to be in the specs.  especially,
> > to make sure no functionality is unreachable, but also where possible
> > to modify column numbers / fill to look ok.  i am slowly trying
> > transient in gptel and still getting used to it.
>
> Thanks for the important input!
> Does transient look fine on your screen? If yes, we should not have a
> problem.
>
> --
> 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>



--
The Kafka Pandemic

A blog about science, health, human rights, and misopathy:
https://thekafkapandemic.blogspot.com

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

[Samuel Wales]

thanks for interest.

the transient menu i am trying uses more character columns and
lines than window (c-u m-x gptel-send).  transient's solution
does not work well for me.

1) wrapping
   - 3 text columns.  rightmost text column (header: "Response
     to") has lines that wrap at rhs (not word) to lhs (not
     beginning of that text column), putting lines between items
     in first text column.
   - (for clarity: by rhs and lhs, i mean where text goes, at
     smallest usable font, maximized gui frame, no wm
     decoration, one window in frame, 2 fringes)
   - BEST SOLUTION FOR ME: wrap at word boundaries, /within each
     text column/, as is done when one exports org tables to
     html
2) vertical scrolling
   - BEST SOLUTION FOR ME: SPC DEL, as is done in org export
     menu
     - in transient, SPC and DEL make echo area say ? for help.
       ? makes the message go away but nothing else changes.
       another ?  brings up the help for function gptel-send.
       i'd want scrolling.
     - transient is vertically scrollable using up down arrows
       and c-v m-v, but those are harder on rsi and harder to
       locate on my kb than SPC DEL
   - i did not notice that there are lines below window.  if
     there isn't an indicator, BEST LOCATION FOR ME: left fringe
     in cases where fringe exists.
3) transient does not allow changing font size with
   text-scale-increase/decrease (but a smaller font would not be
   legible)


- transient:
    - Archive: gnu
    - Version: 0.8.1
    - dependency issue before explicitly installing this version.

- emacs    29.4
- org    9.7.16
- gptel    ~/.emacs.d/elpa/gptel-0.9.6.0.
20241115.83706


some current org menus, although REALLY GOOD, have possibly
regressed in recent years.  in any case, for example, export is
scrollable; todo kw is not, so i cannot access some todo kw.

i put a lot of effort into this response, hope it makes sense.


[p.s.  attempting re-send as reply to all as my first response went to
ihor only.  i hope this is not too jarring as a top post and sent to
all recipients.  idk which recipients are included 1] because others
included them easrlier in the thread/conversation vs. 2] because they
are not on the emacs devel list or the org list an want copies.]

[p.p.s. this webmail ui drives me nuts and some solecisms are due to
it being inaccessible [e.g. no compose button showing, not clear how
to change subject header.]

> On Mon, Dec 16, 2024 at 10:52 AM Ihor Radchenko <yantar92@posteo.net> wrote:
> >
> > Samuel Wales <samologist@gmail.com> writes:
> >
> > > a couple of quick and somewhat obvious comments.  i use VERY large
> > > fonts with a maximized gui emacs [vt / maximized urxvt  when needed],
> > > still resulting in, throughout emacs, often, a smaller number of
> > > screen lines or columns than content, even with my usual one window
> > > per frame.  most org menus respect this and can be scrolled or
> > > tweaked.  just wanted compatibility to be in the specs.  especially,
> > > to make sure no functionality is unreachable, but also where possible
> > > to modify column numbers / fill to look ok.  i am slowly trying
> > > transient in gptel and still getting used to it.
> >
> > Thanks for the important input!
> > Does transient look fine on your screen? If yes, we should not have a
> > problem.
> >
> > --
> > 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>
>
>
>
> --
> The Kafka Pandemic
>
> A blog about science, health, human rights, and misopathy:
> https://thekafkapandemic.blogspot.com



-- 
The Kafka Pandemic

A blog about science, health, human rights, and misopathy:
https://thekafkapandemic.blogspot.com

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

[Ihor Radchenko]

Kierin Bell <fernseed@fernseed.me> writes:

> I think that the built-in Emacs thingatpt.el should not be overlooked
> here.
>
> Instead of implementing an entire system specific to Org, imagine a
> generic action-at-point interface that works on "things" from
> thingatpt.el. For the various targets, Org could add new "providers" to
> `thing-at-point-provider-alist', `forward-thing-provider-alist', and
> `bounds-of-thing-at-point-provider-alist'. [ Org actually does already
> register its own 'url' provider for links. ]

This is actually not what I had in mind in this thread. I was only
hoping to get input about customizing menu interface in a way that menu
UI can be chosen by user.

As for `thing-at-point', it is not enough for Org's needs.
Let me show you an example of one of the Org "action" commands.

org-ctrl-c-ctrl-c does the following:

1. If performs specific actions depending on Org syntax element at point
2. It performs alternative action in Org edit buffers where
   org-finish-function is defined.
3. It performs different things depending on context around thing at
   point. For example, first paragraph inside a list will trigger a
   different action compared to just a paragraph.

While (1) can be easily ported to thing-at-point, (2) is much harder,
and (3) will involve creating artificial "things" just for the purposes
of specific Org command.

> Then, Org could implement a number of action selection interfaces that
> act on the various classes of "thing". An exemplary package would be
> Philip Kaludercic's great =do-at-point= package, which provides a simple
> action selection menu for the thing-at-point using
> `read-multiple-choice', which I find elegant and intuitive.[1]

I'd like Org _not to implement interfaces_. Instead, I want to reuse the
existing interfaces - transient, menus, which-key, etc. My main question
is whether we can do such thing cleanly.

> I may also be misunderstanding the proposed interface. For example,
> instead of a generic interface for acting on a single thing at point,
> maybe you are describing more of an interface for associating commands
> with multiple potential targets that must be located (e.g., in a
> subtree), which are then each associated with actions.

Yup, something more like this.

> Even if that's the case, there is a good case for implementing
> thingatpt.el providers for the targets, so that users could bring our
> own action-at-point packages/interfaces. [ I would be willing to help
> write some of those providers. ] And if thingatpt.el isn't generalized
> or fast enough, then there is a case for creating a new, more flexible
> /de facto/ library like this for Emacs.

Better interoperability with thingatpt.el will be certainly welcome.
I even coined this idea in the context of tree-sitter in the past.

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

Transient: accessibility problems for users who need to use large fonts (was: [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]

Samuel Wales <samologist@gmail.com> writes:

> thanks for interest.
>
> the transient menu i am trying uses more character columns and
> lines than window (c-u m-x gptel-send).  transient's solution
> does not work well for me.

Just to be 100% clear, existing Org menus will not disappear any time
soon. Even if we change menus to transient, I will make sure to leave a
customization to switch back.

New menus will probably use transient/other external menu UI though.

As for your response, I will provide some comments, but I also suggest
you to send a bug report to Emacs, so that Jonas has a chance to pay
close attention.

> 1) wrapping
>    - 3 text columns.  rightmost text column (header: "Response
>      to") has lines that wrap at rhs (not word) to lhs (not
>      beginning of that text column), putting lines between items
>      in first text column.
>    - (for clarity: by rhs and lhs, i mean where text goes, at
>      smallest usable font, maximized gui frame, no wm
>      decoration, one window in frame, 2 fringes)
>    - BEST SOLUTION FOR ME: wrap at word boundaries, /within each
>      text column/, as is done when one exports org tables to
>      html

In other words, you need visual-line-mode inside transient buffers. Do I
understand correctly?

> 2) vertical scrolling
>    - BEST SOLUTION FOR ME: SPC DEL, as is done in org export
>      menu
>      - in transient, SPC and DEL make echo area say ? for help.
>        ? makes the message go away but nothing else changes.
>        another ?  brings up the help for function gptel-send.
>        i'd want scrolling.
>      - transient is vertically scrollable using up down arrows
>        and c-v m-v, but those are harder on rsi and harder to
>        locate on my kb than SPC DEL

I think you can bind SPC and DEL in `transient-base-map' to make things
easier for you. I agree that SPC/DEL doing scrolling are expected from a
menu.

>    - i did not notice that there are lines below window.  if
>      there isn't an indicator, BEST LOCATION FOR ME: left fringe
>      in cases where fringe exists.

How do you usually know it?

On my side, there is an indication after I customized
(setq-default indicate-buffer-boundaries 'left)

> 3) transient does not allow changing font size with
>    text-scale-increase/decrease (but a smaller font would not be
>    legible)

This sounds like an omission. Again, you can use `transient-base-map' to
work around, but it is better to be discussed as a dedicated bug report.

> some current org menus, although REALLY GOOD, have possibly
> regressed in recent years.  in any case, for example, export is
> scrollable; todo kw is not, so i cannot access some todo kw.

AFAIU, org-fast-todo-selection interface was never scrollable. At least,
it looks like it was the case 17 years ago. May I know more details
about which exact menu you are talking about and when did scroll stopped
working? I may be missing something.

-- 
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: Transient: accessibility problems for users who need to use large fonts (was: [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?))

[Samuel Wales]

On Tue, Dec 17, 2024 at 11:02 AM Ihor Radchenko <yantar92@posteo.net> wrote:
> In other words, you need visual-line-mode inside transient buffers. Do I
> understand correctly?

that sounds similar to current behavior.

my suggestion would wrap to the character column of the beginning of
the third text column.

similar to html for non-first table cells.  not lhs.

lorem ipsum        akemashite omedetou    hello
asjnfaksjdnf         ajsk dfnkajsd fkaj sf        this line
                                                                   long

code likely exists someplace in core for this.  in org or a browser.

> I think you can bind SPC and DEL in `transient-base-map' to make things
> easier for you. I agree that SPC/DEL doing scrolling are expected from a
> menu.

thank you.

it would make sense, for me, usually, for SPC to wrap around to first
page in menus, so that DEL is not strictly needed.  transient wraps
for arrow; idk SPC.

> On my side, there is an indication after I customized
> (setq-default indicate-buffer-boundaries 'left)

you are absolutely right.  i have had something similar forever.

the reason i didn't notice it is 1] for me the fringe glyph is small
-- can it be larger? and 2] the cursor is always at bol in transient
in my case for that menu, so the fringe is less noticeable next to a
large block cursor.

also, i just noticed that transient has a dim horizontal line at eob
in that menu which is thoughtful and useful.  i don't know what face
it uses.

> which exact menu you are talking about and when did scroll stopped
> working? I may be missing something.

i don't think you're missing anything significant.  i didn't mean to
make you do forensics.  it was merely a recollection of a possibility;
 i'd find it useful if todo kw scrolled, but i cannot say that it
regressed.

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

[Jonas Bernoulli]

Hello!


Samuel Wales <samologist@gmail.com> writes:

> thanks for interest.

I've worked with other users with impaired vision to improve their
experience with Transient before and tend to prioritize that work.

That being said, the end-of-year crunch is upon us, so I likely won't
find the time to do anything beyond what I have already done today.

You should update Transient to the development version.  I found two
regressions affecting the features described below and fixed them this
morning.  These fixes will make it into the next release, scheduled for
very early 2025.

> the transient menu i am trying uses more character columns and
> lines than window (c-u m-x gptel-send).

I didn't notice that you effectively said "too small in both
dimensions", so the first advice below, might not be as useful as I
initially though.  Or it might, let's see.

> transient's solution does not work well for me.
>
> 1) wrapping
>    - 3 text columns.

You can tell Transient to only ever use a single column, and
additionally you can always display the menu in a new window on the
right instead of at the bottom.

(setq transient-force-single-column t)
(setq transient-display-buffer-action
      '(display-buffer-in-side-window
        (side . right)
        (dedicated . t)
        (inhibit-same-window . t)))

>                       rightmost text column (header: "Response
>      to") has lines that wrap at rhs (not word) to lhs (not
>      beginning of that text column), putting lines between items
>      in first text column.
>    - (for clarity: by rhs and lhs, i mean where text goes, at
>      smallest usable font, maximized gui frame, no wm
>      decoration, one window in frame, 2 fringes)
>    - BEST SOLUTION FOR ME: wrap at word boundaries, /within each
>      text column/, as is done when one exports org tables to
>      html
> 2) vertical scrolling
>    - BEST SOLUTION FOR ME: SPC DEL, as is done in org export
>      menu
>      - in transient, SPC and DEL make echo area say ? for help.
>        ? makes the message go away but nothing else changes.
>        another ?  brings up the help for function gptel-send.
>        i'd want scrolling.
>      - transient is vertically scrollable using up down arrows
>        and c-v m-v, but those are harder on rsi and harder to
>        locate on my kb than SPC DEL

You can add your preferred bindings like so:

  (keymap-set transient-map "SPC" #'transient-scroll-up)
  (keymap-set transient-map "DEL" #'transient-scroll-down)

Some Transient menus to *not* prevent commands from other keymaps to
be invoked while the menu is active.  If you bind "SPC" in all menus
as shown above, then that will shadow the regular binding, making it
hard to insert a space.

[However, I'll have to deeply think about the bindings used for commands
available in all transient menus in general soon, but this is not the
time and place.]

[I'll likely also have to support horizontal scrolling eventually.]

>    - i did not notice that there are lines below window.  if
>      there isn't an indicator, BEST LOCATION FOR ME: left fringe
>      in cases where fringe exists.

This enters "I'll look into that next year" territory.

The color of that line actually indicates whether a menu allows
"outside" commands to be invoked while the menu is active.

Most users don't notice this piece of information, myself included.
You can hide it, if you wish, using:

  (setq transient-mode-line-format nil)

> 3) transient does not allow changing font size with
>    text-scale-increase/decrease (but a smaller font would not be
>    legible)

To permanently use a different font size in transient menus use:

  (defun transient-i-shrunk-the-glyphs ()
    (text-scale-set -1)) ; or 1 to grow

  (add-hook 'transient-setup-buffer-hook #'transient-i-shrunk-the-glyphs)

To allow changing the size on demand, use something like:

  (defun transient-text-scale-increase (inc)
    (interactive "p")
    (with-current-buffer transient--buffer
      (text-scale-increase inc)))

  (defun transient-text-scale-decrease (dec)
    (interactive "p")
    (with-current-buffer transient--buffer
      (text-scale-decrease dec)))

  (keymap-set transient-map "C-x +" #'transient-text-scale-increase)
  (keymap-set transient-map "C-x -" #'transient-text-scale-decrease)
  (keymap-set transient-predicate-map "<transient-text-scale-increase>"
              #'transient--do-call)
  (keymap-set transient-predicate-map "<transient-text-scale-decrease>"
              #'transient--do-call)

You could also remember the size between menu invocations:

  (defvar transient-text-scale-amount 0)

  (transient-define-suffix transient-text-scale-increase (inc)
    :transient t
    (interactive "p")
    (with-current-buffer transient--buffer
      (text-scale-increase inc)
      (setq transient-text-scale-amount text-scale-mode-amount)))

  (transient-define-suffix transient-text-scale-decrease (inc)
    :transient t
    (interactive "p")
    (with-current-buffer transient--buffer
      (text-scale-decrease dec)
      (setq transient-text-scale-amount text-scale-mode-amount)))

  (keymap-set transient-map "C-x +" #'transient-text-scale-increase)
  (keymap-set transient-map "C-x -" #'transient-text-scale-decrease)

  (defun transient-restore-text-scale ()
    (unless (local-variable-p 'text-scale-mode-amount)
      (text-scale-set transient-text-scale-amount)))

I might make a refinement of one of these variants available by default,
but that will require some more though, which I don't have the time for
until next year.

     Cheers,
     Jonas


> - transient:
>     - Archive: gnu
>     - Version: 0.8.1
>     - dependency issue before explicitly installing this version.
>
> - emacs    29.4
> - org    9.7.16
> - gptel    ~/.emacs.d/elpa/gptel-0.9.6.0.20241115.83706
>
>
> some current org menus, although REALLY GOOD, have possibly
> regressed in recent years.  in any case, for example, export is
> scrollable; todo kw is not, so i cannot access some todo kw.
>
> i put a lot of effort into this response, hope it makes sense.
>
>
> On Mon, Dec 16, 2024 at 10:52 AM Ihor Radchenko <yantar92@posteo.net> wrote:
>>
>> Samuel Wales <samologist@gmail.com> writes:
>>
>> > a couple of quick and somewhat obvious comments.  i use VERY large
>> > fonts with a maximized gui emacs [vt / maximized urxvt  when needed],
>> > still resulting in, throughout emacs, often, a smaller number of
>> > screen lines or columns than content, even with my usual one window
>> > per frame.  most org menus respect this and can be scrolled or
>> > tweaked.  just wanted compatibility to be in the specs.  especially,
>> > to make sure no functionality is unreachable, but also where possible
>> > to modify column numbers / fill to look ok.  i am slowly trying
>> > transient in gptel and still getting used to it.
>>
>> Thanks for the important input!
>> Does transient look fine on your screen? If yes, we should not have a
>> problem.
>>
>> --
>> 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>
>
>
>
> --
> The Kafka Pandemic
>
> A blog about science, health, human rights, and misopathy:
> https://thekafkapandemic.blogspot.com

Re: Transient: accessibility problems for users who need to use large fonts (was: [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?))

[Jonas Bernoulli]

If I understand the below correctly, you prefer noticeable truncation
in combination with convenient scrolling support, over wrapping (which
is bound to look bad and confusing, even if we optimize some more).

That is my own preference (too?), and we can look into further tweaks
in that direction next year.

     Jonas


Samuel Wales <samologist@gmail.com> writes:

> On Tue, Dec 17, 2024 at 11:02 AM Ihor Radchenko <yantar92@posteo.net> wrote:
>> In other words, you need visual-line-mode inside transient buffers. Do I
>> understand correctly?
>
> that sounds similar to current behavior.
>
> my suggestion would wrap to the character column of the beginning of
> the third text column.
>
> similar to html for non-first table cells.  not lhs.
>
> lorem ipsum        akemashite omedetou    hello
> asjnfaksjdnf         ajsk dfnkajsd fkaj sf        this line
>                                                                    long
>
> code likely exists someplace in core for this.  in org or a browser.
>
>> I think you can bind SPC and DEL in `transient-base-map' to make things
>> easier for you. I agree that SPC/DEL doing scrolling are expected from a
>> menu.
>
> thank you.
>
> it would make sense, for me, usually, for SPC to wrap around to first
> page in menus, so that DEL is not strictly needed.  transient wraps
> for arrow; idk SPC.
>
>> On my side, there is an indication after I customized
>> (setq-default indicate-buffer-boundaries 'left)
>
> you are absolutely right.  i have had something similar forever.
>
> the reason i didn't notice it is 1] for me the fringe glyph is small
> -- can it be larger? and 2] the cursor is always at bol in transient
> in my case for that menu, so the fringe is less noticeable next to a
> large block cursor.
>
> also, i just noticed that transient has a dim horizontal line at eob
> in that menu which is thoughtful and useful.  i don't know what face
> it uses.
>
>> which exact menu you are talking about and when did scroll stopped
>> working? I may be missing something.
>
> i don't think you're missing anything significant.  i didn't mean to
> make you do forensics.  it was merely a recollection of a possibility;
>  i'd find it useful if todo kw scrolled, but i cannot say that it
> regressed.