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

Config Loading #391

Merged
merged 2 commits into from
Apr 27, 2021
Merged

Config Loading #391

merged 2 commits into from
Apr 27, 2021

Conversation

a-kenji
Copy link
Contributor

@a-kenji a-kenji commented Apr 27, 2021

  • Configuration is now handled in this order:
  • --config flag
  • ZELLIJ_CONFIG_FILE env
  • config_dir flag
  • ZELLIJ_CONFIG_DIR env
  • get_default_config_dir() (directories-next)
  • $HOME/.config/zellij (for mac convenience)
  • SYSTEM_DEFAULT_CONFIG_DIR (for distributions to specify sensible defaults ontop of zellij)
  • from assets
  • Fix default.yaml file

  • Move default.yaml file to assets directory

@qballer Could you check, if the config is loaded from .config/zellij/config.yaml now please? In order for that to happen the original config directory on mac shouldn't exist.

The default keys are now not anymore specified in the code, but taken from the default.yaml file.

I am happy about suggestions or any input on this pr!

@TheLostLambda
Copy link
Member

From a first glance, this is looking pretty good! Certainly quite a few different options now! One thing I think we should be sure to do is document this well. Your description in this PR is good, but should probably make it's way somewhere more permanent too 🙂

@a-kenji
Copy link
Contributor Author

a-kenji commented Apr 27, 2021

@TheLostLambda
Cheers! Would it be possible to get the documentation back in this repo
at some point and have the website render everything out?

@TheLostLambda
Copy link
Member

Cheers! Would it be possible to get the documentation back in this repo
at some point and have the website render everything out?

Yes, I think so, and that would be a much much better way to do things! I'm not sure when I'll have the time, but I'll bump that up my todo list. It's just some trickery with GitHub actions I think.

@a-kenji
Copy link
Contributor Author

a-kenji commented Apr 27, 2021

Yes, I think so, and that would be a much much better way to do things! I'm not sure when I'll have the time, but I'll bump that up my todo list. It's just some trickery with GitHub actions I think.

Awesome, no rush. Just good to know that its a possibility!

@TheLostLambda
And also, what would be a good way to set for SYSTEM_DEFAULT_CONFIG_DIR a prefix on compilation?
I just have seen conditional compile flags so far, not one where we can pass stuff in yet, for this here it actually shouldn't matter too much but if we want to do it in the future.

@TheLostLambda
Copy link
Member

I just have seen conditional compile flags so far, not one where we can pass stuff in yet, for this here it actually shouldn't matter too much but if we want to do it in the future.

I could be mistaken, but I think things like env!() will do that. All of the macros with the ! run at compile time, as far as I'm aware. There are probably even more macros for doing stuff at compile-time like that.

@a-kenji
Copy link
Contributor Author

a-kenji commented Apr 27, 2021

@TheLostLambda
Awesome, thanks! Yeah that is what I was looking for.

* Configuration is now handled in this order:
 --config flag
 ZELLIJ_CONFIG_FILE env
 get_default_config_dir() (directories-next)
 HOME/.config/zellij (for mac convenience)
 SYSTEM_DEFAULT_CONFIG_DIR (for distributions to specify sensible
 defaults ontop of zellij)
 from assets

* Fix default.yaml file

* Move default.yaml file to assets directory
@a-kenji a-kenji merged commit f172a01 into zellij-org:main Apr 27, 2021
@a-kenji a-kenji deleted the config-loading branch April 27, 2021 20:58
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.

2 participants