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

fixes #739 #767

Merged
merged 3 commits into from
Sep 27, 2016
Merged

fixes #739 #767

merged 3 commits into from
Sep 27, 2016

Conversation

xconverge
Copy link
Member

Weird bug, weird fix, but I am ok with it if you are

Yay! We love PRs! 🎊

Please include a description of your change & check your PR against this list, thanks:

  • Commit message has a short title & issue references
  • Each commit does a logical chunk of work.
  • It builds and tests pass (e.g gulp tslint)

More info can be found by clicking the "guidelines for contributing" link above.

Copy link
Member

@johnfn johnfn left a comment

Choose a reason for hiding this comment

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

How does this handle, say, c} in the same situation?

I think this may be a special case on w, not c.
Add your review

@xconverge
Copy link
Member Author

hah ok so you are right, I fixed this on w, however c} does not work right (and did not before so I will fix that as well since you brought it up)

@rebornix
Copy link
Member

Just one comment and you may also want to write a simple test to cover this scenario.

@johnfn
Copy link
Member

johnfn commented Sep 20, 2016

I don't understand why this behavior would happen for cw but not dw. I see two possibilities:

  1. We have another special case handling thing in d. If so, we should move both into w.
  2. We are not handling dw as a special case, but it happens to work anyway. If so, this is very strange and worrying, and we should figure out the source of the strangeness and worrisomeness.

@xconverge
Copy link
Member Author

@johnfn what is the special case I have in "c" now?

@johnfn
Copy link
Member

johnfn commented Sep 26, 2016

My bad, @xconverge. I misread your PR. (And also I completely missed your reply. I have trouble with Github's notification system...) It looks good to me. If you bring it up to date, I'll merge it! :)

@jpoon jpoon merged commit 879912f into VSCodeVim:master Sep 27, 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.

4 participants