onivim / libvim

libvim: The core Vim editing engine as a minimal C library
https://v2.onivim.io
MIT License
691 stars 34 forks source link

Add 2 new tests validating visual block mode. #215

Closed mtippett closed 4 years ago

mtippett commented 4 years ago

"c" works, "I" fails. See bug onivim2 bug 1633 for bug.

This is an initial pull request for discussion. I'll dive into libvim over the next couple of days to see if I can get to the bottom of it.

bryphe commented 4 years ago

Thanks for adding these tests, @mtippett ! Much appreciated.

Just found out about your twitch stream - very cool 😎

I've been procrastinating on the I case because I was considering it in the context of multiple cursors - I plan on having multiple cursors in Onivim, and a potentially improved experience could be to use block mode / visual mode + I to use a bunch of cursors to show the edits immediately.

I was considering a new API, to indicate from libvim when new cursors should be added: https://github.com/onivim/libvim/pull/167 - hoping to revisit for next milestone.

The actual multiple cursors would be handled outside libvim - I already have some test cases around multiple cursors for reason-libvim: https://github.com/onivim/reason-libvim/blob/dc4740a87833e0587eae03b17912e10aaaa6cfc6/test/MultiCursorTest.re#L62

but I haven't actually hooked up the UI or an entry point to create multiple cursors... yet...

The challenge, otherwise, is that this is a case where Vim has a blocking input strategy - so it'd have to be refactored into our state machine model.

mtippett commented 4 years ago

Hey @bryphe I noticed that I had made a mistake with the rebasing and re-included your previous change ( a4adddf above) and the autobuild marker. It should be net neutral, but I hadn't fixed it before you had merged.

bryphe commented 4 years ago

No worries @mtippett - I did a squash merge so that previous change fell out 👍