Add clipboard provider configuration

[AlfGalf]

This change adds the clipboard-provider setting to the editor section of configuration.

This option can have values of:

• none (use an internal buffer) (on windows only)
• windows (use native windows clipboard) (on MacOS only)
• macos (use pbcopy/pbpaste) (on neiter of the above)
• wayland
• xclip
• xsel
• win23yank (for wsl) (on all targets with "term")
• termux
• tmux
• term (osc codes)
• custom (see below for the configuration)

Note for a custom provider the configurations should look like:

[editor.clipboard-provider.config]
copy = ["tee", "test.txt"]
paste = ["cat", "test.txt"]
primary-copy = ["tee", "test-primary.txt"] # optional
primary-copy = ["cat", "test-primary.txt"] # optional

This can be configured at runtime with the usual:

set clipboard-provider term

Note: I was unable to work out a syntax expression for setting a custom provider at runtime. In my opinion this is probably a fine limitation to have but I am curious if there is a correct way I couldn't work out.

This ports over the previous provider selection logic so hopefully the same default behaviour should apply.

I updated the health command to reflect the provider. Note: this required reading the user configurations within the health command which warrants discussion as this seems to not have been done before.

This is my first contribution, I am a C++ developer by profession and a rust hobyist at best so nits and style updates very welcome.

Note: This adds the nonempty crate as a new dependency. This is because I wanted a way to fail parsing custom commands if they were empty and didn't want to write custom parsign Serde code. I do not know what the process is for considering new dependencies and am very willing to change this for an alternative solution when presented with a better one! I looked for a serde annotation as I thought that was likely but couldn't find one.

[AlfGalf]

Predictably it wont build on machines other than mine 🙃. I'm going to look into cross compiling and clean this up

[AlfGalf]

Okay as far as I can tell this is working on Windows, Ubuntu, and MacOS

[AlfGalf]

This is a fix for #8826 by the way

[markstos]

I would personally put even fewer defaults in the core: target covering 80 to 95% of users. So: Mac, Windows, X11, Wayland, maybe term... and that's it? Once the plugin system is online, I imagine people will be able to provide plugins that support less common options.

[AlfGalf]

I would personally put even fewer defaults in the core: target covering 80 to 95% of users. So: Mac, Windows, X11, Wayland, maybe term... and that's it? Once the plugin system is online, I imagine people will be able to provide plugins that support less common options.

Ive matched the previous modes and default behavior.
I would be hesitant to remove any supported mode as that may create regressions for users. (Though maybe that comes from too much compiler development)

[AlfGalf]

Ping for this PR?
Do I need to do anything to request reviews?

[kirawi]

There's nothing you need to do. It seems like two of the main maintainers are away from Helix right now, so you may have to wait a little.

[AlfGalf]

There's nothing you need to do. It seems like two of the main maintainers are away from Helix right now, so you may have to wait a little.

Ahhh okay great thank you

[the-mikedavis]

I believe this will reset the internal buffers (within NoneProvider for example) when you reload config even if you don't change your clipboard config. I'm not sure there's an easy way to fix this though. We could consider a much larger refactor for clipboard providers to remove state from the clipboard providers (since we store yanks and pastes on the Registers type anyways), and probably remove the clipboard provider trait and just use the enum directly. I'm not sure how large of a change this would take though - I will try out some changes locally and revisit this

[AlfGalf]

Yeah, I can think of some gross solutions to this, but this seemed like the least bad option?

[AlfGalf]

Sorry for the delay, Ive been traveling, I will try address those comments!

[AlfGalf]

Just rebased this onto master

[AlfGalf]

Ping?

[AlfGalf]

I have made the required changes so I think the "waiting on author" isn't correct

[the-mikedavis]

The current tag is waiting-on-review

[AlfGalf]

Oh my apologies! I completely misread the log

[AlfGalf]

Rebased this to master

[AlfGalf]

I have also added documentation for this option.

[the-mikedavis]

With the current changes introducing a ClipboardConfig enum I think it's the right time to refactor this module to use an enum rather than a trait and solve the refactor I was thinking of at the same time (dropping the internal buffer from FallbackProvider and switching set_contents to take &self rather than &mut self).

There are a bunch of things about the clipboard module that could use a refactor:

• We should eliminate the ClipboardProvider trait and replace it with an enum very similar to what you have in ClipboardConfig. There was discussion along these lines when the trait was introduced in Add system clipboard yank and paste commands #310 wondering whether it was worthwhile to use a trait here. I believe the trait makes this module hard to follow and adds some extra complexity we're better off without.
• Currently we use anyhow's Result and Error types here. Instead we should introduce a ClipboardError error type using thiserror (which we already depend on in helix-view). For ClipboardProvider::get_contents for the OSC52 provider we don't implement reading so we should return an appropriate error. In the Registers type and module we can match on this error and use the internal buffer instead, eliminating the need for the FallbackProvider to have extra buffers.
• There's a lot of conditional compilation. We need to keep some of it - only building in support for OSC52 and Windows on appropriate platforms and features - but we can eliminate some of it like that the FallbackProvider is both a sort of "none" and a "termcode" provider depending on platform. Those can be different ClipboardProvider enum members. The conditional compilation also mixes a few states together. Windows now supports OSC52 for example so we should gate the crossterm command's compilation only on #[cfg(feature = "term")] where currently it's based on that plus #[cfg(target_os = "windows")]. We use conditional compilation to remove any dependencies on std::process for WASM targets. I would prefer to just define a dummy ClipboardProvider type for WASM that is always a no-op since WASM will not be able to interface with external commands.
• We use the term "copy" which is inconsistent with the rest of the editor: we should use "yank" instead.

I would also drop the use of NonEmpty here. It's pretty small but for the case of specifying external commands we should match what we do for language configuration and use a command and optional (defaulted) args, for example { command = "tmux", args = ["save-buffer", "-"] }.

I have an incomplete draft of what I'm thinking in this commit. The idea for being able to configure any command would be to add a member to that enum mostly identical to ClipboardConfig::Custom but reusing CommandProvider. Then you could choose between the builtin providers in config by using serde to lowercase/kebab-case their names, like "wl-clipboard", rather than introducing an extra name field.

[AlfGalf]

Sorry for the delay on this, been a bit overwhelmed with the day job and life.

The latest patch updates this inline with what you suggested above hopefully. I think its in a reasonable state.

The only sins I'm not too comfortable about are the CommandProvider and CommandProviderStatic structs which I dreamed up to please the ownership system. If there's a better way to achieve this I would be all ears.

[AlfGalf]

Sorry for the force pushes, tested on my remote Linux box to check building.

[AlfGalf]

There's some windows stuff I don't understand here, that I'd like a look over. Not entirely sure what execute_winapi does.
I don't have a windows machine to test on but my ancient Linux box did build for Linux and Windows so there aren't anymore build failures or Linter failures as far as I can tell.

[AlfGalf]

I think that's all the comments resolved. Building on Linux and windows just fine also.

[AlfGalf]

Thank you for the help with Cow and DynAccess also, Cow at least I had tried to get working and so thats nice to see working

[AlfGalf]

Ahh yes, makes sense, resolved