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

Visual Mode + Rudimentary Operators #144

Merged
merged 5 commits into from
Feb 24, 2016
Merged

Conversation

johnfn
Copy link
Member

@johnfn johnfn commented Feb 13, 2016

First cut at Visual Mode!

@johnfn johnfn force-pushed the visual-mode branch 4 times, most recently from 85c7ef3 to 3318413 Compare February 19, 2016 07:46
@johnfn
Copy link
Member Author

johnfn commented Feb 19, 2016

I now think this is good enough to get reviewed and merged in. There's still lots of work to do, obviously, but this works more or less like visual mode does (at least with our limited set of motions). Next I want to revisit what @guillermooo was looking at in #94 to make better abstractions for operators, with an eye for implementing numbers, the dot operator, and macros.

I left TODOs as reminders of the most obvious places that need to be fixed when that infrastructure work is in place.

@johnfn johnfn changed the title [WIP] Visual Mode + Rudimentary Operators Visual Mode + Rudimentary Operators Feb 19, 2016
@jpoon
Copy link
Member

jpoon commented Feb 19, 2016

Beautiful. I'll take a closer look tomorrow :)

}

setCurrentModeByName(modeName : ModeName) {
this._modes.forEach(mode => {
for (let mode of this._modes) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd prefer forEach :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? IMO, for of is much better than forEach - it reads way nicer, and you can do for (const ... of) which is not possible with forEach. (Are you sure you haven't confused it with for in?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙆‍♀️ Not that strongly attached to it so let's just go with for of

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha. The closer I get to bike shedding, the more argumentative I get, or something. 🌈

@jpoon
Copy link
Member

jpoon commented Feb 22, 2016

Sickkkk! 👍 ❤️

Dropped a couple of comments.

@johnfn
Copy link
Member Author

johnfn commented Feb 23, 2016

@jpoon I think I got it all. Except the forEach thing, which I'll defend until I die 😉 Or until someone gives me a good reason to use it, hehe.

(Dunno what's up with Travis CI, but gulp tslint passes for me.)

@jpoon
Copy link
Member

jpoon commented Feb 24, 2016

Weird, all the builds are failing. I'll take a look in a couple of days -- work's got me busy :(

johnfn added a commit that referenced this pull request Feb 24, 2016
Visual Mode + Rudimentary Operators
@johnfn johnfn merged commit d0eb62a into VSCodeVim:master Feb 24, 2016
This pull request was closed.
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.

2 participants