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

Refactor motion. #87

Merged
merged 1 commit into from
Dec 3, 2015
Merged

Refactor motion. #87

merged 1 commit into from
Dec 3, 2015

Conversation

jpoon
Copy link
Member

@jpoon jpoon commented Dec 3, 2015

Create a base motion class in which cursor and caret extend. The difference between thecaret vs cursor and why I thought it necessitated separate classes:

  • Caret's can never have a position that at the very end of the string. Therefore, some of the logic differs when calculating the end of the line.
  • Whenever we figure out styling of the caret block, we can place that in their respective class

Another reason for the refactor was to move towards a model of OOP rather than static functions. Additionally, motions are now composable. This makes repeatable actions super easy:

new Cursor().left().left().left().left()

or mix and match:

new Cursor().left().lineEnd().down()

The UI is only updated on the move() command => new Cursor(0, 2).lineBegin().down().move()

@jpoon
Copy link
Member Author

jpoon commented Dec 3, 2015

As this is a major refactor, let me know what you think @guillermooo or @adriaanp or @kimitake (as you had done the original work).

@jpoon jpoon force-pushed the refactor-motion branch 3 times, most recently from d6a110a to baf8889 Compare December 3, 2015 11:50
@jpoon jpoon force-pushed the refactor-motion branch 2 times, most recently from 6dd2b41 to 2aab565 Compare December 3, 2015 19:58
Create a base motion class in which cursor and caret extend. The difference
between the two classes:

* Valid positioning for Caret = [0, EOL)
* Valid position for Cursor = [0, EOL]
* Whenever we do get a styling for cursor/caret, that logic
can be placed in their respective classes.

Motions were becoming cumbersome. With this refactor, motions are now
composable making repeatable actions super easy:

`new Cursor().left().left().left().up().down().move();

The UI updates on the `move()` command. Also fixes #89 in this refactor.
@adriaanp
Copy link
Contributor

adriaanp commented Dec 3, 2015

👍 Love it!

@kimitake
Copy link
Contributor

kimitake commented Dec 3, 2015

I strongly agree with to instantiate the cursor/caret in mode class, actually I tried to do when I created caret class, but I needed to modify a lots, so I gave it up :)

btw, do you have a plan to add default argument as 1 for left, right, up, down etc?

@jpoon
Copy link
Member Author

jpoon commented Dec 3, 2015

do you have a plan to add default argument as 1 for left, right, up, down etc?

No, you can achieve the same composing it => new cursor().right().right().right().

@kimitake
Copy link
Contributor

kimitake commented Dec 3, 2015

e.g. the user inputs "10h", so

for (var i=0; i<10; i++) {
this.caret.left();
}
this.caret.move();

Is this correct? how about 1000G etc?

@jpoon
Copy link
Member Author

jpoon commented Dec 3, 2015

Well, 10G vs 100G is the same. I think that logic should be somewhere else as I'm trying to follow separation of concerns. For instance, that sort of stuff should be in like a Repeatable class or what not cause as we'll need to do the same thing not for motions but for other commands too.

@benjaminRomano
Copy link
Contributor

👍

jpoon added a commit that referenced this pull request Dec 3, 2015
@jpoon jpoon merged commit c9eb896 into master Dec 3, 2015
@jpoon jpoon deleted the refactor-motion branch December 3, 2015 22:18
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.

4 participants