-
Notifications
You must be signed in to change notification settings - Fork 283
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 #496: Crash when pasting from commandline #1506
Conversation
src/Store/InputStoreConnector.re
Outdated
| (true, Vim.Types.CommandLine) => | ||
// LEGACY: Support `commandLineFocus` too. | ||
Hashtbl.add(ret, "commandLineFocus", true); | ||
Hashtbl.add(ret, "commandLineMode", true); |
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.
Why add commandLineMode
if it's exactly the same as the existing commandLineFocus
?
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.
To be consistent with the other modes - we'll eventually have:
insertMode
normalMode
visualMode
...
It seems inconsistent... although I guess it's debatable if commandLine
should be a mode. If we decide to build a 'first-class' command-line mode (like, powered by our text input model + menu infra, with completion, etc), I see that as a good argument for commandLineFocus
instead though. I'll switch it back.
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.
Ah, I see. Yeah, from a normal user POV commandlineFocus
makes much more sense I think, but from a vim POV commandlineMode
definitely makes more sense. I'd lean towards favoring the user POV too though.
Issue: When pasting via
<C-v>
in the command line, we'd crash.Defect:
<C-v>
in the command line actually engages the 'insert-literal' behavior, which blocks for input (causing a hang). This root cause is addressed in onivim/libvim#191 , removing that functionality (we can bring it back in the Onivim 2 input layer if it is desired).Fix: In the Onivim 2 side, hook up our paste command to work with the command line too.
Related to #1233 , and might not be necessary at all with that change (especially if we move more of the command line behavior to Onivim 2)
TODO:
reason-libvim
Fixes #496