Runtime configuration of clipboard providers

[The0x539]

Commit adf97e0 is mostly reasonable, but as a WSL user, it broke my workflow. The win32yank provider does work on WSL, and it's great.

Rather than simply revert the changes (or specifically the removal of win32yank on Linux), I took the opportunity to make the system more flexible. The majority of providers are already command-based, which really shouldn't require the whole WASM plugin system, hence this implementation. All of Neovim's presets are available, and the user can specify in config.toml which ones should be tried and in what order, as well as defining their own.

[pickfire]

Do we really need a macro for this?

[The0x539]

I mean, there was a macro before, and it didn't reduce boilerplate by nearly as much as these two macros.
This approach handles:

• Converting every argument into a String so that the presets can coexist with deserialized config data.
• Building a map of providers, for by-name lookup.
• Building a list based on order of appearance, for specifying precedence.
• Implicit defaults for env, primary, and test, which are each relatively rare among the presets.

[pickfire]

I think this macro is unnecessary, it's just a couple of lines.

[The0x539]

I generally follow the rule of three for avoiding repetitive code. This is five (or four, without test_commands), and I'm not a fan of how it looks without the macro:

pub fn is_available(&self) -> bool {
    if !self.copy.is_empty() && !exists(&self.copy[0]) {
        return false;
    }
    if !self.paste.is_empty() && !exists(&self.paste[0]) {
        return false;
    }

    if let Some(copy) = &self.primary_copy {
        if !copy.is_empty() && !exists(&copy[0]) {
            return false;
        }
    }
    if let Some(paste) = &self.primary_paste {
        if !paste.is_empty() && !exists(&paste[0]) {
            return false;
        }
    }
    for var in &self.env_vars {
        if !env_var_is_set(var) {
            return false;
        }
    }
    for cmd in &self.test_commands {
        // FIXME: check performance of is_exit_success
        if !cmd.is_empty() && !is_exit_success(&cmd[0], &cmd[1..]) {
            return false;
        }
    }

    true
}

[archseer]

and the user can specify in config.toml which ones should be tried and in what order, as well as defining their own.

I think this is a bit of overkill. We either want to automatically detect a suitable provider and use it, or we want the user to manually specify clipboard = { paste = { cmd = "foo", args = ["bar"] } } that overrides.

[archseer]

Looking at what the code was before, you want to always use the WindowsProvider on windows, and win32yank on linux which will cover WSL.

[The0x539]

Sure, my particular use case doesn't need anything other than clipboard-win on cfg(windows), but I believe the lemonade client also works on Windows.

[The0x539]

I think this is a bit of overkill. We either want to automatically detect a suitable provider and use it, or we want the user to manually specify clipboard = { paste = { cmd = "foo", args = ["bar"] } } that overrides.

This approach makes it easier to have a single portable configuration. I personally use helix primarily in two Linux environments, one of which should use wl-clipboard and one of which should use win32yank. Both of those happen to be available as presets, but I don't want to discount the possibility of other providers that aren't. "Detection xor choice", either as a program decision or as a design decision, would interfere with such a use case, and detection shouldn't be limited to hardcoded presets.
Sure, adding a detection is an easy change to make, but then you're either forgoing your package manager to build from source or waiting for a contribution to get merged and make it into a versioned release.

[pickfire]

This approach makes it easier to have a single portable configuration. I personally use helix primarily in two Linux environments, one of which should use wl-clipboard and one of which should use win32yank

The auto-detection ideally should cover all these cases, the user should only override it to a single specific config. In your case, if the auto-detection is done correctly then it should work without manual config, ideally we don't want users to do any config for this.

[pascalkuthe]

clsoing this one out as stale. We would still like to add the ability to do this but this has siginificant conflicts and open review comments so it makes more sense to start over with a new PR if somebody wants to pick this up. Thank you for contributing!