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

Add OSC 52 clipboard setting command and a Windows equivalent #697

Closed
wants to merge 2 commits into from

Conversation

groves
Copy link
Contributor

@groves groves commented Aug 1, 2022

I'm using OSC 52 in helix-editor/helix#3220. I'd like to pull it up into crossterm since it's a generally useful thing.

@sigmaSd
Copy link
Contributor

sigmaSd commented Aug 1, 2022

I think it should be behind a compile time flag since it adds new dependencies.

Does this work on wayland or only on x11?

@groves
Copy link
Contributor Author

groves commented Aug 2, 2022

I think it should be behind a compile time flag since it adds new dependencies.

Forgive my lack of Rust knowledge, but is that generally what one does for new dependencies on a library? Or is it because in this case it's for a feature that not everyone will use?

On the corresponding Helix issue, it was suggested that we add a simple base64 implementation rather than pulling in a dependency. I could do that here if it seems reasonable.

Does this work on wayland or only on x11?

It does work on Wayland: https://wiki.gnome.org/Initiatives/Wayland/PrimarySelection. Maybe I should say "Linux" instead of X11? Not sure what the correct general term is.

@sigmaSd
Copy link
Contributor

sigmaSd commented Aug 2, 2022

It's not a general rust thing, it's just crossterm has strived to keep minimal dependencies, there is even a section on the readme explaining what each dependency is for.

Also projects using crossterm so far will pay the cost for new dependencies while not using the new feature.
I guess this is more relevant to crossterm since its a lowish level library and if we start acumlating dependencies at this level it will make the chain bloated.

If the manual implementation of base64 has no drawback I would vote for it.

Regarding terminology I'm not sure either, my opinion is if it works on wayland let's just remove x11 wording because its usually indicates that it relies on x11 server

@TimonPost
Copy link
Member

TimonPost commented Aug 2, 2022

Thanks for opening a PR, and all your contributions. Yes, up to now very few people have requested the feature for over 4 years. So adding a feature that is an edge case and includes new dependencies for just that feature is a bit too much overhead.

On another note, crossterm aims to be minimal in what functionality it provides. It tries to only provides on the necessarily API for interfacing with the terminal. We might argue clipboard is not really something we want in crossterm by design. While it is a fun feature to have I think we might want to keep it in user space rather than in crossterm. Hence, you could create a crate that has this feature as a crossterm-plugin. If you think otherwise feel free to elaborate.

@groves
Copy link
Contributor Author

groves commented Aug 3, 2022

Makes sense to me. Closing!

One question that comes up now that I'll be putting this directly into Helix. The thing that got me to think about creating a Command was that I didn't see an obvious Command for printing arbitrary terminal codes. I initially used crossterm::style::Print, but that seemed like a misuse of something for styling. Is there a better builtin command for writing arbitrary terminal codes?

@groves groves closed this Aug 3, 2022
@TimonPost
Copy link
Member

You can perfectly build your own commands in helix as you wish. The ´Command´ trait is exposed and you can just create an implementation for that as you desire.

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.

3 participants