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

evil-ex-shortcut-map broken by #1944 #1949

Open
Deewiant opened this issue Jan 2, 2025 · 4 comments
Open

evil-ex-shortcut-map broken by #1944 #1949

Deewiant opened this issue Jan 2, 2025 · 4 comments

Comments

@Deewiant
Copy link

Deewiant commented Jan 2, 2025

Issue type

  • Bug report

Environment

Emacs version: GNU Emacs 29.4.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo version 1.16.0) of 2024-06-27
Operating System: Ubuntu Jammy (22.04)
Evil version: Evil version 1.15.0
Evil installation type: MELPA
Graphical/Terminal: X
Tested in a make emacs session (see CONTRIBUTING.md): Yes

Reproduction steps

  • Start Emacs with make emacs
  • Evaluate (define-key evil-ex-shortcut-map "abc" #'(lambda () (interactive) (message "hello")))
  • Type the Ex command :abc (no <RET>)

Expected behavior

  • See hello at the bottom of the screen and in the *Messages* buffer

Actual behavior

  • See Unknown command: ‘abc’ at the bottom of the screen and in the *Messages* buffer

Further notes

This was broken by #1944: It works in commit ebc7854 (Account for large values of scroll-margin in evil-window-top+bottom) but not in commit 35fe630 (Call evil--ex-update one last time in minibuffer teardown, 2024-12-14).

My guess is that the (exit-minibuffer) call at

evil/evil-ex.el

Line 429 in cc1a7bd

(exit-minibuffer))))
doesn't interact well with the change to evil-ex-teardown.

tomdl89 added a commit to tomdl89/evil that referenced this issue Jan 3, 2025
@tomdl89
Copy link
Member

tomdl89 commented Jan 3, 2025

Thanks for the bug report @Deewiant. This is an area of the codebase @axelf4 is more familiar with than me, having made a lot of changes here a year and a bit ago. My branch does appear to fix the problem, and doesn't break any tests, but @axelf4 if you can take a look and lmk if that looks ok to you, I'd prefer to wait for your 👍 before merging.

edit - ah it does now throw an error. back to the drawing board...

@axelf4
Copy link
Collaborator

axelf4 commented Jan 4, 2025

I pushed a potential WIP fix to axelf4/evil/fix-ex-shortcuts. Have not tested it enough yet, may have regressed on the original issue.

tomdl89 added a commit to tomdl89/evil that referenced this issue Jan 4, 2025
@tomdl89
Copy link
Member

tomdl89 commented Jan 4, 2025

Ah, I also pushed something. Much less change than yours, but works for both cases. Probably less correct. If you're happy to @axelf4 , I'll leave you to close this issue with either / something else.

@tomdl89 tomdl89 closed this as completed in 6afd86b Jan 5, 2025
@tomdl89
Copy link
Member

tomdl89 commented Jan 5, 2025

In the interests of urgency, I've merged, but @axelf4 feel free to revert if you find a better fix. Cheers

@tomdl89 tomdl89 reopened this Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants