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

Support primary clipboard #548

Merged
merged 31 commits into from
Aug 12, 2021
Merged

Conversation

dsseng
Copy link
Contributor

@dsseng dsseng commented Aug 4, 2021

Fixes #532

Also some refactoring is done for fallback

TODO:

  • In-memory fallback (none provider now stores buffer in memory for copy/paste inside the Helix instance)
  • Struct support for primary selection
  • wl-copy/wl-paste
  • Command access to primary clipboard
  • Handle mouse events and write to primary buffer
  • Replace/paste multiple from primary selection
  • xsel
  • Check other tools support
    • pbcopy/pbpaste
    • wl-copy/wl-paste ✔️
    • xclip ✔️
    • xsel ✔️
    • lemonade
    • doitclient
    • win32yank
    • termux
    • tmux
    • windows

helix-view/src/clipboard.rs Outdated Show resolved Hide resolved
@dsseng dsseng marked this pull request as ready for review August 4, 2021 11:06
@dsseng dsseng changed the title Support primary selection Support primary clipboard Aug 4, 2021
@cessen
Copy link
Contributor

cessen commented Aug 4, 2021

As I mentioned in #532 please make these behaviors disable-able in the config.

Although I do think we should support these behaviors for people who want them, it's also a behavior that really annoys some people (myself included). I actually go to great lengths to disable the behavior on my local Linux system, and having it hard-coded in Helix would subvert that in my now-primary editor.

I'm even fine having it enabled by default. I just want to be able to disable it in my config.

@dsseng
Copy link
Contributor Author

dsseng commented Aug 4, 2021

Sure, I'll do that option now

Signed-off-by: Dmitry Sharshakov <[email protected]>
helix-term/src/config.rs Outdated Show resolved Hide resolved
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
@dsseng
Copy link
Contributor Author

dsseng commented Aug 5, 2021

I've decided not to use combined wl-copy + xsel, since it will still be inconvenient for those who do not use Xwayland and use on-demand start of it. Anyway, if somebody wants to change default provider, we should add some config option later.

helix-view/src/clipboard.rs Outdated Show resolved Hide resolved
helix-view/src/clipboard.rs Outdated Show resolved Hide resolved
helix-view/src/clipboard.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
helix-term/src/application.rs Outdated Show resolved Hide resolved
@dsseng dsseng requested a review from sudormrfbin August 8, 2021 11:37
Copy link
Member

@sudormrfbin sudormrfbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and tested on linux.

@@ -37,18 +69,24 @@ pub fn get_clipboard_provider() -> Box<dyn ClipboardProvider> {
command_provider! {
paste => "wl-paste", "--no-newline";
copy => "wl-copy", "--type", "text/plain";
primary_paste => "wl-paste", "-p", "--no-newline";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the config says middle_click_paste, I'd rename all references in this file from primary to middle_click too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of window systems (both X11 and Wayland) it's named primary selection, as well as in tools used for access to it. Config option is for UI actually, and we still have commands for accessing both clipboards, so setting actually disables mouse behavior (copy on select and paste on middle click), not the buffer.

}
} else if env_var_is_set("DISPLAY") && exists("xsel") && is_exit_success("xsel", &["-o", "-b"])
{
// FIXME: check performance of is_exit_success
command_provider! {
paste => "xsel", "-o", "-b";
copy => "xsel", "--nodetach", "-i", "-b";
primary_paste => "xsel", "-o";
primary_copy => "xsel", "-i";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why --nodetach is not used for the primary one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I has a freeze with it (xsel stuck untill I killed it via system monitor). It works fine without --nodetach, not sure why it was necessary there. If I'm incorrect in removing that I can test it again and re-enable.

Copy link
Member

@CBenoit CBenoit Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t know exactly why, but they use it in neovim.
Exact command used is xsel --nodetach -i -p (-p is default as you know, so it’s identical except for the --nodetach).
IIRC this is related to stdin/out stuff, but if it works without, I guess it’s fine 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be okay. I'll leave this thread open for other reviewers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CBenoit I don't think all neovim's options are directly portable. I had issues with wl-copy --foreground for example since the command would never return and stall out: 994ff4b#diff-68b83255fe92299d6a62dcca53412b8976d5eda55d3640b4f0dcec474249ba27

@archseer archseer merged commit 7d51805 into helix-editor:master Aug 12, 2021
@dsseng dsseng deleted the primary-clipboard branch August 12, 2021 04:39
@cessen
Copy link
Contributor

cessen commented Aug 12, 2021

How do I disable this in my config.toml? I can disable the entirety of mouse support like this:

[editor]
mouse = false

But middle_click_paste = false doesn't seem to do anything.

@dsseng
Copy link
Contributor Author

dsseng commented Aug 13, 2021

@cessen they are renamed to kebab-case (#[serde(rename_all = "kebab-case", default)]), so use middle-click-paste = false.

@cessen
Copy link
Contributor

cessen commented Aug 13, 2021

@sh7dm Thanks!

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.

Middle click paste and primary clipboard
6 participants