Binding any custom function to "0" in the motion state map causes motions with a "0" in the count argument to fail

[erganemic]

Issue type

• Bug report

Environment

Emacs version: GNU Emacs 27.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.27, cairo version 1.17.4) of 2021-03-26
Operating System: Arch Linux, Linux 5.10.64-1-lts x86_64
Evil version: 1.14.0
Evil installation type: Melpa
Graphical/Terminal: Graphical
Tested in a make emacs session (see CONTRIBUTING.md): Yes

Reproduction steps

• Start Emacs
• Evaluate the following lines, which just rename the function bound to 0 while keeping the logic exactly the same:

(defalias 'my-beg 'evil-digit-argument-or-evil-beginning-of-line)
(evil-redirect-digit-argument evil-motion-state-map "0" 'my-beg)

• Try any motion command with a 0 in the count argument: e.g. d10j, c20l, y3040k and note in the echo area that the interpreted keystrokes are different than expected, meaning the commands don't work as intended.

Expected behavior

Users should be able to bind custom commands to 0, modifying the behavior of evil-beginning-of-line without breaking how motions are read. E.g. I should be able to create a function called my-beg that has special behavior, bind it to 0, and still be able to call d10j to delete ten lines down.

Actual behavior

If a custom command is bound to 0, the reading of motions is broken. As an example, when we use evil-redirect-digit-argument to bind a command to 0 (just like Evil does), the command d10j is interpreted by Emacs as C-u 62 j. Therefore, instead of deleting ten lines down, Emacs will jump 62 lines down without modifying the buffer at all.

Further notes

I discovered this issue after realizing that while evil-org-mode was active, any motions with a 0 in the count argument failed, which I documented at Somelauw/evil-org-mode#91. The reason for this issue is as follows:

When an evil operator calls evil-operator-range to get the region of text that should operate on, it eventually calls evil-keypress-parser, which performs the actual atomic operations of reading and interpreting keystrokes. Typically, the way evil-keypress-parser processes a "0" keystroke is to look up whether "0" is bound to evil-redirect-digit-argument-or-evil-beginning-of-line and if a count argument has already been started. If that's the case, the parser treats "0" just as a digit argument, appending a zero to the end of the current count argument (in effect), and continuing to read for further input.

However, the fact that this takes place as a hardcoded check for a specific function name has a couple implications.

Most notably, if the function bound to 0 in the motion state map has any name other than the one expected, all motion commands with a "0" in the count argument break. When a "0" keystroke is recorded, the branch I described above that depends on the name of the function bound to the key isn't taken, so evil-keypress parser doesn't terminate in the expected fashion - the redirected wrapper function just falls through, where it eventually just ends up calling digit-argument, with the original operator command getting ignored except for causing some weirdness by modifying the prefix value.

Secondly, evil-redirect-digit-argument doesn't actually do anything effective, from what I can tell. In the entire Evil codebase, it's only used to create and bind the wrapper for evil-beginning-of-line to 0 - but since this is a motion, any use of it will pass through evil-read-motion, and thus evil-keypress-parser - and, as stated above, evil-keypress-parser already contains logic to determine whether something is a digit argument or another function. In addition, that logic is necessary - removing it and trying to just rely on the redirect wrappers to tell if something was a digit argument wouldn't work without heavily rewriting the parser. This means that completely eliminating evil-redirect-digit-argument, changing the hardcoded check to compare against evil-beginning-of-line, and just using a plain define-key to bind evil-beginning-of-line to 0 works just as well from what I can see (although it still has the inherent fragility of the first issue I mentioned).

I believe I've got the beginnings of a PR that hopefully resolves both of these annoyances. It passes all of the tests in evil-tests.el, but I'm new to contributing to Emacs in general and Evil specifically, so I'm aware it may not use canonical methods (e.g. it uses customize-set-variable if evil-respect-visual-line-mode is non-nil; the pattern of conditionally setting a custom variable is one I didn't find anywhere else, and don't particularly like).