Cut/Yank multiple words

[alsuren]

Currently, using control-w to cut multiple words leaves only
the most recently cut word in your cut buffer for yanking with
control-y.

This patch makes reedline behave like readline: it restores all
concecutive cuts as a single buffer. This means that typing
control-w control-w control-y gets you back where you started.

Before this is merged, it would be useful to:

• make a trait ClipboardExt with fn update(&mut self, content: &str, mode: ClipboardMode, operation: ReplaceOrAppendOrPrepent); rather than copy-pasting the same 5 lines everywhere turns out there's a lot of repeated code to do with cutting, and it was easy enough to DRY out. It also made more sense to keep everything internal to Editor rather than pushing it out into the Clipboard trait.
• write some unit tests
  • [ ] it may be possible to write a proptest/fuzz test that generates a random buffer and a random position and a random set of cut commands
• follow UX_TESTING.md to make sure I've not broken anything.
  • here the behavior in VI mode and probably also what happens when you undo/redo certain operations will be the most critical to me.
  • I also made a quick stab at a kind-of integration test suite for the vi edit mode keybindings, but I'm not really sure what UntilFound([MenuUp, Up]) really means (as generated by the k key in vi), so I didn't test multiline editing that well. Feedback welcome, or we can just ship it as-is and improve it later.
• double check whether each of the things should be a prepend or append operation. I only use control-w and control-y in readline, so I don't really know how the other edit commands are supposed to behave.
  • with 1 2 3 4 5 6 7 8 9 in my editor, and the cursor in the middle, C-w followed by C-k or C-u then C-y does the same thing as bash, which is good enough for me. https://www.masteringemacs.org/article/keyboard-shortcuts-every-command-line-hacker-should-know-about-gnu-readline lists a whole bunch more keybindings, but I don't even know how to type meta on macos and I don't have a physical linux box around to test, so I didn't try all keybindings.
• [ ] should I actually check what readline's logic is? If I read the source code, does my contribution here become a derived work? I decided to treat it as a black box.

There is a known edge case that if you delete a word and then delete the whole line that you're on, you won't get back where you started, because the line will be appended to the word. As far as I can tell, bash's emacs mode doesn't have any whole-line-cutting shortcuts, and vi now sends the StopCutting command, so it is impossible to hit this edge case. If it ever becomes a problem in the future then we could change the LastCutCommand state machine to include a ClipboardMode, and reset the cut buffer if ClipboardMode changes from Normal to Lines.

If we decide that we don't want control-w control-w control-y to get you back where you started, I think the first few commits are probably worth having. I'm happy to clean them up for inclusion first if you prefer.

[amtoine]

@alsuren
you have a bunch of unticked tasks, is this ready for review?

[alsuren]

you have a bunch of unticked tasks, is this ready for review?

Hrm. It's certainly not ready for merging (apart from maybe the first two commits). I will put it back in draft.

If anyone has any early comments on the general shape, or edge cases that I've missed (I'm sure there will be loads), I would still appreciate comments on those things.

[sholderbach]

it may be possible to write a proptest/fuzz test that generates a random buffer and a random position and a random set of cut commands, and

I would consider proptest and fuzzing to be outside the scope of the PR. Fuzzing as in just looking for broken invariants is independent, and proptests looking trying to emulate a ground truth also require some good design of those invariants to be worth the runs (they would be still appreciated but not necessary for this PR)

follow UX_TESTING.md to make sure I've not broken anything.

The checklist may be quite out of date. Here the behavior in VI mode and probably also what happens when you undo/redo certain operations will be the most critical to me.

double check whether each of the things should be a prepend or append operation.

Not sure what you mean by that?

should I actually check what readline's logic is?

We are not a GNU readline clone so you don't need to match edge case for edge case. We should make our users happy but that's about it.

If I read the source code, does my contribution here become a derived work?

I am not a layer :D

[sholderbach]

@alsuren To help with the review of added tests I set up a coverage analysis job. If you merge main, you can see if particular lines are hit by our tests.

[codecov]

Codecov Report

Merging #631 (824e203) into main (ebd653b) will increase coverage by 1.88%.
The diff coverage is 58.40%.

@@            Coverage Diff             @@
##             main     #631      +/-   ##
==========================================
+ Coverage   49.80%   51.69%   +1.88%     
==========================================
  Files          41       41              
  Lines        7805     7810       +5     
==========================================
+ Hits         3887     4037     +150     
+ Misses       3918     3773     -145     

Files Changed	Coverage
src/enums.rs	50.00%
src/core_editor/editor.rs	54.94%
src/edit_mode/vi/parser.rs	66.66%
src/edit_mode/vi/mod.rs	87.50%

[alsuren]

This rstest is me trying to implement something like https://matklad.github.io/2021/05/31/how-to-test.html#Data-Driven-Testing

I tried to avoid writing test cases in terms of internal data structures like EditCommand because they tend not to be refactor-proof. Vim commands are already text based, so they feel like a natural base for a testing microformat/DSL. Might be worth making an equivalent DSL for Emacs using the C-w notation from the Emacs/bash docs. Probably something for another PR though.

I think this particular example might be stretching the usefulness of the interface I built a bit though. A better way to write this test would probably be on top of a check_commands(before, commands, after) helper, probably using $0/$1 to encode the cursor position, as the rust-analyzer tests do. The rest of the sum_to_zero test cases could then be rebased on top of such an abstraction.

I'm not sure when I will have another opportunity for recreational hacking. It's been wiping me out recently. I am planning to take some time off in October, so I will have another stab at it then if I haven't finished it before.

[alsuren]

I've switched back to using bash, and I've lost momentum on this.

I'm going to close this PR. I will leave the branch up in case anyone wants to cherry-pick from it.

[sholderbach]

That is unfortunate to hear. Thanks for giving it a shot! This PR looked very promising, hopefully someone can take it as inspiration.