Windows - 'My Music' folder won't be assumed to be on C drive #387
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Windows has a nice registry check to get the absolute path of the 'My Music' folder. This helps because some people change their location of their music folder.
linusg
suggested changes
Oct 5, 2018
spotdl/internals.py
Outdated
| @@ -4,6 +4,10 @@ | |||
|
|
|||
| from spotdl import const | |||
|
|
|||
| try: | |||
| from winreg import * | |||
There was a problem hiding this comment.
No starred imports! Keep the namespace clean...
spotdl/internals.py
Outdated
| # Explorer and Finder are actually in English on the file system. | ||
| # So, defaulting to C:\Users\<user>\Music or /Users/<user>/Music | ||
| # respectively is sufficient. | ||
| # On Linux, default to /home/<user>/Music if the above method failed. |
There was a problem hiding this comment.
Please leave all this in for future reference, at least put it before the final fallback return.
spotdl/internals.py
Outdated
| try: | ||
| key = OpenKey(HKEY_CURRENT_USER, r"Software\Microsoft\Windows\CurrentVersion\Explorer\Shell Folders", 0, KEY_ALL_ACCESS) | ||
| return QueryValueEx(key, "My Music")[0] | ||
| except Exception: |
There was a problem hiding this comment.
Usually no plain excepts ever, might be an exceptional case here. Still would be better to catch possible exceptions explicitly.
Let me know if there are anymore improvements 👍
linusg
suggested changes
Oct 5, 2018
spotdl/internals.py
Outdated
| @@ -179,7 +179,7 @@ def get_unique_tracks(text_file): | |||
| return lines | |||
|
|
|||
|
|
|||
| # Get user's localized music directory | |||
| # a hacky way to user's localized music directory | |||
There was a problem hiding this comment.
lol
Was more thinking about leaving the reference to the original issue here than enforcing a note about the hacky way 😄
linusg
approved these changes
Oct 6, 2018
|
Looks good. Thanks! 😃 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Windows has a nice registry check to get the absolute path of the 'My Music' folder. This helps because some people change their location of their music folder.