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

added :clip command #117

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

added :clip command #117

wants to merge 4 commits into from

Conversation

lyze237
Copy link

@lyze237 lyze237 commented Mar 7, 2018

implemented suggestion from #116

@lyze237
Copy link
Author

lyze237 commented Mar 7, 2018

Ah I just realised that it doesn't put special fields (e.g. otp token, pass) into the clipboard. I'll update the PR in a bit.

@lyze237
Copy link
Author

lyze237 commented Mar 7, 2018

Example:
user :tab pass :enter :clip otp
Writes the username, presses tab, writes the password, presses enter, copies the otp token to the proper clipboard.

Copy link
Contributor

@moviuro moviuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was fast 👍 Some slight changes to do though.

esac
if [[ ! -z "$wordToPrint" ]]; then
if [[ ${inclip} == "true" ]]; then
clipcontent=$(printf '%s%s ' "$clipcontent" "$wordToPrint")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the '%s%s ' supposed to be? Why is there a whitespace at the end?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'%s%s ' basically requires two string arguments. (in that case clipcontent wordToPrint).

I don't know if the whitespace is required but lets look at an example:

If we have:

:clip user pass

without the whitespace the clipboard content after the auto fill would look like: userpass. Which is probably not the expected behaviour, therefore there's a whitespace after it so it turns to user pass.

Copy link
Contributor

@moviuro moviuro Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly you mean '%s %s' instead of '%s%s '?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's say we have this auto pass field: :clip user otp url

Then the content of the clipcontent looks like this (loop iteration -> result):

Look at the whitespaces here:

  • clipcontent=""
  • clipcontent=$(printf '%s%s ' "" "<user>") -> result: clipcontent="<user> "
  • clipcontent=$(printf '%s%s ' "<user> " "<otp>") -> result: clipcontent="<user> <otp> "
  • clipcontent=$(printf '%s%s ' "<user> <otp> " "<url>") -> result: clipcontent="<user> <otp> <url> "

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the whitespace at the end of the string not pose any issue?

if [[ ${inclip} == "true" ]]; then
clipcontent=$(printf '%s%s ' "$clipcontent" "$wordToPrint")
else
printf '%s' "$wordToPrint" | xdotool type --clearmodifiers --file -
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably exile xdotool type --clearmodifiers --file - into a function of its own. Certainly for another PR, though.

rofi-pass Outdated
@@ -72,21 +72,35 @@ checkIfPass () {

autopass () {
x_repeat_enabled=$(xset q | awk '/auto repeat:/ {print $3}')
inclip='false'
clipcontent=""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should put both inclip and clipcontent as local variables.

README.md Outdated
@@ -42,6 +42,7 @@ in a convenient way using [rofi](https://github.com/DaveDavenport/rofi).
respectively.
`:otp` will generate an OTP, either `pass-otp(1)` style, or according to the
`otp_method:`, if it is defined.
`:clip` puts every field after it into the clipboard.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add an example.

@lyze237
Copy link
Author

lyze237 commented Mar 8, 2018

Alright, I'll update the PR on sunday or monday once I'm at home from vacation again!

@carnager
Copy link
Owner

carnager commented Mar 8, 2018

What exactly is the use case for this, by the way?
Also when this is updated, it's probably time for another release

@lyze237
Copy link
Author

lyze237 commented Mar 8, 2018 via email

@carnager
Copy link
Owner

carnager commented Mar 8, 2018

I see. Makes sense.

@moviuro
Copy link
Contributor

moviuro commented Mar 9, 2018

Happy side-effect is that we don't even need to wipe the clipboard since it only contains a Time-based OTP.

README.md Outdated
user: foo
url: http://example.com
oauth://[...]
autotype: user pass :clip :otp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that be user :tab pass :clip :otp?

Copy link
Owner

@carnager carnager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use something like
${text%%+([[:space:]])} for removing trailing whitespace?

Needs extglob option of bash though

EDIT which is most likely used as a replacement for the find command anyway in next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants