-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Make vim bindings opt-in #41834
Make vim bindings opt-in #41834
Conversation
This comment has been minimized.
This comment has been minimized.
Originally introduced in JuliaLang#41576
and make `simulate_input` more robust against menus not supporting vim bindings.
d98ab12
to
ab74b57
Compare
This should be good now. #41834 (comment) remains relevant, though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to fix it this way, we'll have to backport #41576 as well. The safest option is to revert from 1.7 and do all this in 1.8. Thoughts?
I am in favor of that. |
There is no urge, I was just hoping to get that feature in half a year earlier (or what else the release schedule is). 🙃 Regarding the backport: I would just disallow vim bindings for |
Personally I think we should wait until Julia 1.8. The issue is this: this is an extension API. Whatever we pick has to be something we're happy supporting for the long term. This design seems reasonable, but it adds three additional methods that need to be defined to support a very simple feature, and it would be package-specific rather than user-specific. There might not be a better way to do this, but are we sure? I would argue that the stabilization process of 1.7 is not the right time to make that decision; better to "do no harm" than to rush something. |
+1 for revert and get some extra time to think about it. The three extra methods also stuck out to me and I think it deserves a bit more thought. |
Ok. So should #41835 be merged into |
From triage: seems like this should not be on the 1.7 milestone? Let's plan for it in 1.8. |
@jonas-schulze, if you do rewrite this for 1.8, just couple of thoughts:
Perhaps you could mimic my own strategy in rewriting the old API, and declare a further subtype of |
Should plan for 1.9 now if we want to. |
as an alternative to #41799.
If accepted, the second commit should be backported to
v1.7
.