Unintended consequences of removing org-speed-commands-user

Unintended consequences of removing org-speed-commands-user

[Shankar Rao]

Hello all,

I discovered that upgrading to 9.5 broke my configuration because the
variable `org-speed-commands-user' was removed. I read the thread
(https://list.orgmode.org/87v9hzzhrn.fsf@gmail.com/) where this change
was proposed and I completely agree that exposing the whole set of
`org-speeds-commands' to the user for customization is an improvement
over the previous state of affairs. However, I believe there were some
unintended consequences of this change that can make it difficult to
customize `org-speed-commands' for users that are not elisp gurus.

The main problem is that `org-speed-commands' serves two purposes
simultaneously:

1.) It contains all the mappings between speed keys and commands
2.) It contains headlines for command categories.

Because of this second purpose, both the contents and order of entries
in `org-speed-commands' are important. For example, suppose I want to
replace the usual "n" command with my own. According to the usual
conventions for alists
(https://www.gnu.org/software/emacs/manual/html_node/elisp/Association-Lists.html),
new associations such as this are added to the front of the list. But
if I do so, by doing something like:

  (setq org-speed-commands (cons '("n" . #'my-org-next-heading)
org-speed-commands))

Then the speed key "n" will show up twice when
`org-speed-command-help' is invoked. I could first delete the old
association by replacing `org-speed-commands' in the above with
`(assoc-delete-all "n" org-speed-commands)', but then my modified
command will no longer appear in the "Outline Navigation" section of
the speed command help. Alternatively, I could replace the association
for "n" using `alist-get':

  (setf (alist-get "n" org-speed-commands nil nil #'equal)
#'my-org-next-heading)

However, this solution won't work for new speed commands (e.g., if I
want to bind `my-org-next-heading' to "N" instead), because in that
case `alist-get' will return `nil'.

Below is the relevant portion of my config file where I customize
`org-speed-commands':
-----
  (defun alist-set (key value alist-symbol &optional testfn)
    "Set KEY to VALUE in alist referenced by ALIST-SYMBOL.

  If KEY is not present in the alist, then add (KEY. VALUE) to the
  front of the alist. Compare keys with TESTFN. Defaults to equal."
    (if-let ((keyval (assoc key (eval alist-symbol) testfn)))
        (setf (cdr keyval) value)
      (set alist-symbol (cons (cons key value) (eval alist-symbol)))))


  (defvar sbr-org-speed-commands-user '(("User Custom Speed Commands")
                                        ("N" . ded-org-show-next-heading-tidily)
                                        ("P" .
ded-org-show-previous-heading-tidily)
                                        ("h" . sbr-org-speed-insert-subheading)
                                        ("u" . org-up-heading-or-item)
                                        ("b" . org-backward-heading-or-item)
                                        ("f" . org-forward-heading-or-item)
                                        ("p" . org-prev-heading-or-item)
                                        ("n" . org-next-heading-or-item))
    "My custom Org speed commands")

  (dolist (keyval (reverse sbr-org-speed-commands-user))
    (alist-set (car keyval) (cdr keyval) 'org-speed-commands))
-----

As you can see, I defined my own function `alist-set', which modifies
an association in an alist if the key is already present, or adds the
new association to the front of the list otherwise. In my opinion,
functionality like `alist-set' should be built into Emacs itself. My
code then constructs my own list of custom speed commands with its own
section header and uses `alist-set' to add/modify speed commands.
While this code works, it's a bit unsatisfying because

1.) It relies on my custom `alist-set' function
2.) It relies on knowledge of the structure of `org-speed-commands'

More specifically, it requires that my new speed commands need to be
inserted in reverse order into `org-speed-commands' in order to be
displayed properly in `org-speed-commands-help'.

I don't know what is the best solution to enable Org users to add
and/or modify speed commands while also keeping the display of
`org-speed-commands-help' organized. Here is what I propose:

1.) Keep the whole set of `org-speed-commands' exposed to user
customization for power users
2.) Bring back `org-speed-commands-user', but instead of just
appending it to `org-speed-commands' as was done prior to Org 9.5, use
something like my `alist-set' above to add/modify speed command
associations as needed while preserving the display order in
`org-speed-commands-help'.

With my proposal, Org users wouldn't have to concern themselves with
the section headers in `org-speed-commands', but they would still be
able to add/modify/remove commands as they wish.

Let me know if anyone has a simpler alternative to achieve these
goals. If there is sufficient interest in my proposal, I would be
happy to provide a patch.

Thanks,
Shankar

Re: Unintended consequences of removing org-speed-commands-user

[dal-blazej]

Hi,

If you want to insert a new element in the list after a particular
element, you could do :

#+begin_src emacs-lisp 
(let ((bk (cdr (member '("Agenda Views etc") org-speed-commands))))
  (setf (cdr (member '("Agenda Views etc") org-speed-commands))
        (cons '("@" . my-foobarized-speed-command) bk)))
#+end_src

Use append to insert a list of new elements instead of one.

Also simply add a new list at the end, use append :

#+begin_src emacs-lisp :results code
(setq org-speed-commands 
      (append org-speed-commands '(("my foo commands!")
                                   ("@" . my-foobarized-speed-command)
                                   ("&" . my-barfooized-speed-command))))
#+end_src

However if you define many new commands, simply redefining the whole
list is simpler ;)

Re: Unintended consequences of removing org-speed-commands-user

[Shankar Rao]

Thank you for the tip. I didn't realize that `member' could be used in
this way to insert an element at a particular point in a list. But
this doesn't address the main issue in my post, that the user has two
different approaches for adding commands to `org-speed-commands'
depending on whether it is a brand new command or overwriting an old
one.

After thinking about it over the weekend, I believe I've come up with
a simple solution that will achieve easy addition to/ modification of
`org-speed-commands' without bringing reverting back to
`org-speed-commands-user'. I'll submit a patch for this soon

Shankar

On Sun, Nov 28, 2021 at 1:15 PM <dal-blazej@onenetbeyond.org> wrote:
>
>
> Hi,
>
> If you want to insert a new element in the list after a particular
> element, you could do :
>
> #+begin_src emacs-lisp
> (let ((bk (cdr (member '("Agenda Views etc") org-speed-commands))))
>   (setf (cdr (member '("Agenda Views etc") org-speed-commands))
>         (cons '("@" . my-foobarized-speed-command) bk)))
> #+end_src
>
> Use append to insert a list of new elements instead of one.
>
> Also simply add a new list at the end, use append :
>
> #+begin_src emacs-lisp :results code
> (setq org-speed-commands
>       (append org-speed-commands '(("my foo commands!")
>                                    ("@" . my-foobarized-speed-command)
>                                    ("&" . my-barfooized-speed-command))))
> #+end_src
>
> However if you define many new commands, simply redefining the whole
> list is simpler ;)

[PATCH] New function org-speed-command-add for adding/modifying speed commands

[Shankar Rao]

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

Over the weekend I thought about this issue of configuring
`org-speed-commands' more easily, and I believe I have come up with a
simpler solution that I have provided in the attached patch.

This patch creates a new function `org-speed-command-add' that can be
used to add new speed command shortcuts to `org-speed-commands', as
well as modify existing shortcuts. This function takes as argument an
alist in the same format as `org-speed-commands'. For each command for
which the shortcut key is already present in `org-speed-commands', the
old command is replaced with the new command at the same position.
Commands with brand new shortcut keys are added to the user section of
`org-speed-commands'.

I believe this patch strikes a balance between power users, who can
still directly customize `org-speed-commands', and non-experts, who
may want to add/modify some speed commands without having to
understand the structure and organization of `org-speed-commands'.

Shankar

On Wed, Nov 24, 2021 at 10:36 AM Shankar Rao <shankar.rao@gmail.com> wrote:
>
> Hello all,
>
> I discovered that upgrading to 9.5 broke my configuration because the
> variable `org-speed-commands-user' was removed. I read the thread
> (https://list.orgmode.org/87v9hzzhrn.fsf@gmail.com/) where this change
> was proposed and I completely agree that exposing the whole set of
> `org-speeds-commands' to the user for customization is an improvement
> over the previous state of affairs. However, I believe there were some
> unintended consequences of this change that can make it difficult to
> customize `org-speed-commands' for users that are not elisp gurus.
>
> The main problem is that `org-speed-commands' serves two purposes
> simultaneously:
>
> 1.) It contains all the mappings between speed keys and commands
> 2.) It contains headlines for command categories.
>
> Because of this second purpose, both the contents and order of entries
> in `org-speed-commands' are important. For example, suppose I want to
> replace the usual "n" command with my own. According to the usual
> conventions for alists
> (https://www.gnu.org/software/emacs/manual/html_node/elisp/Association-Lists.html),
> new associations such as this are added to the front of the list. But
> if I do so, by doing something like:
>
>   (setq org-speed-commands (cons '("n" . #'my-org-next-heading)
> org-speed-commands))
>
> Then the speed key "n" will show up twice when
> `org-speed-command-help' is invoked. I could first delete the old
> association by replacing `org-speed-commands' in the above with
> `(assoc-delete-all "n" org-speed-commands)', but then my modified
> command will no longer appear in the "Outline Navigation" section of
> the speed command help. Alternatively, I could replace the association
> for "n" using `alist-get':
>
>   (setf (alist-get "n" org-speed-commands nil nil #'equal)
> #'my-org-next-heading)
>
> However, this solution won't work for new speed commands (e.g., if I
> want to bind `my-org-next-heading' to "N" instead), because in that
> case `alist-get' will return `nil'.
>
> Below is the relevant portion of my config file where I customize
> `org-speed-commands':
> -----
>   (defun alist-set (key value alist-symbol &optional testfn)
>     "Set KEY to VALUE in alist referenced by ALIST-SYMBOL.
>
>   If KEY is not present in the alist, then add (KEY. VALUE) to the
>   front of the alist. Compare keys with TESTFN. Defaults to equal."
>     (if-let ((keyval (assoc key (eval alist-symbol) testfn)))
>         (setf (cdr keyval) value)
>       (set alist-symbol (cons (cons key value) (eval alist-symbol)))))
>
>
>   (defvar sbr-org-speed-commands-user '(("User Custom Speed Commands")
>                                         ("N" . ded-org-show-next-heading-tidily)
>                                         ("P" .
> ded-org-show-previous-heading-tidily)
>                                         ("h" . sbr-org-speed-insert-subheading)
>                                         ("u" . org-up-heading-or-item)
>                                         ("b" . org-backward-heading-or-item)
>                                         ("f" . org-forward-heading-or-item)
>                                         ("p" . org-prev-heading-or-item)
>                                         ("n" . org-next-heading-or-item))
>     "My custom Org speed commands")
>
>   (dolist (keyval (reverse sbr-org-speed-commands-user))
>     (alist-set (car keyval) (cdr keyval) 'org-speed-commands))
> -----
>
> As you can see, I defined my own function `alist-set', which modifies
> an association in an alist if the key is already present, or adds the
> new association to the front of the list otherwise. In my opinion,
> functionality like `alist-set' should be built into Emacs itself. My
> code then constructs my own list of custom speed commands with its own
> section header and uses `alist-set' to add/modify speed commands.
> While this code works, it's a bit unsatisfying because
>
> 1.) It relies on my custom `alist-set' function
> 2.) It relies on knowledge of the structure of `org-speed-commands'
>
> More specifically, it requires that my new speed commands need to be
> inserted in reverse order into `org-speed-commands' in order to be
> displayed properly in `org-speed-commands-help'.
>
> I don't know what is the best solution to enable Org users to add
> and/or modify speed commands while also keeping the display of
> `org-speed-commands-help' organized. Here is what I propose:
>
> 1.) Keep the whole set of `org-speed-commands' exposed to user
> customization for power users
> 2.) Bring back `org-speed-commands-user', but instead of just
> appending it to `org-speed-commands' as was done prior to Org 9.5, use
> something like my `alist-set' above to add/modify speed command
> associations as needed while preserving the display order in
> `org-speed-commands-help'.
>
> With my proposal, Org users wouldn't have to concern themselves with
> the section headers in `org-speed-commands', but they would still be
> able to add/modify/remove commands as they wish.
>
> Let me know if anyone has a simpler alternative to achieve these
> goals. If there is sufficient interest in my proposal, I would be
> happy to provide a patch.
>
> Thanks,
> Shankar

[-- Attachment #2: 0001-org-keys-Add-function-org-speed-command-add-to-add-modify-spe.patch --]
[-- Type: application/x-patch, Size: 3999 bytes --]

Re: [PATCH] New function org-speed-command-add for adding/modifying speed commands

[Ihor Radchenko]

Shankar Rao <shankar.rao@gmail.com> writes:

> Over the weekend I thought about this issue of configuring
> `org-speed-commands' more easily, and I believe I have come up with a
> simpler solution that I have provided in the attached patch.
>
> This patch creates a new function `org-speed-command-add' that can be
> used to add new speed command shortcuts to `org-speed-commands', as
> well as modify existing shortcuts. This function takes as argument an
> alist in the same format as `org-speed-commands'. For each command for
> which the shortcut key is already present in `org-speed-commands', the
> old command is replaced with the new command at the same position.
> Commands with brand new shortcut keys are added to the user section of
> `org-speed-commands'.
>
> I believe this patch strikes a balance between power users, who can
> still directly customize `org-speed-commands', and non-experts, who
> may want to add/modify some speed commands without having to
> understand the structure and organization of `org-speed-commands'.

I am not sure about usefulness of the proposed command. For non-power
users, we have cusomize interface. It should work just fine without a
need to introduce another function.

Also, org-speed-command-add may behave strangely if the argument
contains descriptive headline like ("My command group title") as its
first element.

Best,
Ihor

Re: [PATCH] New function org-speed-command-add for adding/modifying speed commands

[Shankar Rao]

Thank you for looking into this. As you recommended, I tried modifying
``org-speed-commands'' using the customize interface. While it did
enable me to configure and organize ``org-speed-commands'' in just the
way I would like, I found using the customize interface to be a bit
cumbersome. Using the customize interface, for each command that I
wished to add, I had to visually scan over the whole structure of
``org-speed-commands'' to see if the key for this command was already
present in the list, and replace it with my command if it is, or
prepend my command to the top of the list if not it was already
present. Then after I saved the customization, it added the whole new
value of ``org-speed-commands'' to the end of my init.el, making it
more cluttered.

Alternatively using my ``org-speed-command-add'' requires me to add a
single command to my init.el containing only my modifications:

    (org-speed-command-add
        '(("N" . ded-org-show-next-heading-tidily)
          ("P" . ded-org-show-previous-heading-tidily)
          ("h" . sbr-org-speed-insert-subheading)
          ("f" . org-forward-heading-or-item)
          ("b" . org-backward-heading-or-item)
          ("u" . org-up-heading-or-item)
          ("n" . org-next-heading-or-item)
          ("p" . org-prev-heading-or-item)))

Using ``org-speed-command-add'', I don't have to be concerned with
whether or not a given key is already present in
``org-speed-commands''.

I agree that ``org-speed-command-add'' has undefined behavior when
provided with only a descriptive headline as its first element. Would
you be more amenable to this command if it either ignored or
explicitly disallowed descriptive headline entries?

Shankar


On Tue, May 3, 2022 at 12:23 PM Ihor Radchenko <yantar92@gmail.com> wrote:
>
> Shankar Rao <shankar.rao@gmail.com> writes:
>
> > Over the weekend I thought about this issue of configuring
> > `org-speed-commands' more easily, and I believe I have come up with a
> > simpler solution that I have provided in the attached patch.
> >
> > This patch creates a new function `org-speed-command-add' that can be
> > used to add new speed command shortcuts to `org-speed-commands', as
> > well as modify existing shortcuts. This function takes as argument an
> > alist in the same format as `org-speed-commands'. For each command for
> > which the shortcut key is already present in `org-speed-commands', the
> > old command is replaced with the new command at the same position.
> > Commands with brand new shortcut keys are added to the user section of
> > `org-speed-commands'.
> >
> > I believe this patch strikes a balance between power users, who can
> > still directly customize `org-speed-commands', and non-experts, who
> > may want to add/modify some speed commands without having to
> > understand the structure and organization of `org-speed-commands'.
>
> I am not sure about usefulness of the proposed command. For non-power
> users, we have cusomize interface. It should work just fine without a
> need to introduce another function.
>
> Also, org-speed-command-add may behave strangely if the argument
> contains descriptive headline like ("My command group title") as its
> first element.
>
> Best,
> Ihor

Re: [PATCH] New function org-speed-command-add for adding/modifying speed commands

[Ihor Radchenko]

Shankar Rao <shankar.rao@gmail.com> writes:

> Using ``org-speed-command-add'', I don't have to be concerned with
> whether or not a given key is already present in
> ``org-speed-commands''.

Sure. However, the same can be achieved with fairly simple Elisp, as you
saw in the earlier messages. I do not clearly see how a significant
number of users can really benefit from this command. Most people will
simply not notice it. Beginners should better use customize interface.
Power users can just write elisp.

The only way I can see the proposed approach useful is creating a
unified framework to set various non-trivial Org options programatically
(like org-agenda-custom-commands). I would be in favour of merging such
framework, but not individual commands. Having convenience setters for
some customisations, but not other will be just confusing. Org already
has a huge number of cusomisations.

Alternatively, if many people jump in and say that your idea is very for
them, the patch can be merged.

> I agree that ``org-speed-command-add'' has undefined behavior when
> provided with only a descriptive headline as its first element. Would
> you be more amenable to this command if it either ignored or
> explicitly disallowed descriptive headline entries?

There is not much point discussing this part given the above issue I
raised.

Best,
Ihor