jj icon indicating copy to clipboard operation
jj copied to clipboard

FR: look in $XDG_CONFIG_HOME for jj config on OSX

Open ckoehler opened this issue 1 year ago • 15 comments

Is your feature request related to a problem? Please describe.

OSX CLI users often have config files in $XDG_CONFIG_HOME, like Linux. jj uses the dirs crate which is very opinionated about not adding this (see https://github.com/dirs-dev/dirs-rs/pull/40)

Describe the solution you'd like In addition to the config location in Application Support, I'd like jj to check $XDG_CONFIG_HOME/jj, with priority given to the one in Application Support. There's already some code to handle this under different circumstances, OSX just never gets the option to use another location.

We could use another crate than dirs to enable this, but I am not sure what's out there. Would love to hear people's thoughts.

Describe alternatives you've considered Setting JJ_CONFIG works, but OSX CLI users may be surprised by it (I was).

Additional context

ckoehler avatar Apr 03 '24 13:04 ckoehler

Could use https://github.com/lunacookies/etcetera, or with a slightly different scope, home, which is cargo maintained.

ckoehler avatar Apr 03 '24 13:04 ckoehler

Starship looks at the home dir, unless an env var is set. It uses dirs to find the home dir.

Wezterm also defaults to a file in the $HOME dir on all platforms (-ish, see docs). Uses a dirs fork (that looks identical to dirs) to find $HOME.

ckoehler avatar Apr 03 '24 13:04 ckoehler

If setting JJ_CONFIG isn't an option, another workaround is symlinking to your preferred location from Application Support:

$ ln -s ../../.config/jj ~/Library/Application\ Support/jj

jennings avatar Apr 03 '24 15:04 jennings

If setting JJ_CONFIG isn't an option, another workaround is symlinking to your preferred location from Application Support:

$ ln -s ../../.config/jj ~/Library/Application\ Support/jj

Yep, also a way!

This FR is more to align it with many (?) OSX CLI users' expectations that it should work. I am setting JJ_CONFIG currently and it works.

ckoehler avatar Apr 03 '24 16:04 ckoehler

Is there any desire to change this?

We can:

  1. Not change it
  2. Check if config_dir == home_dir + "Library/Application Support" (which is how dirs builds the config_dir), and if so, check first for home_dir + .config/jj, and if empty, fall back to the old one. Not super clean, but also not terrible.
  3. follow some other apps and just look for config in home_dir + .config/jj. This would be a breaking change, but would allow us to drop dirs and maybe use the home crate, which has no deps on other crates. May speed up build times.

ckoehler avatar Apr 04 '24 02:04 ckoehler

If we change things, we should be careful to follow the XDG specification: check $XDG_CONFIG_HOME/jj, and only use ~/.config/jj if that variable doesn't exist.

Another option that just came to mind would be to have a command that automatically symlinks the application support dir to the XDG dir. This is what I do. As long as jj follows symlinks, I don't care much which we do, though people who understand Mac philosophy better might know reasons for or against.

I wish there was a crate that did this for us. We can always make our own wrapper to dirs, though.

ilyagr avatar Apr 04 '24 02:04 ilyagr

@ilyagr as far as I can tell with the etcetera crate we could attempt to use the Xdg strategy on both MacOS and Linux, if there is no config in that location on MacOS we could check with the Apple strategy for backwards compatibility but only fall back to it if there is actually a config in that location (that way for new users, the config would automatically be created in $XDG_CONFIG_HOME/jj/config.toml or $HOME/.config/jj/config.toml). For Windows we'd always use the windows one.

noahmayr avatar Apr 04 '24 11:04 noahmayr

I wish there was a crate that did this for us. We can always make our own wrapper to dirs, though.

We could use etcetera, tho it's a bit old (still Rust 2018 edition), but I think just making two functions conditional on #[cfg(target_os = "macos")] and #[cfg(not(target_os = "macos"))] that return the config dir would do the trick. Not-MacOS would just passthru dirs, MacOS would check XDG env var, then $HOME/.config, then passthru to dirs.

I can probably make a PR pretty quickly.

@noahmayr etcetera looks fine, too, but for how little we actually need, it may be okay to stick with dirs as the more widely used one, and just override what we need.

ckoehler avatar Apr 04 '24 13:04 ckoehler

t may be okay to stick with dirs as the more widely used one, and just override what we need.

Wouldn't everyone need that? Submit the change upstream?

joyously avatar Apr 04 '24 13:04 joyously

No, see the description, the author of dirs very much doesn't want this. :(

ckoehler avatar Apr 04 '24 13:04 ckoehler

dirs will not accept that change, the maintainers' opinion is that all software on MacOS should follow Apple's standards (even if they are mainly meant for GUI applications)

noahmayr avatar Apr 04 '24 13:04 noahmayr

We could use etcetera, tho it's a bit old (still Rust 2018 edition) I mean the last dirs update is like 5 days newer than the last etcetera update, dirs also is still on 2015 edition (default if nothing is specified) and editions don't matter for interop between crates afaik

I'm not saying we need to use it, however I don't think that this is a good argument. Also why use dirs if they don't want to support our usecase?

noahmayr avatar Apr 04 '24 13:04 noahmayr

That's fair, didn't check that. We can use etcetera and https://doc.rust-lang.org/std/env/consts/constant.OS.html or conditional compilation to check for OS.

ckoehler avatar Apr 04 '24 14:04 ckoehler

I can probably make a PR pretty quickly.

I think you can go ahead with that, we should just make sure to review and test it well on all platforms (with and without XDG env vars) to make sure nobody's current setup breaks

noahmayr avatar Apr 04 '24 17:04 noahmayr

I can probably make a PR pretty quickly.

I think you can go ahead with that, we should just make sure to review and test it well on all platforms (with and without XDG env vars) to make sure nobody's current setup breaks

That was in reference to just wrapping dirs::config_dir() for OSX; is that what you'd recommend? I am kinda liking the etcetera swap, tho the changes will be a bit more substantial.

ckoehler avatar Apr 04 '24 17:04 ckoehler