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

Allow the download directory to be different from the current working directory #83

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

triple-j
Copy link
Contributor

Allow the download directory to be different from the current working directory. This makes it easier to run this as a cron job in Linux.


This PR also contains some clean ups to the --human-folders parameter.

Remove the need for the Game class to parse sys.argv. We'll store the option in the Library class and pass it to Game as needed.

Update the ArgumentParser.add_argument() call to make --human-folders a proper on/off flag.

Fix the naming convention for humanFolders. According to Python's PEP 8 Style Guide, function and variable names should be lowercase with words separated by underscores (aka snake_case). The rest of this project seems to follow PEP 8, so humanFolders should be human_folders.

To remove the need for the `Game` class to parse `sys.argv`, we'll store
the option in the `Library` class and pass it to `Game` as needed.

The `ArgumentParser.add_argument()` call has been updated to show up as an
'on/off' argument.

Also fix the naming convention. According to Python's PEP 8 Style Guide,
function and variable names should be lowercase with words separated by
underscores (aka snake_case). The rest of this project seems to follow PEP
8, so `humanFolders` should be `human_folders`.
@Emersont1
Copy link
Owner

Emersont1 commented Apr 16, 2023

could this be achieved more simply by changing the working dir with os.chdir()?

@triple-j
Copy link
Contributor Author

could this be achieved more simply by changing the working dir with os.chdir()?

This has never crossed my mind.

The problem I could see with os.chdir() is that it can be unclear where a file is stored.

Say we use os.chdir() in itchiodl/downloader/__main__.py and leave Path(".") in itchiodl/game.py. If I was looking in itchiodl/game.py it wouldn't cross my mind that Path(".") wouldn't reference the directory where the program was run. Whereas if we use a variable (self.output_path) it is clear to the developer that the path can change.

In more complicated programs os.chdir() could cause more issues. If you need to save files in multiple folders, you'd need to make sure os.chdir() isn't run more than once. Otherwise where a file is saved could change based on where in the program you are.

I just think it is better to be explicit.

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