emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Unintended consequences of removing org-speed-commands-user
@ 2021-11-24  9:36 Shankar Rao
  2021-11-28 12:15 ` dal-blazej
  2021-11-30 14:19 ` [PATCH] New function org-speed-command-add for adding/modifying speed commands Shankar Rao
  0 siblings, 2 replies; 4+ messages in thread
From: Shankar Rao @ 2021-11-24  9:36 UTC (permalink / raw)
  To: emacs-orgmode

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Unintended consequences of removing org-speed-commands-user
  2021-11-24  9:36 Unintended consequences of removing org-speed-commands-user Shankar Rao
@ 2021-11-28 12:15 ` dal-blazej
  2021-11-30 12:13   ` Shankar Rao
  2021-11-30 14:19 ` [PATCH] New function org-speed-command-add for adding/modifying speed commands Shankar Rao
  1 sibling, 1 reply; 4+ messages in thread
From: dal-blazej @ 2021-11-28 12:15 UTC (permalink / raw)
  To: Shankar Rao; +Cc: emacs-orgmode


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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Unintended consequences of removing org-speed-commands-user
  2021-11-28 12:15 ` dal-blazej
@ 2021-11-30 12:13   ` Shankar Rao
  0 siblings, 0 replies; 4+ messages in thread
From: Shankar Rao @ 2021-11-30 12:13 UTC (permalink / raw)
  To: dal-blazej; +Cc: emacs-orgmode

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] New function org-speed-command-add for adding/modifying speed commands
  2021-11-24  9:36 Unintended consequences of removing org-speed-commands-user Shankar Rao
  2021-11-28 12:15 ` dal-blazej
@ 2021-11-30 14:19 ` Shankar Rao
  1 sibling, 0 replies; 4+ messages in thread
From: Shankar Rao @ 2021-11-30 14:19 UTC (permalink / raw)
  To: emacs-orgmode

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-30 14:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-24  9:36 Unintended consequences of removing org-speed-commands-user Shankar Rao
2021-11-28 12:15 ` dal-blazej
2021-11-30 12:13   ` Shankar Rao
2021-11-30 14:19 ` [PATCH] New function org-speed-command-add for adding/modifying speed commands Shankar Rao

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).