Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace copy_to_clipboard code with arboard #76

Closed
Ninjani opened this issue Oct 9, 2020 · 12 comments · Fixed by #118
Closed

Replace copy_to_clipboard code with arboard #76

Ninjani opened this issue Oct 9, 2020 · 12 comments · Fixed by #118
Labels
good first issue Good for newcomers

Comments

@Ninjani
Copy link
Member

Ninjani commented Oct 9, 2020

arboard seems to do the right thing when it comes to X11 clipboard handling. Should allow replacing the current very ugly xclip/pbcopy hack in utils.rs. Also, replacing clipboard with arboard in tests/cli.rs should allow copy tests to run on Linux CI as well, which is ideal.

@jonasmalacofilho
Copy link

I started to work on this, just for fun, but I'm having some problems getting unrelated tests to pass, even before any changes:


failures:

---- add_two_cmd_snippets stdout ----
thread 'add_two_cmd_snippets' panicked at 'assertion failed: add_two_cmd_snippets_rexpect(config_file).is_ok()', tests/cli.rs:231:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- change_snippet stdout ----
thread 'change_snippet' panicked at 'assertion failed: change_snippet_rexpect(config_file).is_ok()', tests/cli.rs:240:5

---- add_two_snippets stdout ----
thread 'add_two_snippets' panicked at 'assertion failed: add_two_snippets_rexpect(config_file).is_ok()', tests/cli.rs:222:5

Additionally, the change_snippet test does not appear to be deterministic, and only sometimes fails.

Do you have any idea of what may be going on? Is there some special setup that I need first? I'm having a hard time understanding how these tests work, so I'm haven't been able to figure out what's missing.


Back to the clipboard issue itself, the changes are very straightforward, but I noticed and just reported a small issue with arboard on Linux (ArturKovacs/arboard#4).

@Ninjani
Copy link
Member Author

Ninjani commented Oct 16, 2020

Hi, thanks for looking into this! The tests need to be run with a single thread (cargo test -- --test-threads=1) since the same environment variable is changed separately for each test. I'll add a contributing file with this info.

@jonasmalacofilho
Copy link

I figured this could be it but, err, got the options wrong and was trying --jobs 🤦

@Ninjani
Copy link
Member Author

Ninjani commented Oct 16, 2020

Haha yeah cargo test has an unbelievable number of options

@tpoliaw
Copy link
Contributor

tpoliaw commented Sep 23, 2021

Doesn't look like there's much happening here for the last year, @jonasmalacofilho, are you still on this?
Was there a reason for choosing arboard over the more widely used copypasta fork of rust-clipboard?

@Ninjani
Copy link
Member Author

Ninjani commented Sep 23, 2021

Was there a reason for choosing arboard over the more widely used copypasta fork of rust-clipboard?

So with copypasta there's this issue (or expected behavior I suppose) that clipboard contents are cleared after the program exits (alacritty/copypasta#5). arboard solves this without the need for integrating another clipboard manager.

An additional point to add to this issue is Android support - right now it's done with termux-clipboard-set, would be nice to keep this while changing the others to arboard

@Jarsop
Copy link
Contributor

Jarsop commented Dec 31, 2021

Hello, I would like to work on this issue but it seems that the maintainer of arboard plans to stop maintaining this repository soon and archive it. Do you have any ideas/preferences for a replacement ?

@Ninjani
Copy link
Member Author

Ninjani commented Jan 1, 2022

Thanks @Jarsop. It seems like there won't be a one size fits all library here so maybe it makes most sense to continue the way it is now and shell out to external programs.

In that case, the program used should be a config variable (as requested here) and should support xclip, pbcopy, xsel, termux-clipboard-set, and wl-copy for now. Would be good if it's organized in a way that makes it easy to add more such programs if people request them.

Thoughts?

@Jarsop
Copy link
Contributor

Jarsop commented Jan 8, 2022

I suggest adding an array of CopyWrapper structures that could be selected by a variable in the configuration file (with a default value in missing cases and for the backward compatibility). What do you think about it?

@Ninjani
Copy link
Member Author

Ninjani commented Jan 8, 2022

Yes, sounds good. The default value can be set using conditional compilation on the detected OS (so these lines would move here)

@Jarsop
Copy link
Contributor

Jarsop commented Jan 8, 2022

Yes we are aligned :)

@Jarsop
Copy link
Contributor

Jarsop commented Jan 8, 2022

I have a first implementation here. If you have any suggestions, they are welcome.

@Ninjani Ninjani linked a pull request Feb 2, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants