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 clipboard with arboard, which is actively maintained #705

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

Tastaturtaste
Copy link
Contributor

Adresses #606 and this request

Replace clipboard dependency with arboard.

Considerations

A look at crates.io shows that there are currently two major cross-platform clipboard crates to choose from:

  1. arboard
  2. copypasta

copypasta is maintained by the alacritty project, which gives some confidence in the future maintenance story. Using this crate as a drop-in replacement for clipboard works just from an API standpoint, but leads to failure of cargo test --features system_clipboard core_editor::clip_buffer::tests::reads_back on windows. This is due this bug and copypastas outdated version of clipboard-win. While this issue could be worked around, it does not instill much confidence in me that future issues will be resolved and dependencies kept up-to-date.

arboard is actively maintained by a corporation depending on it and has roughly three times the number of downloads of copypasta. While the API is not a drop-in replacement for clipboard, the required changes are minimal.

Dependencies

arboard does pull in a few dependencies, mostly depending on the target OS. Many of the dependencies, like log, thiserror, parking_lot (on linux) are already present in reedline's dependency graph anyway. In addition
However, as mentioned in #606, wayland support adds many dependencies to the list.

Number of unique dependencies for linux with system_clipboard enabled after cargo update:
current main: 53
arboard: 59
arboard + wayland: 99

Without having looked further into it copypasta also seems to pull in the majority of dependencies for wayland support. So I guess its mostly a wash in that department when comparing the alternatives.

Copy link

codecov bot commented Jan 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ef7b96c) 49.32% compared to head (397a2e1) 49.32%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #705   +/-   ##
=======================================
  Coverage   49.32%   49.32%           
=======================================
  Files          46       46           
  Lines        7998     7998           
=======================================
  Hits         3945     3945           
  Misses       4053     4053           
Files Coverage Δ
src/core_editor/clip_buffer.rs 100.00% <ø> (ø)

Cargo.toml Show resolved Hide resolved
@fdncred
Copy link
Collaborator

fdncred commented Jan 14, 2024

Looks like pretty minimal changes, very nice! Thanks for this. I just had one further question above and then I think we could move forward.

@fdncred fdncred merged commit dc27ed8 into nushell:main Jan 15, 2024
8 checks passed
@Tastaturtaste Tastaturtaste deleted the clipboard branch January 15, 2024 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants