Fixing DOCUMENTS key and raised exception

[Nibodhika]

Hello, I was trying to use plyer and I found that I couldn't access document folder on my Linux, this was because of two reasons:

 with open(self._get_home_dir() + USER_DIRS, "r") as f:

raises a FileNotFoundError if ~/.config/user-dirs.dirs doesn't exist (but the current code is only handling the case of KeyError, which happens if the key does not exist in user-dirs).

Also line 47 was sending DOCUMENT which worked for the user-dirs.dirs because it started with the same characters, but failed when trying to access PATHS because the key is DOCUMENTS

[Nibodhika]

Also, I didn't wanted to change that because it might be a reason, but shouldn't the line 33 be:

return self._get_home_dir() + PATHS[name]

Otherwise if the first check fails the function returns /Documents instead of ~/Documents

[Nibodhika]

Since no one is replying with any counter-argument, I went ahead and fixed it in case this is merged.

[ghost]

Hello! Thanks for the changes, I left some reviews.

[ghost]

This is right, according to https://cgit.freedesktop.org/xdg/xdg-user-dirs/tree/user-dirs.defaults.

[ghost]

Although this change will return the desired path, I don't agree with having default paths and catching the FIleNotFoundError here. PATHS is assuming that by default all those folders are called like that, which is not always correct. I think the developer is the one who should catch the FileNotFoundError exception, and not plyer. For example:

try:
    storagepath.get_documents_dir()
except FileNotFoundError:
    """
    Show a message that the script could not find the folder location. Perhaps
    you can let the user set the Documents directory manually.
    """

[Nibodhika]

In that case I think other changes should be made, the code is already returning a default value if the key is not present in the file, which is prone to the same sort of errors, and that would raise a KeyError exception.

Also while I agree with you that there are good reasons to let the exception propagate, I would like to suggest a few changes:

1. We should create a unified exception for both cases so that the user should not need to catch multiple exceptions.

2. If the default folder exists it should be returned instead of raising the exception.

3. There should be a parameter or a flag to create the default folder if it doesn't exist instead of raising an exception, and this should be the default.

IMO we need to think that this is just one platform of a multi-platform app and users shouldn't be expected to know that a specific platform can fail due to a quirk of how the platform manages folders, nor should they have to create platform specific code outside of plyer.

[ghost]

It's a good idea! For that, I think another pull request should be opened, leaving this one for the DOCUMENTS fix.

[Nibodhika]

Out of curiosity, why do you think two pull requests should be made for this instead of just making the changes in this PR?

[ghost]

This is not a rule, but...

I think the DOCUMENTS fix is ok, so it can be merged instantly. The other part of your pull request is not complete and might change. Although it is not explicit, in here says:

2. Create a new, appropriately named branch in your local repository for that specific feature or bugfix. (Keeping a new branch per feature makes sure we can easily pull in your changes without pulling any other stuff that is not supposed to be pulled.)

Ideally there should not be a pull request to solve this and that, but one for each bugfix. Here you are presenting at least two different fixes in one plyer facade. Having them in different pull requests is helpful, because as I said, the DOCUMENTS fix is mergeable, and the other changes can be discussed without interfering the first fix.

[Nibodhika]

Just created a new commit to revert the other exception handling (I think they should be merged though, but I didn't wanted to do it to keep the history until this is ready to merge), I'll be working on the way I think linux systems that don't have /.config/user-dirs.dirs should be handled and we can discuss the details in that PR. But I think it should return a default folder instead of failing, especially to keep consistency with other systems where a failure in this method is not expected.