mozilla / ff-tool

INACTIVE - http://mzl.la/ghe-archive - Python CLI tool for downloading desktop Firefox version, managing profiles and test prefs
Mozilla Public License 2.0
4 stars 12 forks source link

Refactor cloud-services centric args #92

Closed rpappalax closed 8 years ago

rpappalax commented 8 years ago

r? @pdehaan

Fixes #76

pdehaan commented 8 years ago

Not sure if its a config issue, or a lack of me specifying params, but this implodes pretty hard:

$ ff --no-download

================================================================================
FF-TOOL: download, install & launch Firefox!
================================================================================

Traceback (most recent call last):
  File "/Users/pdehaan/dev/github/ff-tool/venv/bin/ff", line 9, in <module>
    load_entry_point('ff-tool', 'console_scripts', 'ff')()
  File "/Users/pdehaan/dev/github/ff-tool/fftool/main.py", line 44, in main
    prefs_dirs=options.prefs_dirs,
  File "/Users/pdehaan/dev/github/ff-tool/fftool/firefox_profile.py", line 120, in create_mozprofile
    for path in prefs_paths(prefs_dirs):
  File "/Users/pdehaan/dev/github/ff-tool/fftool/firefox_profile.py", line 60, in prefs_paths
    for prefs_dir in prefs_dirs:
TypeError: 'NoneType' object is not iterable

UPDATE: I believe this is vomiting somewhere around here:

    for path in prefs_paths(prefs_dirs):
        print('PREFS.ADD_FILE(PATH): ' + path)
        prefs.add_file(path)

We probably just need to wrap that :poop: in a None check for prefs_dirs.

pdehaan commented 8 years ago

Apart from my two smallish nits, I think this is fantastic! 🎉

rpappalax commented 8 years ago

awesome man, thanks!

rpappalax commented 8 years ago

@pdehaan: the only thing I didn't address in this PR is your notes on os.path. dhunt explained to me that if we're already using os.path.join this will already create cross-platform compatible paths for us. Using: os.path.normpath(path), abspath, etc. will do things like expand a path into users home dir or flesh out abs path, etc. Seems like there's some fanciness there we could eventually explore, but I'm not even sure those would be useful for us since we're using /c/cygdrive (which probably wouldn't be expanded properly by those functions.)

Thx for all the notes. let me know if anything else that could use enfixination or OK to enmerginate.

rpappalax commented 8 years ago

I believe this also Closes #85.

It's not often that the download link fails, but if an HTTPS ConnectionError happens, we now should be handling that.

pdehaan commented 8 years ago

Beautiful. R+!