config: Support config in $XDG_CONFIG_HOME #334
Conversation
There was a problem hiding this comment.
Looks good to me, but in the README.md file you should also add:
powerline-shell --generate-config > $XDG_CONFIG_HOME/powerline-shell/config.json
as an alternative way to generate the initial configuration file. There should also be a warning that if $XDG_CONFIG_HOME is not set, one should use the default:
powerline-shell --generate-config > ~/.config/powerline-shell/config.json
In fact, since many systems don't set that variable at all, the default should be added to the code that gets the value. I made a comment in the code.
Wouldn't it also be better to remove from the README.md the original path for the configuration file? That would make sure that the new one becomes the norm. I'd preserve the code in powerline_shell/__init__.py to find the configuration in older locations, of course.
What do you think?
powerline_shell/__init__.py
Outdated
| @@ -134,6 +134,7 @@ def find_config(): | |||
| for location in [ | |||
| "powerline-shell.json", | |||
| "~/.powerline-shell.json", | |||
| os.path.join(os.environ.get("XDG_CONFIG_HOME", ""), "powerline-shell", "config.json"), | |||
There was a problem hiding this comment.
Perhaps it would be better to set a default:
os.path.join(os.environ.get("XDG_CONFIG_HOME", "~/.config"), "powerline-shell", "config.json"),
since many systems don't set the XDG_CONFIG_HOME environment variable.
| @@ -151,7 +151,8 @@ alias precmd 'set prompt="`powerline-shell --shell tcsh $?`"' | |||
| ### Config File | |||
|
|
|||
| Powerline-shell is customizable through the use of a config file. This file is | |||
| expected to be located at `~/.powerline-shell.json`. You can generate the | |||
| expected to be located at `~/.powerline-shell.json` or | |||
| `$XDG_CONFIG_HOME/powerline-shell/config.json`. You can generate the | |||
There was a problem hiding this comment.
I'd say that it would be better to delete the ~/.powerline-shell.json reference in order to promote the new location and escape from "the tyranny of the default".
|
I went ahead and changed the default in As for the documentation, I'm going to defer to @b-ryan... |
|
Sorry for the long delay on this. I thought about it fairly frequently and kept having mixed feelings. At this point I am onboard & will include this in the next release. I will update the docs to encourage just the new config path. |
Resolves #328.