zed-industries
linux: Fix backspace copying characters to primary clipboard
Closes https://github.com/zed-industries/zed/issues/14362
This PR moves the primary clipboard writing logic of the editor crate to mouse up, instead of selections_did_change. This prevents actions like Backspace from copying to the primary clipboard.
Selecting with the keyboard also no longer copies to the primary clipboard, but this seems to vary from program to program.
Release Notes:
- Linux: Fixed backspace copying characters to primary clipboard.
| Warnings | |
|---|---|
| :warning: |
This PR includes links to the following GitHub Issues: #14362
If this PR aims to close an issue, please include a |
Generated by :no_entry_sign: dangerJS against 826e8ad411e41c4db306f4ddad03b0f3378ecb6c
Backspace and Copying reminded me of the keybinding showing up weirdly in my linux system (fedora 40, wayland). I suppose this is a different issue. Is this a known issue or should I file a new issue. I think Cut, Copy and Paste should be Ctrl+X, Ctrl+C and Ctrl+V?
Backspace and Copying reminded me of the keybinding showing up weirdly in my linux system (fedora 40, wayland). I suppose this is a different issue. Is this a known issue or should I file a new issue. I think Cut, Copy and Paste should be Ctrl+X, Ctrl+C and Ctrl+V?
The keyboard shortcuts shown in the menu and the ones you mentioned are all valid for their respective actions. I agree that the shown keybindings should be Ctrl+X, Ctrl+C and Ctrl+V (since those are what all other programs show). IIRC this is because of how GPUI handles keybindings internally (insertion order is ignored). I haven't really looked at that code, so I might be wrong.
Nevertheless, that issue is different from what this PR is trying to address. You should file a new issue if one doesn't already exist.
@apricotbucket28 This seems like a reasonable change to me, is it still a draft for a reason?
@ConradIrwin The previous primary clipboard implementation was broken because deleting a character actually selects the character before the cursor, and replaces that selection with nothing. This would fix this issue, but introduces a new one where selecting text with the keyboard would not add it to the primary, only when selecting it with the mouse.
The real solution would most likely introduce some sort of intent given to the function that selects text, so that it can differentiate between something that happened due to some zed internal implementation details like deleting a character, or if it was a user action.
Makes sense.
I don't know if we have a way to say "user manually changed selection" yet. One idea would be to put it into the Editor::select* methods directly; but that won't catch edge cases like jump to definition / search result which create a selection (though maybe that's actually good?).
Another idea would be to register for the SelectionsChanged event from the editor - this will only be fired once per update so you won't see the intermediate selected characters in backspace - would that work?
Unfortunately it seems like the SelectionsChanged event is fired from the same place that currently writes to the primary clipboard:
https://github.com/zed-industries/zed/blob/b1a581e81b6eaa951a7ca195cb0c46c335d0d5c7/crates/editor/src/editor.rs#L2288
https://github.com/zed-industries/zed/blob/b1a581e81b6eaa951a7ca195cb0c46c335d0d5c7/crates/editor/src/editor.rs#L2415
@apricotbucket28 Right, but it's not received by listener until after the current cx.update has completed (at which point backspace will have resolved).
@apricotbucket28 do you want to pair on this? https://calendly.com/conradirwin/pairing
Sorry, I've been quite busy and missed your reply! I'd like to fix this but currently don't have the time. Feel free to use the code in this branch if it helps.