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

Closed
wants to merge 1 commit into 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. :)

@jcollie
Copy link
Member

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.

@unknown
Copy link
Contributor

unknown commented Feb 24, 2025

@jnichols0 Any progress on this? If not, I'd be happy to work on it.

@jnichols0
Copy link
Contributor Author

jnichols0 commented Feb 25, 2025

@jnichols0 Any progress on this? If not, I'd be happy to work on it.

Please go for it, I probably don't have time to tackle this soon.

@mitchellh
Copy link
Contributor

Thanks, superseded by #5995

@jnichols0 jnichols0 deleted the fix-copy-on-select branch February 28, 2025 16:23
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.

5 participants