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

Make windows config location more sensible #1857

Merged
merged 3 commits into from
Dec 27, 2018

Conversation

zacps
Copy link
Contributor

@zacps zacps commented Dec 2, 2018

Contrary to my initial idea this does not remove platform specific code, mostly because after dealing with fallbacks and supporting the old windows location it's not worth it.

This changes the configuration file location from (in fallback order):

  • %HOME%\alacritty.yml
  • %USERPROFILE%\alacritty.yml
  • GetUserProfileDirectory()\alacritty.yml

To:

  • %APPDATA%\Roaming\alacritty\alacritty.yml

If a configuration file is found in the old location a warning is emitted pointing to the new location.

Fixes #1660

Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, I've got just some minor comments for the changes made here.

It looks like this PR is only using dirs for Windows at the moment, but xdg is still used for macOS. I'm not familiar with the exact functionality of dirs, but would it be possible to make use of it on Linux too, so we can reduce the number of dependencies?

CHANGELOG.md Outdated Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
@zacps
Copy link
Contributor Author

zacps commented Dec 2, 2018

Using dirs instead of xdg is possible, but ensuring they have identical behaviour is difficult.

They follow the same usual case if XDG_CONFIG_HOME is set. However, the way it's written now it also searches XDG_CONFIG_DIRS which dirs has no equivalent for.

I think it's probably not worth the effort.

@chrisduerr chrisduerr force-pushed the windows-config-location branch 4 times, most recently from b5eb7e8 to ccfe863 Compare December 25, 2018 20:12
The change log entry has been reworded and some minor code refactoring
has been done.
The current behavior incorrectly created the `alacritty/alacritty.yml`
directory, instead of making `alacritty.yml` a file. This has been fixed
by creating all directories for the parent of the target file, instead
of the file itself.
Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now. Thanks!

@chrisduerr chrisduerr merged commit 6d7647c into alacritty:master Dec 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants