🐛 Fix repeat-insertion bug referenced in #1006

[stefanvlaski]

Hi all,

I took a shot at fixing the repeat-insertions-after-change bug referenced in #1006 and #1010. As @t9md pointed out, there is an issue with item.confirmChanges(changes) function of the Insert class. That function constructs an instance of TransactionBundler which relies on change.newRange and some other properties which don't exist. As a result, bundler.buildInsertText() always returns "", which is why the dot-command after cwText repeats the deletion, but not the insertion.

This fix makes use of the getChangesSinceCheckpoint function, implemented in atom/text-buffer@9f9025f. That function replaces the function of the same name implemented in lib/vim-state.coffee, which uses private API's. It essentially supplies equivalent information in a different format. The TransactionBundler class is changed accordingly.

The dot-command now behaves as expected on conventional patterns like iText, cwText, c2wText, ccText, etc.

There is some funky behavior when moving the cursor outside of the edited region while in insert-mode and continuing to type (not when fixing something that was just typed). However, I don't think this is a result of this fix, but rather a consequence of the way TransactionBundler bundles the inserted text by calling @editor.getTextInBufferRange. Similar behavior occurs using the release version of the vim-mode package on the 1.6.2 Atom release, when the dot-command was still working. To illustrate, take

One two three.

In command-mode, move the cursor to the beginning of two. Type cwHello which replaces two by Hello. Without leaving insert-mode, move the cursor to the beginning of the line and type world.<space><escape>. I see

world. One Hello three.

Now move the cursor to the beginning of the word three and hit .. I see in

Atom 1.7.3 + this fix:
world. One Hello world. One Hello.
All of world. One Hello is taken to be the typed text, and hence re-inserted after hitting .. Everything between insertions is grouped together and taken to be typed text, even if it was there before.

Atom 1.6.2 + vim-mode 0.65.0
world. One Hello d. One H.
This time d. One H is taken to be the typed text and reinserted after hitting ..

(Neo)vim
world. One Hello world. three.
In vim, it seems like cw is forgotten about once the cursor leaves the original region. . simply repeats the last insert (world.<space>) and does not delete three.

Since this kind of pattern isn't really the intended use of the cw, it's hard to say what the behavior should be, but the difference to vim is worth noting.

Feedback is greatly appreciated.

[stefanvlaski]

Update:

After Travis threw some failed tests when repeating insertions with multiple cursors, I did some more digging. buffer.getChangesSinceCheckpoint already bundles changes in a reasonable manner, making the TransactionBundlerClass redundant. In fact, the way I had implemented it, TransactionBundler overbundled changes, which caused the issues with inserting with multiple cursors.

Commit @e13e028 removes the TransactionBundler class completely and grabs the bundled changes directly from buffer.getChangesSinceCheckpoint. This resolves the failed Travis checks in relevant tests. The build is still not successful, but that is caused by three failures in unrelated tests, which are present in the master branch as well.

This change also has a side-effect on the previously mentioned funky behavior. I now get in Atom 1.7.3 + this fix:
world. One Hello world. .
The buffer.getChangesSinceCheckpoint bundles the two insertions Hello and world.<space> separately, allowing us to fetch only the most recent bundle and executing cwworld.<space> on the . command. This is a step closer to the vim behavior (iworld.<space>). Fully emulating vim behavior here (if desired) should probably be a separate PR, since it is related to the operator history, not the text itself.

[BinaryMuse]

@stefanvlaski Thanks very much for taking this on!

[jacekkopecky]

Thanks @stefanvlaski !