Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for #1518 custom functions bound to "0" break motion parsing #1519

Merged
merged 4 commits into from
Oct 21, 2021

Conversation

erganemic
Copy link
Contributor

@erganemic erganemic commented Sep 29, 2021

This PR should fix #1518.

This PR gets rid of evil-redirect-digit-argument, and replaces its effects with a custom variable. The variable is a list of functions that can be bound to digits (always 0 in practice, I'd imagine, but I'm sure someone's workflow would be affected). This allows for greater consistency in how functions can be bound to 0 - just use any of the define-key variants like normal, then add the relevant function to the custom variable, instead of binding with evil-redirect-digit-argument.

It passes all tests, except for the ones that explicitly rely on "0" being bound to the wrapper function for evil-beginning-of-line, which I've fixed.

One thing I'd like feedback on is that I don't like the way that the new evil-digit-bound-motions custom variable is set with customize-set-variable when evil-respect-visual-line-mode is enabled. Since I typically setq custom variables in my init.el instead of using a custom file, I don't know if setting this one programmatically will cause issues in general.

This PR also removes evil-beginning-of-line-or-digit-argument, since it hasn't been touched since 2011.

@erganemic erganemic changed the title Fix for #1518 Fix for #1518 custom functions bound to "0" break motion parsing Sep 29, 2021
@tomdl89
Copy link
Member

tomdl89 commented Sep 29, 2021

Awesome! Didn't expect an issue and a PR! I'm on hols right now, and no one else is much active around here, so don't be disheartened if you don't get feedback or merged soon. Shout/pester if you get impatient. I'll try to take a look in a week or two.

@tomdl89
Copy link
Member

tomdl89 commented Oct 2, 2021

Just took a look and it looks (suspiciously) simple, so great job if my suspicions are unfounded. One thing I noticed is that ln78 of evil-keybindings still has a reference to evil-digit-argument-or-evil-beginning-of-line so pressing 0 when in Info mode now doesn't work. Otherwise, I'll need to spend a bit longer trying to break it before giving a 👍.

@erganemic
Copy link
Contributor Author

@tomdl89 Awesome, glad to see this pulled in! I guess my concern about a custom variable to maintain a list of digit-bound motions was unfounded? I'm super new to Emacs package development, so I wasn't sure if this solution was the best way to provide other packages in the ecosystem with an interface for binding their own motions to digits or not.

My concern was that while the old behavior was actively harmful (breaking motions with a 0 in them if you tried rebinding another command to 0), the new behavior might not be that useful, since external packages wouldn't have an easy way of setting the custom variable.

If you think that they'd have an avenue for doing that, great! If not, I can keep looking into different ways to accomplish this behavior - maybe using cl-digit-char-p in the keypress parser in order to determine if a given command is bound to a digit?

hlissner added a commit to hlissner/evil-multiedit that referenced this pull request Oct 23, 2021
This commit updates evil-multiedit for newer versions of Evil where
evil-digit-argument-* motions have been removed.

Ref: emacs-evil/evil#1519
hlissner added a commit to hlissner/evil-multiedit that referenced this pull request Nov 18, 2021
evil-digit-bound-motions was added in emacs-evil/evil#1519,
evil-multiedit used it as of 9dc5634, but it was then removed in
emacs-evil/evil#1534, so this commit removes its reference.

Fix: #45
Ref: emacs-evil/evil#1519
Ref: emacs-evil/evil#1534
Ref: 9dc5634
hlissner added a commit to doomemacs/doomemacs that referenced this pull request Nov 21, 2021
evil-digit motions were refactored in emacs-evil/evil#1519 (where
evil-digit-argument-evil-beginning-of-line was removed), which was
pulled in during 8645634.

Fix: #5731
Ref: 8645634
Ref: emacs-evil/evil#1519
peterhoeg pushed a commit to peterhoeg/doomemacs that referenced this pull request Nov 26, 2021
evil-digit motions were refactored in emacs-evil/evil#1519 (where
evil-digit-argument-evil-beginning-of-line was removed), which was
pulled in during 8645634.

Fix: doomemacs#5731
Ref: 8645634
Ref: emacs-evil/evil#1519
linkenneth pushed a commit to linkenneth/doom-emacs that referenced this pull request Jan 10, 2022
evil-digit motions were refactored in emacs-evil/evil#1519 (where
evil-digit-argument-evil-beginning-of-line was removed), which was
pulled in during 8645634.

Fix: doomemacs#5731
Ref: 8645634
Ref: emacs-evil/evil#1519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants