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.

Sign up for GitHub

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

Jump to bottom

Corfu: evil-repeat support #798

Merged
merged 2 commits into from Apr 2, 2024
Merged

Corfu: evil-repeat support #798

merged 2 commits into from Apr 2, 2024

Conversation

aaronjensen
Copy link
Contributor

Support for evil-repeat with Corfu.

See:

minad/corfu#169
minad/corfu#225

@condy0919
Copy link
Collaborator

condy0919 commented Mar 25, 2024 •
edited 

modes/corfu/evil-collection-corfu.el:157:8: Error: the function ‘corfu-quit’ is not known to be defined.

But it's declared before being used. weird.

@aaronjensen
Copy link
Contributor Author

Yeah, that's odd. Any ideas?

@aaronjensen
Copy link
Contributor Author

By the way, the Makefile doesn't actually work. It expects evil to be installed, but doesn't install it. Do you have instructions for running tests locally? That would allow me to iterate locally.

@aaronjensen
Copy link
Contributor Author

aaronjensen commented Mar 25, 2024 •
edited

Nevermind, I missed the cask install. It would have been helpful to have this in the README.

@aaronjensen
Copy link
Contributor Author

There, that should work. The declare-function forms were incorrect.

@asymmetric
Copy link
Contributor

The two listed corfu PRs were not merged, how does this affect whether this PR should be merged?

@aaronjensen
Copy link
Contributor Author

aaronjensen commented Mar 26, 2024 •
edited

It's the reason that I'm putting the code here. The author would prefer a different technique to support evil-repeat, but I do not know of any additional techniques. In other words, it not being merged there is additional justification for it being merged here. I don't like having to patch one of corfu's methods, but there is no other option yet (aside from not supporting evil-repeat, of course).

aaronjensen
aaronjensen commented Mar 27, 2024
View reviewed changes
modes/corfu/evil-collection-corfu.el
@@ -89,7 +107,7 @@ This key theme variable may be refactored in the future so use with caution."
;; https://github.com/minad/corfu#tab-and-go-completion
(when (memq 'tab-n-go evil-collection-corfu-key-themes)
(setq corfu-cycle t
corfu-preselect-first nil)
corfu-preselect 'prompt)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changed a bit ago, so corfu-preselect-first didn't actually exist anymore.

@aaronjensen
Copy link
Contributor Author

By the way, if there's any other way to do this, either by modifying evil-repeat or another way to make corfu compatible, I'd be interested. I started a thread on Emacs-devel, but haven't heard anything yet.

@condy0919
Copy link
Collaborator

condy0919 commented Apr 2, 2024 •
edited

Oops, this PR has conflict with the HEAD commit. I thought it's caused by merging #800

@aaronjensen
Copy link
Contributor Author

@condy0919 updated

@condy0919 condy0919 merged commit e0982fc into emacs-evil:master Apr 2, 2024
5 checks passed
@aaronjensen aaronjensen deleted the corfu-evil-repeat branch April 2, 2024 19:03
@aaronjensen aaronjensen mentioned this pull request Apr 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@aaronjensen @condy0919 @asymmetric