cdown / clipmenu

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

Fix clipdel cutting timestamps from file cache #94

Closed Gravemind closed 5 years ago

Gravemind commented 6 years ago

The actual bug I encountered was that clipdel apparently re-ordered the clipmenu entries (seen when writing my own clipmenu-del).

Looking into it, I found clipdel cuted the timestamp column from the line cache file!

So, I tried to fix this, and keep the pattern retro-compatible:

Turns out sed is quite powerful, I discovered more sed commands that did the trick. But there could be a better solution I didn't see.

sed command h and g seems standard (at least non-gnu compatible), but I did not actually test it with a non-gnu sed (on bsd ?).

Also, I thought of using sed -i instead of for+mktemp, but I know there might be compatibility issues with non-gnu-sed, so I dropped it...

cdown commented 6 years ago

Bleh, this is bad and highlights lack of support for things I don't really use (like clipdel, which is basically only used by external people). I really should get around to #78 some time.

As for the actual solution, wouldn't removing cut and doing LC_ALL=C sed "\\#^[0-9]\+ ${esc_pattern}#d" do? Untested, but logically seems to make sense to me.

Gravemind commented 6 years ago

doing LC_ALL=C sed "\\#^[0-9]\+ ${esc_pattern}#d" do?

I thought of that, but then clipdel -d '^exact_match$' wouldn't work anymore (breaks the regexp: ^[0-9]+ ^exact_match$...).

Gravemind commented 6 years ago

(Not sure if you were talking about that, but I just re-pushed without a last minute mistake I made in the regex: [0-9]+ instead of [0-9]\+ in s#^[0-9]\+ ##;\\#${esc_pattern}#)

cdown commented 6 years ago

Hmm, you're right. Let me think a bit about this and get back to you.

cdown commented 5 years ago

Sorry, forgot all about this. Clearly I thought too much, so merged. Thanks for your contribution, sorry for the delay.