Properly handle case where last character in a cancelled command begins a new command

[sscheele]

Closes #6878

[pascalkuthe]

Thanks for taking a stab at this! Looks good mostly just one thing I noticed. However I would like to see some integration tests here. The integration testing framework is actually really week suited for something like this. Just great a test that inputs key sequences where the first key is types twice (like ]]p and check that the command is executed directly (so the selection moves in that example).

[sscheele]

Thanks! I've updated to code to do the recursive call (locally) and am working on the integration tests now.

[sscheele]

I've updated the code to make the recursive call and added two test cases to test/movement.rs, but I think I might need some help with the tests, as the second case is failing for reasons I don't fully understand. I'm pretty sure my code is working and I've written the test itself wrong, but I'm having trouble understanding the formatting for the string that's supposed to represent the editor state. Could someone with more insight into this maybe explain where I've gone wrong?

[pascalkuthe]

I think this might be a bug with the integration testing Framework. It seems that \n\n gets turned into \n somehow.

CC @dead10ck

[sscheele]

I actually think that in addition to the integration test problem, there's unexpected behavior associated with my code here. I've been running this code for a bit and a weird thing is that when I type j and then a non-display character, they either register in reverse order (eg: j then <up> will move the cursor up and then insert a j) or the j gets ignored entirely (this happens if you switch modes, which makes sense - so j then <esc> simply exits to normal mode without moving down a line). I don't have time to work on a fix for this atm, but I didn't want this to get merged with a bug in it.

[dead10ck]

I think this might be a bug with the integration testing Framework. It seems that \n\n gets turned into \n somehow.

CC @dead10ck

This is actually intentional, see #6156. It's a somewhat hacky way to be able to explicitly express the cursor being at the end of the line, on the EOF. I'm open to suggestions, if you can think of any ideas for a better way to do this.

[pascalkuthe]

I think this might be a bug with the integration testing Framework. It seems that \n\n gets turned into \n somehow.
CC @dead10ck

This is actually intentional, see #6156. It's a somewhat hacky way to be able to explicitly express the cursor being at the end of the line, on the EOF. I'm open to suggestions, if you can think of any ideas for a better way to do this.

Hmm I see, I will try to thinm of omething but nothing comes to mind right now. If just a single line ending si removed would just adding an extra newline \n|]\n\n work as a workaround?

[dead10ck]

Hmm I see, I will try to thinm of omething but nothing comes to mind right now. If just a single line ending si removed would just adding an extra newline \n|]\n\n work as a workaround?

Yes it would. I did that in a few tests when I made the change.

[dead10ck]

The change itself looks good to me, just needs that fix for the test. Thanks!

[dead10ck]

Yeah, a quick workaround for this would just be too add another \n, or express this with a multi line string literal.

[aromeronavia]

FWIW, tested this on my end, checked out @sscheele's branch and this worked 👍
I was trying to fix this one on my end but didn't notice this already had a PR hehehe