Fix stuff in mex's code review

[vext01]

https://gist.github.com/egelmex/3f25c78665fedee7129f

[egelmex]

Code review

pep 8 issues

https://github.com/vext01/passout/blob/master/passout.py#L2

imports should be one per line

xdg

https://github.com/vext01/passout/blob/master/passout.py#L4

If followig xdg specs config should be in $XDG_CONFIG_HOME. and dat in $XDG_DATA_HOME https://developer.gnome.org/basedir-spec/

check_dirs

Should raise an error if directory is not found and can't be made.

https://github.com/vext01/passout/blob/master/passout.py#L33

get_pass_file

https://github.com/vext01/passout/blob/master/passout.py#L35

possibly confusing name, returns a path but is called get_X_file.

https://github.com/vext01/passout/blob/master/passout.py#L39

Likewise called pw_file actaully pw_file_path or similar.

lexists vs exists

https://github.com/vext01/passout/blob/master/passout.py#L41

Are you sure you want to use lexists here?

with popen

https://github.com/vext01/passout/blob/master/passout.py#L49

This code could be cleaned up using with popen. See context mangers in https://docs.python.org/3.5/library/subprocess.html

user a config parser

https://github.com/vext01/passout/blob/master/passout.py#L63

Plenty of off the shelf parsers that would make your life easier, python has a built in json one that would work well for this.

python 3.3

https://github.com/vext01/passout/blob/master/passout.py#L1

Do you really need python 3.3 or would python 3 do? I only have python 3.4 isntalled.

setup instructions

"gpg= could be gpg=which gpg ?

mkdir ~/.passout && echo "gpg=\nid=\n" > \ ~/.passout/passoutrc left "\n" in the file instead of new lines

document wht id should be

[vext01]

Thanks for the PR.

pep8

Not really bothered about pep8.

xdg

I was up for this change until i realised that the config and data would be separate.

check dirs

The error message is sufficient, no?

wilfred:~> passout ls                                                          
INFO:root:Creating '/home/edd/.passout/crytpo_store'
Traceback (most recent call last):
  File "/home/edd/eck2/bin/passout", line 201, in <module>
    entrypoint()
  File "/home/edd/eck2/bin/passout", line 180, in entrypoint
    check_dirs()
  File "/home/edd/eck2/bin/passout", line 31, in check_dirs
    os.mkdir(d)
PermissionError: [Errno 13] Permission denied: '/home/edd/.passout/crytpo_store'

get_pass_file

I tend to agree. Will fix.

lexists

IIRC there was an issue with using exists(), but I don't remember now. exists() seems to work and returns True for broken symbolic links sounds wrong for our purposes anyway.

Popen

Sure, I could use a context manager, but communicate() already closes fds and I need that call either way to collect stdout. Have I missed the point?

Config parser

If there is one built into Python which works, then OK, but I don't want to pull any deps in. JSON is a bit overkill for a 2 line config file with no nesting. Do you know a simple K=V parser?

Python version

Pip will rewrite the shebang. This will work on any Python3, but there is no portable way of doing this without pip.

setup instructions

cant use which gpg because I have two binaries gpg and gpg2. I specifically need gpg2 for gpg-agent support.

Id is aready described in the README:

Where `<your_gpg_binary>` is the path to the gpg binary you want to use (I
recommend using version 2) and `<your_gpg_id>` is the email address
associated with the gpg key you want to use with PassOut.

I wonder how I should package up the README. Is there a standard place for docs in a Python module tree?

[vext01]

Will re-open these separately if we decide to fix.

[vext01]

(pep8 was done by @richlanc)