One problem that I'm noticing with this change:

• Seems like the editor crashes when attempting to paste into multiple cursors after copying multiple entire lines with multi-cursors.
  • Not sure what the root cause is, but perhaps the do_paste handler is expecting some of the newlines that were made conditional with this change.
  • I'll attempt some more debugging and exploring to see what's the expected format of clipboard content.

I've posted a fix to the crash, but the code itself is not worthy to be merged 😄

But here's my notes on what I've found when digging through the code for do_copy and do_paste:

• Prior to my changes in this code, we would always add a newline (after the first selection) for each selection. Which meant that copying two entire lines would produce an extra newline between the lines.
  • I thought this was a bug, since this extra newline is not counted in the selection length, but it seems to be an intentional padding between selections, since the do_paste handler seems to sometimes account for this extra newline when pasting.
  • Here's the code snipped for the extra newline:

    zed/crates/editor/src/editor.rs

    Lines 12507 to 12512 in 4724aa5

    	for trimmed_range in trimmed_selections {
    	if is_first {
    	is_first = false;
    	} else {
    	text += "\n";
    	}

• With my initial changes, I was conditionally adding that extra newline, since the current logic for copying entire lines will copy the entire line + plus the very beginning of the next line (effectively copying the newline).
  • However this would lead to a crash, because that intentional piece of newline padding would not be present during the do_paste operation, which would cause an out-of-bounds crash because the do_paste handler is expecting the extra newline.
  • Here's the snippet that leads to an out-of-bounds crash:

    zed/crates/editor/src/editor.rs

    Lines 12582 to 12587 in 4724aa5

    	if let Some(clipboard_selection) = clipboard_selections.get(ix) {
    	let end_offset = start_offset + clipboard_selection.len;
    	to_insert = &clipboard_text[start_offset..end_offset];
    	entire_line = clipboard_selection.is_entire_line;
    	start_offset = end_offset + 1;
    	original_indent_column = Some(clipboard_selection.first_line_indent);

    • Notice how we're doing:

        let end_offset = start_offset + clipboard_selection.len;
        // ...
        start_offset = end_offset + 1;

• I've tried modifying the do_paste handler, instead of changing do_copy, but I was running into some issues with properly extracting the clipboard selections without the extra newline and pasting their contents into the correct cursor positions.
  • The solution would essentially allow for the extra newlines, and when the clipboard selections count did not match the current cursor count, it would attempt to scan over the clipboard text and create new editor edits for each clipboard selection (effectively removing the extra newline).

That all being said, I'm wondering what the best approach would be to resolve this issue.

• Do we keep this extra newline padding in the clipboard contents and allow the paste handler to strip it out?
• Or do we adapt the paste handler to not assume there's an extra newline?

@SomeoneToIgnore thanks for the feedback!

I've pushed up another change that handles the editor commands for Cut, Copy, and Paste. I've also revised the tests based on your suggestions, and added a new test for the Cut command.

I've started looking into the Vim commands too, since it seems they also add an extra newline between multi-cursor selections of entire lines (using visual mode with line selection).

Should I continue to make more changes in this PR for the Vim commands?
And is there anywhere else I should check that could be affected by these editor changes?

Thank you for the fix, this looks good as the first glance, but I hope for @ConradIrwin to check the Vim part first to see what should we do about it.

Thanks for the offer! If you'd like to make the same changes to vim mode, happy to accept a second PR, but no need to block on that. I also don't think it's super urgent as I haven't (yet) had any complaints about this – vim users are a little less likely to use multi-cursor actions like this)