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 clipdel cutting timestamps from file cache #94

Merged
merged 1 commit into from
Apr 28, 2019

Conversation

Gravemind
Copy link
Contributor

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:

  • still using the pattern in sed
  • not using sed -E
  • not break clipdel -d '^exact_match$' (the pattern is matched only against the text, without the timestamp).

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...

@Gravemind Gravemind force-pushed the clipdel-fix-timestamp-deleted branch 3 times, most recently from 43560fa to 64b4dae Compare November 12, 2018 12:06
@cdown
Copy link
Owner

cdown commented Nov 12, 2018

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
Copy link
Contributor Author

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 Gravemind force-pushed the clipdel-fix-timestamp-deleted branch from 64b4dae to fbd4e98 Compare November 12, 2018 14:39
@Gravemind
Copy link
Contributor Author

(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
Copy link
Owner

cdown commented Nov 12, 2018

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

@cdown cdown added the bug label Nov 12, 2018
Clipdel `cut`ed the timestamp column from the line cache files.

For the end-user, it fixes clipdel apparently re-ordering clipmenu
entries.
@Gravemind Gravemind force-pushed the clipdel-fix-timestamp-deleted branch from fbd4e98 to 91542a0 Compare January 23, 2019 17:22
@cdown
Copy link
Owner

cdown commented Apr 28, 2019

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

@cdown cdown merged commit dc20b9c into cdown:develop Apr 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants