beancount / beancount-mode

Emacs major-mode to work with Beancount ledger files
GNU General Public License v3.0
113 stars 32 forks source link

Change keybindings to respect Emacs conventions #35

Closed kfogel closed 1 year ago

kfogel commented 1 year ago

Adjust the keybindings in beancount-mode-map (which is bound to C-c by default) to respect the Emacs keybinding conventions: namely, keep C-c LETTER reserved for user-defined bindings, and don't have a binding that ends in C-g, since users expect C-g to cancel a sequence.

We accomplish this mostly by just changing all instances of C-c LETTER to C-c C-LETTER, with two exceptions: beancount-transaction-clear is moved from C-c C-g to C-c C-c, and beancount-insert-prices is moved from C-c p to C-c C-i (because C-c C-p is already recommended in README.org as the binding for outline-previous-visible-heading, when one is using outline-minor-mode, and we don't want to shadow that).

Fixes #1.

kfogel commented 1 year ago

@dnicolodi or @blais, this is ready for review from whoever has time. See associated mailing-list thread for context.

kfogel commented 1 year ago

Hi, @dnicolodi and @blais. beancount-mode appears to be unmaintained right now -- no commits since early February, and fairly small PRs are sitting unhandled. I'd like to request to be added as a maintainer. While I can't promise to handle everything in the queue, I do use beancount-mode on a regular basis and am therefore motivated to spend some time on maintenance.

Full disclosure: This particular PR would be a priority for me -- I am constantly running into the bug that this PR fixes. However, I'd be happy to look at some of the other PRs and issue tickets too.

blais commented 1 year ago

I can give you access (we're too busy with other stuff) but can you change it so that the new bindings are the default yet there's a way to restore the old ones? It's really annoying to people when bindings change, there ought to be a way to quickly restore the old bindings and the home page should say how. LMK,

kfogel commented 1 year ago

Thanks, @blais. Is the ";; Restore old-style keybindings" commented-out section in this change not enough to enable people to do that?

My thinking was as follows:

  1. Anyone using beancount-mode in Emacs is likely to be pretty comfortable looking at the source anyway, and would go to the source if they were surprised by a keybinding change.
  2. While someone might miss one or two of the old keybindings, it is probably quite rare that anyone depended on most or all of those bindings. So since all of the old bindings are recommended against in modern Emacs, it's best to just give people a path to selectively re-enable exactly the bindings that are really reflexive for them, rather than offering an all-or-nothing return to the full set of old bindings (because the ones they don't use in Beancount would still be shadowing Emacs features that the user might otherwise want to know about).

If the above doesn't seem like enough to you, I would be happy to implement a function to restore all the old bindings in one go, and to document its availability in the README. I just wanted to present the counterargument first, as personally I don't think an all-or-nothing switch is the best way to go (though my feelings on it are not strong either way).

Best regards, -Karl

blais commented 1 year ago

It's just really annoying when people change bindings. Like, a ton of people will get annoyed all at the same time. In fact I'd argue maybe the default should remain the old ones and issue a warning or something.

kfogel commented 1 year ago

@blais I added commit eeab5ea52d0f1b, which makes it easy to get the old-style keybindings and documents how in README.org. Let me know what you think.

FWIW I would argue very strongly against keeping the old bindings as the default. Right now, the experience for new users is a bad one: beancount-mode clobbers bindings the user is likely to actually depend on -- because that space is reserved for users' custom bindings, thus the bindings in there are exactly the ones people tend to use the most.

kfogel commented 1 year ago

Thanks, @blais!