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

Change config path logic to support XDG #188

Merged
merged 3 commits into from Jun 9, 2018
Merged

Change config path logic to support XDG #188

merged 3 commits into from Jun 9, 2018

Conversation

rj45man
Copy link
Contributor

@rj45man rj45man commented Jun 5, 2018

Someone in IRC helpfully remarked that:

~/.syncplay belongs in XDG_CONFIG_HOME

Here's a PR that makes the Syncplay client a little more XDG-conscious. I refactored the related code to aid my understanding of it, and to simplify the subsequent XDG changes.

https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

Flisk added 2 commits Jun 5, 2018
The default config path on Linux becomes `~/.config/syncplay.ini`,
provided the user hasn't changed her $XDG_CONFIG_HOME. Existing
installations continue to read their config from `~/.syncplay`, should
that file exist.
@albertosottile
Copy link
Member

@albertosottile albertosottile commented Jun 6, 2018

@sometoby Can you close and reopen this? I want to see if, after the changes that I applied to the Travis script, it passes the check. Thanks.

EDIT: nevermind, I applied the patch in your repository directly, hope that you do not mind this. Thanks again.

@rj45man
Copy link
Contributor Author

@rj45man rj45man commented Jun 6, 2018

Your commit seems to have already triggered the CI, I think we're good.

@Et0h
Copy link
Contributor

@Et0h Et0h commented Jun 6, 2018

Thanks for your work on this. The config file does indeed belong in XDG_CONFIG_HOME where appropriate.

I am not familiar with XDG_CONFIG_HOME so I cannot rule out the possibility that on some non-NT systems there are circumstances where the XDG_CONFIG_HOME environmental value is set. It might be safer for the _getXdgConfigHome code to confirm that XDG_CONFIG_HOME exists (and is not empty/blank/null) before using XDG_CONFIG_HOME to create a directory and return a path.

I tested it on Windows 10 and it was fine both when there was and wasn't an existing .ini file. Not tested Linux, macOS or *BSD.

@albertosottile @daniel-123 @nilsding Can you confirm that the change works okay on macOS, Linux and *BSD respectively when finding an existing config file in the old style and when creating a new config file?

@nilsding
Copy link
Contributor

@nilsding nilsding commented Jun 6, 2018

@Et0h the change works fine on FreeBSD; it uses ~/.syncplay if it exists and $XDG_CONFIG_HOME/syncplay.ini otherwise.

@albertosottile
Copy link
Member

@albertosottile albertosottile commented Jun 6, 2018

@Et0h I can confirm that everything works as designed on macOS. It uses ~/.syncplay if available or creates and uses ~/.config/syncplay.ini if there is no previous configuration file in the home folder. AFAIK, $XDG_CONFIG_HOME is never set on macOS but, if the user sets it manually (not a so trivial task in this OS), then Syncplay behaves accordingly.

@Et0h Et0h merged commit 9349b88 into Syncplay:master Jun 9, 2018
2 checks passed
albertosottile added a commit to albertosottile/syncplay that referenced this issue Jun 9, 2018
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.

None yet

4 participants