perf(commands): swap shell output as_ref + clone with take

[RoloEdits]

Current as_ref + clone combo is just to deal with lifetime issues of the loop. Instead, take can take ownership directly, without having to clone . This can improve performance for particularly large shell output.

[the-mikedavis]

Replacing the clone with Option::take is the right choice if an option isn't otherwise used but this option is used between iterations of the loop so this is a behavioral change. Instead of reusing the value in shell_output, using take means we consume it (replacing it with None). So this changes the behavior so that we re-run the shell command for every selection rather than reusing the value from the first iteration. That is almost certainly more expensive than cloning the existing value and can lead to a different behavior for some shell usages. For example !echo $RANDOM<ret> with multiple selections sets a random number for every other selection with this branch and only produces one random number on master

[RoloEdits]

Hmm, I see. Indeed an interesting change in behavior. Something I might actually play around with more on a branch as that could have its own workflow.

I guess all that could be achieved, as far as performance goes, would be to get rid of a single clone by directly assigning the output_shell with result here? And then do an as_ref().unwrap().clone() every other time.

    let mut output: Option<Tendril> = None;
    let mut offset = 0isize;
    for range in selection.ranges() {
        if output.is_none() {
            let fragment = range.slice(text);
            match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
                Ok(result) => {
                    output = Some(result);
                }
                Err(err) => {
                    cx.editor.set_error(err.to_string());
                    return;
                }
            }
        };

        let output = output.as_ref().unwrap().clone();
       
      //...

I can pivot to this change, if I hadn't missed anything, or we can just disregard it altogether. #10952 is still open, and this would conflict with that change, so I could wait for that to merge first before pushing here. Its only a single clone saved, and its already small string optimized, most clones could just end up being copies already, so I'll accept whatever direction you want to go.

[the-mikedavis]

We need to avoid assigning shell_output when pipe is true so that each selection's contents is fed to the shell command. In any case though I don't think it's worth shuffling around this block to avoid a clone per invocation so I will close this out