-
Notifications
You must be signed in to change notification settings - Fork 13
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
Set config directory according to runtime platform #57
base: master
Are you sure you want to change the base?
Conversation
This commit changes the definition of the persistent state and configuration files to derive from the output of [platform.system()][1], rather than being fixed to an eponymous, dot-prefixed directory in the top level of the user's `$HOME`. Furthermore, on POSIX-compliant platforms it tests for the presence of the XDG_CONFIG_HOME environment variable and establishes the config directory there, with a fallback to the `$HOME/.config` directory as defined in the [XDG Base Directory Specification][2]. On Microsoft Windows® systems, it places the directory inside the path defined by the `%LOCALAPPDATA%` environment variable, which is the vendor-specified path for all machine- specific application data belonging to the active user. On all other platforms the former behavior is retained. Several individual definitions of this path were consolidated into the `SOCO_CLI_DIR` constant already present in the file, which itself was relocated along with the attendant `_confirm_soco_cli_dir` method earlier in the file, nearer to the other defined constants. Include updated documentation sections in README.md that relate to where configuration and data are stored on various platforms, and what to delete for a complete uninstall. Validate markup syntax using NodeJS markdownlint module, and supply syntax- highlighting keywords for fenced code blocks a la github-linguist. [1]: https://docs.python.org/3/library/platform.html#platform.system [2]: https://specifications.freedesktop.org/basedir-spec/latest/index.html
4a28d99
to
883318e
Compare
Thanks for the PR and apologies for the delay in getting to it -- I've been busy/travelling. I'll take a look as soon as I can. I should note that it was a deliberate choice to have a consistent location across platforms, but I'm not resistant to revisiting this choice. I'll also need to think about the breaking change aspect -- I don't want to break stuff that users have already created. |
Thanks for considering the changes, @pwt. No major hurry on the review, I've got my hands full too. This is actually my first code contribution to an open source project, so I didn't expect it to be "smooth sailing," either. I understand your concern about the breaking change and had considered adding a method to test for the presence of the old config file path (easy enough since it's hard-coded and platform agnostic, too) and automatically copying it to the location defined in the new logic, but didn't want to add more complexity to the PR before gauging what your attitude was re: avoiding large scale changes vs. backwards compatibility. I mocked up these changes because the jewelry store that I work at uses Sonos speakers to play the background music on the showroom floor as well as in the gallery rooms and while they do a great job, their control surface is not only rigid but difficult to adjust with subtlety. In the gallery rooms there's just a Windows PC running the POS/Inventory system and Sonos controller software, but it's pretty well locked down and Just out of curiosity, where would you stand on adding the ability to provide a signed integer as the argument to the volume commands to indicate a desired deviation from the current volume, whatever that may be? Maybe even with the ability to make such changes be logarithmic instead of linear by appending Anyhow, I'll leave you to ponder all of this and wait for your comments on the matter, whenever you find the time to do so. Thanks for all the work you've put into this amazing tool to date, if I hadn't encountered it in the first place I never would have thought it possible to experiment with my own attempts at crafting a custom Sonos control surface. 👍🏼 |
Thanks for the extra context. I hadn't considered that the home directory might not be writeable (I don't do much development on Windows). That's extra justification for making the change.
Does the |
Does |
It depends what you mean by 'configuration'. SoCo-CLI does store the following:
I wanted any persistent state to be held somewhere obvious, so that it could be deleted without having to hunt for it -- I hate it when apps spray data in obscure places. I'm aware that the the choice of However, @jlmello57 has identified a use case which SoCo-CLI definitely doesn't support with its current choice of directory. I'll incorporate a form of the PR (which is platform-independent) to fix this, and I apologise for my tardiness in doing so. |
The only thing from that list I would call configuration are the aliases, a feature that I didn't even realize |
This pull request changes the definition of the path for persistent state and configuration files (
SOCO_CLI_DIR
) to derive from the output of platform.system(), rather than being fixed to an eponymous, dot-prefixed directory in the top level of the user's$HOME
. This is a breaking change in that no mechanism is created to test for the presence of the former path and relocate data to the new path on platforms where this is changed.Furthermore, on POSIX-compliant platforms it tests for the presence of the XDG_CONFIG_HOME environment variable and establishes the config directory there, with a fallback to the
$HOME/.config
directory as defined in the XDG Base Directory Specification.On Microsoft Windows® systems, it places the directory inside the path defined by the
%LOCALAPPDATA%
environment variable, which is the vendor-specified path for all machine- specific application data belonging to the active user.On all other platforms the former behavior is retained. Several individual definitions of this path were consolidated into the
SOCO_CLI_DIR
constant already present in the file, which itself was relocated—along with the attendant_confirm_soco_cli_dir
method—earlier in the file, nearer to the other defined constants.