This repository was archived by the owner on Apr 6, 2018. It is now read-only.
Inclusiveness fixes #783
Merged
Inclusiveness fixes #783
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| # differs from VIM, fixed in #774 | ||
| expect(vimState.mode).toBe "insert" | ||
|
|
||
| describe "repeats correctly with .", -> |
There was a problem hiding this comment.
Maybe this should say something general, like describe "repetition and undo", ->.
There was a problem hiding this comment.
quite right, sorry, don't know what happened there; restructured now.
|
Ah, specs tripped by the command/normal rename; I'll fix that after #787 |
e1fdc4d to
6744e61
Compare
This is better because inclusive operations are the special case. CurrentSelection doesn’t get operatesInclusively because it redefines select() anyway.
Conflicts: spec/operators-spec.coffee
and add comments, all for code clarity
6744e61 to
2698424
Compare
|
all new and rebased now |
|
Great! |
t9md
added a commit
to t9md/vim-mode
that referenced
this pull request
Sep 8, 2015
Spec would be updated on atom#774 merge.
t9md
added a commit
to t9md/vim-mode
that referenced
this pull request
Sep 11, 2015
Different approach to atom#783. For me visual Characterwise is exactly inclusiveSelection. So semantically treating this as one functionality is natural. I understand its complicate things maybe, if it became complicated, I will separate function at then.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses one functionality issue with inclusive motion backwards: inclusive movement backwards should include the character under cursor.
At the moment in vim-mode, only
%is an inclusive movement that can go backwards, so only it is affected. When search gets postfixes (basically/e), or when we implement operator-pendingvmotions (e.g.dvbis an inclusive version ofdb), other motions too will be affected.The PR is seemingly more involved than it would have to be, but IMHO it includes useful cleanups:
operatesInclusivelyflag inMotionshould befalseby default because inclusive motions are the exception, the norm is exclusive motions.operatesInclusivelyflag where it's redundant.moveSelectionInclusivelymethod onMotion) and selection movement in visual mode (the newmoveSelectionVisual, which is the oldmoveSelectionInclusively): the old logic was really for any movement in visual mode, and while one could amend it to handle the backward exclusive non-visual movements correctly, I really saw it as combining two functionalities in one unnecessarily.moveSelectionVisual) - it was pretty dense code.The specs are in common with #774, just tweaked for what's fixed here.
Fixes #753.