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

[WIP] Multi-Cursor Mode #587

Closed
wants to merge 70 commits into from
Closed

[WIP] Multi-Cursor Mode #587

wants to merge 70 commits into from

Conversation

johnfn
Copy link
Member

@johnfn johnfn commented Aug 10, 2016

The most requested feature in the history of VSCodeVim... Coming to a Visual Studio Code near you! 💥 🎆 💣 💀

Todo:

  • all motions
  • d
  • i
  • c
  • y
  • I broke insert in insert mode.. :(
  • Only the current cursor is kept within bounds.
  • d$ with two cursors on same line breaks
  • Insert with 2 cursors on same line has trouble
  • I think that count actions are broken
  • o and O don't work and probably other text-inserting actions don't work either.
  • see if i can rewrite the text edit collapsing to take advantage of builtin vscode functions
  • backspace doesn't work over newlines
  • when 2 block cursors combine, they turn into visual
  • esc can't be repeated for every cursor, because it's not idempotent. But it can't be run only once because every cursor needs to shift position. Interesting.
  • When you do 0 on a line with 2 cursors, it doesn't go into visual multi-cursor mode.
  • Paste is broken
  • u should set ALL cursors back.
  • go through commands and figure out which ones plausibly could work with multi-cursor mode
  • Visual Block Mode has issues now - I should add tests
  • 50 failing tests

Stuff which should be fixed before release

  • n/N doesn't work (some cursors are lost)
  • jj doesn't work
  • r doesn't work (this is super weird!)
  • Cursors can stack and then multiple operations are applied at the same location (this is because allCursors is an array, not a set. Should it be a set? I think it should be a set.)
  • Clicking in multi-cursor mode causes wonky behavior
  • selection breaks after visual block mode delete
  • y doesn't work
  • . doesn't work

Stuff which is not crucial for release but should be fixed at some point:

  • double actions like dd yy cc don't work. I knew this design decision would come back to bite me eventually.
  • vaw doesn't work, and probably other word motions don't work either. (except not all, because apaprently va( works.)
  • visual line multi cursor doesn't even exist
  • J doesn't work because it doesn't use the same text insertion mechanisms
  • Y doesn't work either
  • C doesn't work
  • S doesn't work
  • s doesn't work
  • marks work in an insane way that doesn't really work at all.

This is extra tricky. If you leave a mark a in multi cursor mode, we should probably store an array of all the positions that you left. Then if you are in normal mode, but then do ma, you would go into multi cursor mode at where all the a marks were left.

This also makes me wonder if I could do something like gma (syntax pending) to add an additional location to the marks list. That seems really confusing for people other than me, however.

  • substitution commands s/blah/foo/g don't really work across multiple selections.
  • Replace mode is broken

@johnfn johnfn added the WIP label Aug 14, 2016
@johnfn johnfn mentioned this pull request Aug 23, 2016
@johnfn
Copy link
Member Author

johnfn commented Sep 20, 2016

Alright, this is done!

I mean, there's still a bunch of work left to do, but it works reasonably well, and preserves all existing functionality, somehow, which is somewhat of a miracle.

Anyways, @rebornix @jpoon @xconverge @sectioneight or anyone else who has time, feel free to give me feedback, though I realize that this PR is probably rather intimidating to review.

The overview of what I did is, essentially:

  • Wrap all commands in a giant for loop that loops over all cursors and runs the command on each one individually.
  • Rewrite all insert/delete/replace actions such that they return transformation objects rather than being applied immediately. We then apply them all in parallel in an edit() block. (This is necessary because otherwise previous changes could affect the positions of later ones.)

Also:

  • Rewrite the test architecture to be much faster.

@rebornix
Copy link
Member

This pr fixes #790 as well.

@rebornix
Copy link
Member

We need to update engine section to ^1.5.0 to have setLastHistoryEndPosition and get Travis passed.

@rebornix
Copy link
Member

Verified: This PR fixes code snippet issue mentioned in #479 (comment)

@johnfn
Copy link
Member Author

johnfn commented Sep 20, 2016

Ah yeah I need to do updates for Travis. Also, I didn't even intend to fix #479. That's awesome.

@ascandella
Copy link
Contributor

This looks great @johnfn. I can't say I grok every line of it, though. If there's a particular section you'd like some extra eyes on I'm happy to help.

@johnfn
Copy link
Member Author

johnfn commented Sep 30, 2016

Moved to #811 and finished.

@johnfn johnfn closed this Sep 30, 2016
@jpoon jpoon deleted the multi-cursor-mode branch October 5, 2016 08:10
@oliverjhn
Copy link

Is this ever going to be revisited? It seems like it kind of got left half-finished....

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

Successfully merging this pull request may close these issues.

None yet

5 participants