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

Fix copy_on_select GTK toast spam #5010

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jnichols0
Copy link
Contributor

Fixes core issue for #4800, moves copy_on_select clipboard logic in src/Surface.zig out of setSelection and into handleMouseButton. When left mouse is released and the current selection is not empty, saves selection to clipboard and sends an adw toast. The selection will still be updated as the user drags, however the clipboard will be written to just once when the mouse button is released.

This patch doesn't add a delay to sending the toast as suggested, wasn't sure how to best do that without throwing it in an async function. Open to suggestions. :)

src/Surface.zig Show resolved Hide resolved
src/Surface.zig Show resolved Hide resolved
src/Surface.zig Outdated Show resolved Hide resolved
@jcollie
Copy link
Collaborator

jcollie commented Jan 13, 2025

I think adding a delay is unnecessary since the toast spam is fixed in a better way.

@jnichols0
Copy link
Contributor Author

Latest commit fixes memory leak and removes log line.

@jnichols0 jnichols0 force-pushed the fix-copy-on-select branch 3 times, most recently from a7ec94e to 5934bd5 Compare January 23, 2025 05:01
@mitchellh
Copy link
Contributor

The issue I have with this PR is that it makes it so that non-mouse selection (i.e. selection keybinds) do not update the selection clipboard. I'm not sure if that's intended behavior (I haven't checked other terminals that support it), but the way I currently expect selection to work is that any selection method would result in the selection keyboard being updated.

I think an alternate approach is needed here where maybe setSelection isn't changed, but our mouse events need to stop using it and set terminal.screen.select directly and then we call setSelection at the end (with the existing selection) in order to persist it once to the clipboard and avoid the toast spam.

@jnichols0
Copy link
Contributor Author

Ah I see. I'll take another look.

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.

4 participants