DavidoTek
commented
2 months ago on the other hand it may be unexpected if the user doesn't realise they can't write into that directory
Should be fine as the path wouldn't be selected in the first place (except if the user just typed it by hand).
We could optionally make is_valid_custom_install_path a full function and give it a parameter to dictate whether or not we want to check if the selected directory is writeable
Not sure if we need that parameter, but converting it to a full functions might make sense. I think there is no reason to use a lambda over a full function.
should probably make a util function at some point called like sanitize_path or something that can manage expanding paths, and optionally resolving symlinks
We can do that. Needs to consider https://github.com/DavidoTek/ProtonUp-Qt/pull/381 though
Do you want to add anything of the mentioned things here or should I already merge?
sonic2kk
Overview
This PR adds functionality to open the Custom Install Dialog directory at the path in
self.ui.txtInstallDirectoryfirst, if it is a valid directory. If you enter some text manually, or select a folder using the dialog, and then open the dialog again, it will then open from that entered/selected path again, which can be handy if you only enter part of a path and want to browse for the rest, or if you select the wrong path and want to continue navigating from where you left off.If the path is not valid, we fall back to
HOME_DIR.Implementation
We already have a lambda that can check if an install directory is valid in
PupguiCustomInstallDirectoryDialogcalledis_valid_custom_install_path, we use this to enable/disable the "Save" button each time the text intxtInstallDirectoryis changed. So when checking which path to open the dialog with, we re-use this. Ifis_valid_custom_install_pathis not true, then we fall back to usingHOME_DIRas the default location for the dialog.There is a side-effect to this approach of checking the path:
is_valid_custom_install_pathchecks if the path is writeable. So if we enter a path to a folder that is readable but not writeable, then the dialog won't open that directory. On one hand this makes sense, this prevents you from selecting an invalid folder, but on the other hand it may be unexpected if the user doesn't realise they can't write into that directory. When using this lambda for toggling the save button, it makes sense to disable the button if we can't write into the dialog, but maybe it doesn't make as much sense here? Although that would mean we'd have to basically duplicate theis_valid_custom_install_pathcheck.We could optionally make
is_valid_custom_install_patha full function and give it a parameter to dictate whether or not we want to check if the selected directory is writeable. Then we can call it with that parameter as false for checking the path to open the diialog in (meaning we want to ignore if the path is not writeable in that instance) but set the parameter is true when we call it for toggling the "Save" button.Future Work
The path is wrapped in
expandusersince the user could enter a path like~/.local/share/Steam. Setting the directory for the dialog with a path like this results in Qt becoming a little confused and opening the dialog in incorrect paths. We should probably make a util function at some point called likesanitize_pathor something that can manage expanding paths, and optionally resolving symlinks. It might be cleaner than all of the nested path functions we use at the moment but that's future work :sweat_smile:Thanks!