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

Easymotion new features #1967

Merged
merged 15 commits into from
Aug 27, 2017
Merged

Conversation

maxfie1d
Copy link
Contributor

This PR includes some new features and improvements of easymotion.

New Features

  • n-character move <leader><leader>/<character>...
  • two-chars move like <leader><leader>2s<character><character>

Improvements

  • Jump immediately if the only one candidate found to jump
  • Add tests

This PR is a continuation of my previous PR #1932.

@Chillee
Copy link
Member

Chillee commented Aug 13, 2017

Awesome!

Seems like this incorporates your previous PR in it as well, right?

@Chillee
Copy link
Member

Chillee commented Aug 13, 2017

I'm going to update this branch because I fixed some issues with flaky tests on master.

@maxfie1d
Copy link
Contributor Author

Yes, I think it is ok to merge only this one or merge the previous one first, and then this one.

@maxfie1d
Copy link
Contributor Author

Unfortunately, ci-test seemed to fail. I will investigate the reason.

@Chillee
Copy link
Member

Chillee commented Aug 13, 2017

I think you're good. The first 5 tests seem to be failing on master for w.e. reason as well.

@Chillee
Copy link
Member

Chillee commented Aug 25, 2017

So to confirm, this is A-OK to merge?

@maxfie1d
Copy link
Contributor Author

Yes!

@Chillee
Copy link
Member

Chillee commented Aug 26, 2017

Thank you for your great work! Could you just explain the thing brought up in the code review?

@maxfie1d
Copy link
Contributor Author

You mean I explain what is changed with this PR in the code review area on GitHub?

image

@Chillee
Copy link
Member

Chillee commented Aug 26, 2017

@MaxfieldWalker Just specifically, what is <leader><leader> 2s <char><char>?

I don't see it on the easymotion github readme.

@maxfie1d
Copy link
Contributor Author

maxfie1d commented Aug 26, 2017

The difference between 's' and '2s' is characters used for search.
'2s' requires two characters to search.

Like the GIF below, <leader><leader>'s'<a> shows every line as candidate,
while <leader><leader>'2s'<a><b> shows lines as candidates without the first line because the first line contains only "a".

2017-08-26_12-11-17

@Chillee
Copy link
Member

Chillee commented Aug 26, 2017

@MaxfieldWalker Is this a standard vim-easymotion feature?

@maxfie1d
Copy link
Contributor Author

maxfie1d commented Aug 26, 2017

[Edited]

Two-character search motion is explained here.

By default, two-character move motions are not mapped (default mappings are defined here).

I thought it is always required to assign some keymap to some command for this extension, but is it OK not to do so?

README.md Outdated
`<leader> <leader> b`|Start of word backwards
`<leader> <leader> e`|End of word forwards
`<leader> <leader> g e`|End of word backwards
`<leader><leader> s <char>` or `<leader><leader> 2s <char><char>`|Search character
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I'm not familiar with the 2. What's the purpose of that? If it's supposed to show that it support a count, then I think the convention is <count>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At present, count is not supported. '2' means just requiring two characters for search.
Original easymotion plugin seems not to support <count> for this kind of motions.

`<leader><leader> ge`|End of word backwards
`<leader><leader> j`|Start of line forwards
`<leader><leader> k`|Start of line backwards
`<leader><leader> / <char>... <CR>`|Search n-character
Copy link
Member

Choose a reason for hiding this comment

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

Yay!

@Chillee
Copy link
Member

Chillee commented Aug 26, 2017

@MaxfieldWalker Yep our command system isn't expressive enough to express "unmapped" commands.

I was just wondering if there was a "semi-standard" mapping for those commands. Could you also explain them in the README?

@maxfie1d
Copy link
Contributor Author

@Chillee I see.

@Chillee
Copy link
Member

Chillee commented Aug 27, 2017

Thanks for the great work!

@Chillee Chillee merged commit 1de51bb into VSCodeVim:master Aug 27, 2017
Chillee added a commit that referenced this pull request Aug 27, 2017
@Chillee
Copy link
Member

Chillee commented Aug 27, 2017

ah damn I wasn't trying to squash and merge...

@maxfie1d maxfie1d deleted the easymotion-new-features branch September 3, 2017 14:16
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