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

lib/settings: try to load from platform config_dir #85

Merged
merged 1 commit into from
Feb 26, 2022

Conversation

colemickens
Copy link
Contributor

Fixes #84.

  • try to load from the platform config_dir ({config_dir}/jj/config.toml)
  • fallback to ~/.jjconfig
  • error if both are present
  • Tests pass
  • Appropriate changes to README are included in PR (not applicable)

Here's manual testing showing it working from both locations and erroring when both are present:

cole@porty /tmp/jjalias jj="$HOME/code/jj/target/debug/jj"

cole@porty /tmp/jjrm -rf ~/.config/jj/config.toml .jj

cole@porty /tmp/jjjj init .
Initialized repo in "/tmp/jj/."

cole@porty /tmp/jjjj log
@ d45438bb3b08 683fa3619a43 cole.mickens@gmail.com 2022-02-25 15:06:52.181 -08:00
|
o 000000000000 000000000000  1970-01-01 00:00:00.000 +00:00


cole@porty /tmp/jjcp ~/.jjconfig ~/.config/jj/config.toml

cole@porty /tmp/jjjj log
Invalid config: Both `$HOME/.jjconfig` and `$XDG_CONFIG_HOME/jj/config.toml` were found, please remove one.

cole@porty /tmp/jjrm ~/.jjconfig
rm: remove write-protected regular file '/home/cole/.jjconfig'? y

cole@porty /tmp/jjrm -rf .jj

cole@porty /tmp/jjjj init .
Initialized repo in "/tmp/jj/."

cole@porty /tmp/jjjj log
@ a42f9e6cc55c c87bace4620f cole.mickens@gmail.com 2022-02-25 15:07:29.101 -08:00
|
o 000000000000 000000000000  1970-01-01 00:00:00.000 +00:00

@martinvonz
Copy link
Member

Thanks for both the PR and the review.

Do you think we should even drop support for ~/.jjconfig? Seems like there's no reason to keep support for that except for existing users. We could print a message telling users to move that file to the new location.

@martinvonz martinvonz merged commit fbe8eb4 into jj-vcs:main Feb 26, 2022
@colemickens
Copy link
Contributor Author

I didn't want to be too presumptive, if it were me I'd have just removed it entirely, and/or made it an early error if the old location exists.

@martinvonz
Copy link
Member

We're pre-1.0 so I think it's fine to make breaking changes. Making it an early error sounds good to me.

With recent format changes in the .jj/ directory, I've written code to automatically upgrade the repo, but in this case I think it's better to error out so the user doesn't miss the fact that the config location moved.

martinvonz added a commit that referenced this pull request Feb 27, 2022
Since #85, we load the user's config from a path under
`dirs::config_dir()`. It's probably not obvious to all users where to
put the file, so let's describe that. (I didn't know where to put the
file on my Mac until I looked at the function's documentation.)
martinvonz added a commit that referenced this pull request Feb 27, 2022
Since #85, we load the user's config from a path under
`dirs::config_dir()`. It's probably not obvious to all users where to
put the file, so let's describe that. (I didn't know where to put the
file on my Mac until I looked at the function's documentation.)
@colemickens colemickens deleted the config_dir branch February 22, 2024 04:52
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.

Use XDG_CONFIG_HOME for config instead of ~/.jjconfig
3 participants