Merging all the Pull Requests

[bronson]

As an experiment, I've attempted to merge all 31 open pull requests. The result is vim-mode-next which, so far, has been working great. I use it for all my editing.

To try to save vim-mode's maintainers some time and make it easier to find good candiates, I present my findings here. I'll close this issue when five candidates been merged or rejected -- hopefully just enough time to be useful.

16 pull requests can be merged today

These pull requests look well written, have tests, and work great. They required resolving 7 merge conflicts. Ordered roughly by importance and completeness:

• #573 add replace mode.
  
  This is an important feature and this PR works fine. Code and tests look great. Merge it, open the missing features as new issues, and keep improving it.
• #711 fix p at end-of-line leaving cursor in wrong position.
  
  Bug is irritating, fix is obviously correct, tiny code, tested. What's not to like?
• #609 allow inserting with a count and changing count when repeating.
  
  Allowing 80iX<esc> is huge and the rest is pure bonus. code+tests look great.
• #568 don't let undos get too big.
  
  This is an important bugfix, also makes insert mode feel more vimlike. code+tests look great.
• #732 make ctrl-t return from symbol.
  
  tiny fix for what looks to be an innocent typo.
• #717 implement W (whole word) text object.
  
  This is a notorious omission. code+tests look great.
• #718 make find (ftFT) state global.
  
  Fixes a missing feature, very nice. code+tests look great.
• #710 make multiple cursor delete work once, not one-by-one.
  
  Small amount of obviously correct code, tested. No reason not to merge.
• #623 ensure t motion includes the character below the cursor.
  
  Fixes an irritating bug, code is tiny, tests look fine.
• #591 fix scrolling when yanking.
  
  Fixes a mildly irritating bug, code+tests look great.
• #696 improve ip/ap paragraph text objects.
  
  It's true, Atom's getCurrentParagraphBufferRange doesn't behave like Vim's. This fixes the bug and code+tests look fine.
• #720 add ctrl-r in insert mode to paste register.
  
  It's a missing Vim feature, code+tests look great.
• #558 implement zs and ze horizontal scrolling.
  
  Implements a Vim feature and code+tests look great.
• #611 make gg, gugu, and gUgU change case of the whole line.
  
  code+tests look great, I couldn't find any corner cases. Looking forward to guu and gUU.
• #724 add gD, goto global definiton.
  
  Missing Vim feature, code+tests look great. Looking forward to gd.
• #721 add ctrl-e/ctrl-y to copy in insert mode.
  
  I'm not sure many people use these but it's a Vim feature and the code+tests look great.
• #738 make tests fail if deprecated calls are made.
  
  Just infrastructure, PRs that implement missing functionality seem more important to me.

3 can be closed because they already work

• #386 fix broken dj and dk tests
• #417 fix motion when yanking
• #476 can't exit visual mode with v

2 are ready but need tests

These are ready to go except the authors need some hints on how to write tests.

• #723 add ctrl-^ to select recent tab
  
  The code was easy, but mocking the editor in the tests has beaten me.
• #727 add gf, goto file under cursor
  
  Just need to figure out how to test relative file paths. t9md's open-this is another solution.

6 look promising but need discussion or changes

• #397 ctrl-i/ctrl-o jump motions
  Issue says it needs more work but code+tests look pretty good. I've been using t9md's cursor-history package.
• #508 rename command mode to normal mode
  
  I almost put this into the "merge now" category -- it's sorely needed. The only reason it's not in vim-mode-next yet is because it makes continuing to merge with vim-mode painful.
• #513 add move window operations
  
  Looks like this is held up so Atom can improve its API... Any downside to merging it now, and updating it when Atom gets its new calls?
• #581 dw at end of line should not wrap line
  
  Looks OK to me but the author says it's only a proof of concept?
• #658 implement blockwise visual select
  
  I need this feature almost every day. t9md wrote vim-mode-visual-block which also starts to solve this.

6 should probably never be merged

• #432 highlight find motion
  
  This is better handled by adding a hook to vim-mode, then doing the highlighting in another plugin.
• #471 lazy load to save startup time
  
  Not worth it. When the time comes to optimize load times it'll take a lot more effort.
• #506 show prefix char in command input
  
  I don't understand why this never got any comments. I agree, it's a needed feature, but at this point it pretty much needs a rewrite.
• #510 beep when search history is exhausted
  
  Not in Vim so it shouldn't be in vim-mode.
• #653 highlight when text is yanked
  
  Like optimize find motion visual effect #432, this is better handled by adding a hook to vim-mode.
• #686 add vertical/horizontal split key bindings to tree-view.
  
  This is the perfect use for ~/.atom/keymap.cson.

[lee-dohm]

The general rule for Issues is that they be actionable. If the Issue describes a bug, the action desired is to fix it. If the Issue describes a question, the action desired is to answer it. And so on.

From reading this Issue, I'm not clear on what it is you expect or hope to happen other than discussion? If discussion is the goal, perhaps Discuss would be better suited for the purpose. If there is another goal other than gathering general thoughts, could you help clarify it for me?

[bronson]

Thanks @lee-dohm, I agree: uncloseable issues suck. I've reworded this issue to make it clearer how to address it and put a condition on closing it. Hope that's better?

[bronson]

#740 belongs near the top of the patches to be merged. It soundly fixes a bug that I'd guess is tripping up a number of people.

[maxbrunsfeld]

@bronson thanks for doing this investigation. I've got some serious maintenance to do on this package. Following up on some of these contributions today...

[bronson]

Beware, there's a fungus among us: bronson/vim-mode-next#4

I'll figure out which PR caused this regression and report back.

[bronson]

It's #568. I put it on the 'looks promising but' stack and pulled it out of vim-mode-next 0.54.13.

[jacekkopecky]

Does #568 now belong back on the top list?
This is a great list, btw; if I was a maintainer, I'd love this kind of effort.

[bronson]

Yes! Sorry, forgot. Also struck out the two merged PRs.

[bronson]

Updates: #573 is even cooler now.

And we have 8 more PRs ready to merge: #763, #764, #765, #766, #767, #770, #774, #777, #778 I won't add them to this issue though. First things first!

I notice the conflicts are increasing... A few of these PRs fix some pretty fundamental things in vim-mode. I can break them out into their own list if desired. It would make sense to get them in soon so patch authors don't have to spend as much time rebasing.

[bronson]

The list of eligible PRs has moved to https://docs.google.com/document/d/14eNyQl2UJ70N1eYurDlS-qA2TQUkjQrXZfcmBv8_2vo/edit?usp=sharing

Rather than making a big update this post, I'd say the requirements to close it have been met. All further work will be in the google doc.

Lots of merges... This has been a great week!