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

clipdel: Support getting pattern from standard input #91

Merged
merged 2 commits into from
Apr 1, 2020

Conversation

ferki
Copy link
Contributor

@ferki ferki commented Oct 27, 2018

I'm using clipmenu together with pass (and passmenu :)), and
wanted to make sure that pass would remove passwords both from the
clipboard and the clipboard history.

I had to slightly patch pass for that locally, where I also figured it
would be best to simply pipe the password string into clipdel, instead
of passing it as an argument.

This patch makes clipdel look for a pattern coming from standard input
if there's no pattern passed as an argument.

Please review, and let me know if you need me to improve it, or to open
an issue instead for further discussions.

@cdown
Copy link
Owner

cdown commented Oct 27, 2018

Thanks! Conceptually this looks fine, just a couple of things:

  1. User experience is not great if invoked incorrectly now (ie. you don't get the "no pattern provided, see --help" message). This can be mitigated by checking if connected to a terminal with [[ -t 1 ]].
  2. Ideally use shell builtins where available (it's likely not harmful in this case, but they have fewer shenanigans than when using $(). You can use read -r with no IFS set for this.

@ferki
Copy link
Contributor Author

ferki commented Jan 5, 2020

Thanks for your patience here!

I'd like to update this PR, and most of the steps are clear to me (rebase on top of current master, and replace cat usage with IFS= read -r).

I think I need some clarifications on the feedback about checking for a connected terminal, though:

  1. User experience is not great if invoked incorrectly now (ie. you don't get the "no pattern provided, see --help" message). This can be mitigated by checking if connected to a terminal with [[ -t 1 ]].

Where should this [[ -t 1 ]] check go? Into the if that decides about printing the error message, or into a new if that decides how to obtain a value for raw_pattern? Perhaps both, or even somewhere else?

@cdown
Copy link
Owner

cdown commented Mar 24, 2020

Sorry this went stale -- this is still okay, though. Probably the terminal check can just be around raw_pattern assignment itself.

@ferki ferki force-pushed the pipe_to_clipdel branch from 4cd8ffc to 953f3c2 Compare March 29, 2020 20:27
@ferki
Copy link
Contributor Author

ferki commented Mar 29, 2020

No worries, thanks for the feedback!

I've rebased my changes on top of current master, and made a more explicit check for the various sources of possible input. Let me know how it fits the codebase now!

clipdel Outdated Show resolved Hide resolved
@ferki ferki force-pushed the pipe_to_clipdel branch from 953f3c2 to fdf156e Compare April 1, 2020 06:53
@cdown
Copy link
Owner

cdown commented Apr 1, 2020

Thanks! Looks good to me.

@cdown cdown merged commit 618327d into cdown:develop Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants