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

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

[Psionic K]

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

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

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

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

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

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

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

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

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

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

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

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

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

[Ihor Radchenko]

Psionic K <psionik@positron.solutions> writes:

>> intercepts the main loop

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

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

:transient-non-suffix 'leave

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

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

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

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

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

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

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

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

May you please elaborate?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

[Philip Kaludercic]

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

Ihor Radchenko <yantar92@posteo.net> writes:

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

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

[Karthik Chikmagalur]

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

Do you have an example of using buffer-local variables to store
transient state?  Directing me to an appropriate section of the
showcase should be plenty.

Karthik

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

[Karthik Chikmagalur]

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

From the manual page describing the `transient-non-suffix' slot:

> If the value is ‘t’, then non-suffixes can be invoked, when it is
> ‘nil’ (the default) then they cannot be invoked.
>
> The only other recommended value is ‘leave’.  If that is used, then
> non-suffixes can be invoked, but if one is invoked, then that exits the
> transient.

This is one of my issues with Transient -- here it is reimplementing the
idea of a regular transient keymap (the kind you can use with
`set-transient-map') but increasing the cognitive load of applying this
behavior.

I understand that Transient needs to implement behaviors not provided by
Elisp (like infixes), but fashioning whole-cloth replacements for
behaviors that are provided -- like the KEEP-PRED behavior of
`set-transient-map' -- makes writing Transients a very high effort
activity.

Karthik

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

[Psionic K]

> Do you have an example of using buffer-local variables to store
> transient state?  Directing me to an appropriate section of the
> showcase should be plenty.

It's less magic than it sounds.  I basically bypassed the infix system
when making transients for Master of Ceremonies.
https://github.com/positron-solutions/moc

`moc-dispatch` is a kind of utility dashboard for screen casting.
Besides controls, it displays various states you might not see in the
minibuffer or elsewhere.  I used dynamic :description functions rather
than infixes to display these states.  It is much simpler to bring
Elisp states into the interface this way.  There's no need and there
can be no meaning of persisting the current state of Emacs.

> like the KEEP-PRED behavior of `set-transient-map'

There's more.  Calling (or declining to call) `setup-prefix' manually
in an interactive form or using the stack manipulation commands can
switch between menus and pure interactive flows.  It's possible to
construct flows that are more program-driven than simple KEEP-PRED or
repeat maps.  Obviously we can hack them both since they are functions
that can manipulate the downstream flow.  Transient's pre-commands are
a bit complex to understand when hacking in behavior but are related
to its menu stack, something transient maps don't need to express.

Instead of talking about Transient's second system behaviors, we can
identify and fix the worst ones.  Jonas is aware of them and generally
in favor of normalizing how transient meshes with interactive, as long
as it doesn't break magit and all the other dependents.

> > org-speed-keys
> May you please elaborate?

I discovered org speed keys because I was making my own speed-key
system and came across the shadowed bindings.  The bindings are always
shadowed, even when speed keys are off.  Unlike Lispy, where I
customize the shadowing using keymaps, org speed keys has
`org-speed-commands'.  From a discoverability standpoint, it breaks
some things.  The commands are opaque.  All I see is `org-self-insert'
whereas my own bindings have unique command names on every key,
another thing I copied from Lispy.  Even if they have DWIM or
situational behavior, it is easier to identify all behavior when
starting from an entry point that doesn't implement details of other
commands.

org-speed-keys design (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]

Psionic K <psionik@positron.solutions> writes:

>> > org-speed-keys
>> May you please elaborate?
>
> I discovered org speed keys because I was making my own speed-key
> system and came across the shadowed bindings.  The bindings are always
> shadowed, even when speed keys are off.  Unlike Lispy, where I
> customize the shadowing using keymaps, org speed keys has
> `org-speed-commands'.  From a discoverability standpoint, it breaks
> some things.  The commands are opaque.  All I see is `org-self-insert'
> whereas my own bindings have unique command names on every key,
> another thing I copied from Lispy.  Even if they have DWIM or
> situational behavior, it is easier to identify all behavior when
> starting from an entry point that doesn't implement details of other
> commands.

May you please provide more information about the more natural
implementation of context-dependent bindings?

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

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?

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

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

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

And this time with a patch, apologies.

/Tor-björn


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

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

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

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

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

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

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

[Ihor Radchenko]

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

Dear all,

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

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

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

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

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

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

The general UI flow in these scenarios will be:

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

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

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

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

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

Tentatively, I am thinking about the following:

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

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

WDYT?

Best,
Ihor

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

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

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

[Gabriel Santos]

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

Greetings,

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

Below is how I think these options could be used.

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

      Example: Org -> Headings -> Demote Headings

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

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

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

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

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

Ihor Radchenko <yantar92@posteo.net> writes:

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

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

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

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

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

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

Hope to see their responses to this.

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

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

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

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

I'd consider which-key again for this.

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

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

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

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

I use it with embark:

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

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

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

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

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

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

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

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

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

This would require considerable work.

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

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

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

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

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

Regards,

--
*Gabriel Santos*

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

[Ihor Radchenko]

Gabriel Santos <gabrielsantosdesouza@disroot.org> writes:

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

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

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

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

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

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

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

May you elaborate what "it" refers to?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

[Gabriel Santos]

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

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

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

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

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

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

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

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

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

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

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

[Ihor Radchenko]

Gabriel Santos <gabrielsantosdesouza@disroot.org> writes:

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

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

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

[Suhail Singh]

Ihor Radchenko <yantar92@posteo.net> writes:

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

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

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

-- 
Suhail

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

[Ihor Radchenko]

Suhail Singh <suhailsingh247@gmail.com> writes:

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

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

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

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

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

[Suhail Singh]

Ihor Radchenko <yantar92@posteo.net> writes:

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

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

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

-- 
Suhail

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

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

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

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

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

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

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