cdown / clipmenu

Clipboard management using dmenu
MIT License
1.11k stars 90 forks source link

Add support to ignore the clipboard in certain windows #89

Closed Sohalt closed 5 years ago

Sohalt commented 5 years ago

The clipboard does not get recorded when the title of the currenlty active window matches the regular expression in CM_IGNORE_WINDOW. This allows copying passwords from a password manager without the passwords ending up in clipmenu.

The matching is not 100% exact however, as there is a race condition bewteen the time the clipboard is populated, clipmenu queries the clipboard, and the active window gets queried. This race condition can be especially problematic when using polling with large intervals instead of clipnotify.

This implements #86.

Sohalt commented 5 years ago

Maybe the dependency on xdotool should be documented somewhere in the README or help text, as well as the race condition. And maybe the implemented functionality should only work when clipnotify is installed, because without, the possibility for sensitive data ending up in the clipboard is too high and the feature working somewhat might give the user a false sense of security.

cdown commented 5 years ago

Looks generally sane, just a few things to change. Please do also add some code which disables this if it's set and xdotool is not present with a warning. Thanks!

cdown commented 5 years ago

With your hcanges it looks fine, although I see no way to accept your changes here and amend the commit, so you'll need to rebase your changes onto your existing commit locally probably.

Travis failure looks like it should be fixed by these changes.

cdown commented 5 years ago

Thanks! Just waiting for CI.

Sohalt commented 5 years ago

Do you want me to squash all the commits into one?

cdown commented 5 years ago

No worries, I can squash them on merge.

cdown commented 5 years ago

Thanks! Merged and will go into next release.

Sohalt commented 5 years ago

Thank you for the great review!