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 stdin test #3

Merged
merged 3 commits into from
Mar 25, 2018
Merged

Fix stdin test #3

merged 3 commits into from
Mar 25, 2018

Conversation

5bentz
Copy link
Contributor

@5bentz 5bentz commented Mar 25, 2018

This PR includes a fix of a typo in the code.

When editing, I noticed I could improve the code, thus the rewrite. Using run() is the recommended approach according to Python.org. It requires Python >= 3.5 (released in November 2015).

I have also renamed the pref variables to uppercase since they are constants.
This change highlights the difference between the pref variables &
the other variables in the code.

src/passff.py Outdated
@@ -14,14 +14,14 @@
################################################################################
# Default command for MacOS:
#command = "/usr/local/bin/pass"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please uppercase command -> COMMAND also in this line for Mac users.

'stdout': subprocess.PIPE,
'stderr': subprocess.PIPE,
'env': env
}
if 'stdin' is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thanks 👍

@5bentz
Copy link
Contributor Author

5bentz commented Mar 25, 2018

Force-update done.
Mac users are happy again 👍

@tuxor1337
Copy link
Contributor

tuxor1337 commented Mar 25, 2018

This is not urgent, right? Because it's working for me even without this fix.

@tuxor1337 tuxor1337 merged commit 7821b0b into passff:master Mar 25, 2018
@5bentz
Copy link
Contributor Author

5bentz commented Mar 25, 2018

Not urgent at all. I am sorry for the misunderstanding.

If I'm correct, the code would always create a pipe for stdin, even when the code was not giving any input to pass.
All in all, it must have been harmless. This fix is like an excuse to DRY the code 😃

Thanks for the merge 👍

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.

2 participants