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

Minor tweaks and cleanups (details are in commit messages). #5

Merged
merged 8 commits into from
Apr 12, 2021
Merged

Minor tweaks and cleanups (details are in commit messages). #5

merged 8 commits into from
Apr 12, 2021

Conversation

patsytau
Copy link

Using str(Path.home() / "Downloads") avoids needing to query the registry and means the get_download_path() can be removed entirely. It also makes in a bit easier to read.

In addition, the directory name also starts with a capital 'D' on Ubuntu.

@patsytau
Copy link
Author

patsytau commented Apr 12, 2021

I made sure the result was the same on Windows 10 by finding the path using both methods before removing the old function:

downloadsFolder = get_download_path()
downloads_path = str(Path.home() / "Downloads")
assert downloadsFolder == downloads_path

Patsy added 7 commits April 12, 2021 08:59
…iting.

Good practice: open(...) throws OSError if a problem occurs, so catch only this exception type.
It's not necessary to say "if x is not None:" or "if x is None:" - "if x:" and "if not x:" are just equivalent but more idiomatic.
In startflash, return early if the process is already running - allows the scope of the rest of the function to be taken outside the conditinal.
@patsytau patsytau changed the title Use pathlib to find "Downloads" directory. Minor tweaks and cleanups (details are in commit messages). Apr 12, 2021
@pfeerick pfeerick merged commit 749f50a into pfeerick:main Apr 12, 2021
@pfeerick
Copy link
Owner

pfeerick commented Apr 12, 2021

Much appreciated, as was the further discussion we had on Discord. This still works fine on both my test Windows and Linux systems... and the removal of the registry lookup for Downloads on Windows in preference to assuming that the downloads folder hasn't been moved doesn't throw any nasty errors, nor does it have an issue if you move it on Linux either, so it's an acceptable tradeoff for much neater code.

Now, I'm just feeding the code through a linter, and it's saying what I thought... I need to fix up Electr0's creative (inconsistent) use of ' and " throughout... plus some minor layout tweaks...

Next up... I'm thinking of making it so the output is saved to a variable, so it can be shown to the user when somethingGoesWrong:tm:)... rather than just saying something went wrong. i.e. pop up a dialog with the log. That will be a job for another day though.

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