search

cdown / clipmenu

Clipboard management using dmenu
MIT License
1.14k stars 91 forks source link

clipctl disable/enable is too slow with clipmenu 7 #234

Closed vejkse closed 2 weeks ago

vejkse commented 4 weeks ago

Nowadays, because xdotool type --delay 0 "$string" mangles non-ASCII characters, I do this:

  1. clipctl disable
  2. Putting $string in the clipboard.
  3. Pasting the clipboard in the target window.
  4. Restoring the content of the clipboard. (By the way, shouldn’t clipctl enable restore itself the content of the clipboard to whatever is first in the list shown by clipmenu? Now I’m doing it by finding the most recently modified directory in $XDG_RUNTIME_DIR/clipmenu.7.$UID and putting the content of its file 1 in the clipboard.)
  5. clipctl enable

But since I updated to the unreleased (I think) clipmenu 7, to take advantage of the fact that it doesn’t overwrite clips with the same first line, Steps 1 and 5 take a bit more than 100 ms each, which leads to a sensible delay when ‘typing’ in this way.

By looking at the code, I see this is due to a sleep command: https://github.com/cdown/clipmenu/blob/1113ca8df96615aba67aa4494c7c43fb56f3dfde/src/clipctl.c#L96

I understand it is needed to ensure clipmenu is really disabled/enabled before quitting, but isn’t 100 ms too much? Are there circumstances where it takes that long to disable itself?

As a workaround, I tried just backgrounding it in step 1 above: clipctl disable &, but this doesn’t leave enough time for clipmenud to disable itself and the $string is recorded by clipmenud.

Now I’m using another workaround: when launching clipmenud in xinitrc, I cache its PID in $XDG_RUNTIME_DIR/clipmenu.7.$UID/pid and in step 1 above, I just do kill -USR1 "$(<"$XDG_RUNTIME_DIR/clipmenu.7.$UID/pid")" as the old Bash clipctl did (without the caching of the PID). With that, I didn’t have a leak of $string in any of my tests.

So I’m wondering if it would not be possible to check every 1 ms or so rather than every 100 ms, or first every 1 ms and then every 100 ms for the exceptionally slow cases?

cdown commented 3 weeks ago

Huh, 100 msec is indeed pretty high, not sure what I was thinking when I wrote it that way :-) Since MAX_STATE_RETRIES is 20 that would make 2 seconds, which is crazy.

I need to get clipmenu 7 ready for final release and hopefully will have some work on that in the coming weeks. I'll make sure this is also tweaked down. Thanks for the report!

cdown commented 2 weeks ago

Please try out #237 and let me know how it goes, hopefully it will make this better. Thanks for reporting!

vejkse commented 2 weeks ago

As far as I can tell, everything works fine. Disabling/enabling each take less than 10 ms on average.