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

Fixed "d" and "D" in multicursor mode #1029

Merged
merged 8 commits into from
Dec 18, 2016

Conversation

Platzer
Copy link
Contributor

@Platzer Platzer commented Nov 3, 2016

In Multi-Cursor mode "d" in Visual Mode and "D" in Normal-Mode is not working.
Added the current multicursorindex to the Register.Put method.

@Platzer
Copy link
Contributor Author

Platzer commented Nov 4, 2016

I don't get it why that one is failing with my commit:
grafik
On my local machine it is working, i don't think is related to my changes but i cannot figure out why it is not working.

@xconverge
Copy link
Member

@Platzer same problem on my PR as well

@jpoon
Copy link
Member

jpoon commented Nov 17, 2016

Apologies for the delayed review/response. Any chance you can add tests around this? Thanks.

@johnfn
Copy link
Member

johnfn commented Nov 22, 2016

Yeah, my bad for not getting to this earlier. The failing test concerns me. If you can figure that one out (and yeah add another test maybe?) this should be good to go.

@johnfn
Copy link
Member

johnfn commented Dec 6, 2016

I'm actually not sure if this was necessary; I think the bug ended up being somewhere else. Can you check what's currently on master and see if this change is required?

@Platzer
Copy link
Contributor Author

Platzer commented Dec 6, 2016

@johnfn, the deleting part is fixed in current release but putting into multi cursor register is still broken! would be fixed with this PR. I will try to have a look this week!

@Platzer
Copy link
Contributor Author

Platzer commented Dec 13, 2016

@johnfn, travis is failing cause some npm stuff, but tests are okay.

@johnfn johnfn merged commit 2265586 into VSCodeVim:master Dec 18, 2016
@johnfn
Copy link
Member

johnfn commented Dec 18, 2016

Sorry for taking so long on this, @Platzer. I've merged it in now.

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.

4 participants