-
Notifications
You must be signed in to change notification settings - Fork 8
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
[FEATURE] - Improve variables & profiles #121
Comments
Thanks for the nice write up, i appreciate it! I agree with you all points, except the last one. Default ValuesThe current way to handle this case would be to use something along the lines of this:
And i totally agree, that this is in no ways ergonomic. A better solution would be to allow "chained" boolean expressions (e.g. With this syntax the following could be written as:
Which i guess is a bit more ergonomic. Variable TypesI also agree with you on this point, i had the same issues in my setup. "Downsides"For boolean values, we would then have to find a new syntax to check if a variable is not defined.
Profile InheritanceIn my setup i use inheritance with 4-5 levels. It is useful, but maybe a bit over engineered. Personally i would like to keep this feature. What i could do, is to document the inheritance oder/priority in our wiki, so that it is maybe a little bit less confusing. |
With these points and the points from #122, i might look into doing |
Nah, that's what major version increments are for :) Firefox v109, compared to v1 contains a couple of rewrites too 😅
I don't see an issue with that, let's just do it like JS, where a undefined variable is the same as a bool that's set to false. |
Default Values
Right now, IF statements can be a bit tedious.
For once, I have to make sure that for every profile, every single variable is initialized somewhere. This gets a nightmare to manage when having multiple profiles with inheritance. Also, every variable is of the type String, which feels weird sometimes.
For example I have this in my .bashrc file:
Now I have to define the WSL variable in every base profile (this is how I name a profile that does not inherit anything). Otherwise, it will throw an error when deploying that file using a profile that does not define it.
This is why I think that variables should have default values. If I want to change the default value, I could still do it in the base profile, but I don't have to.
Variable Types
Also, it feels a bit weird to implement boolean values by comparing it with a string and could easily lead to errors. Did I use "yes"? Or "Yes"?? Or even "true"?
Most of the time, I actually use booleans and not strings, so maybe we don't need strings? On the other hand in future we might also want to have for loops for that we would need integer values.
YAML solves this quite nicely by interpreting
true
/false
as boolean types, number literals as numbers and text in quotes as strings.Having boolean values would also make the syntax much nicer, since we could just have:
Also, variable types would allow us to have default values like "false" for boolean types.
Profile Inheritance
Having the possibility to "extend" multiple profiles is nice, but is there actually a use case for it? This feels a bit overengineered and I am not even sure how it will handle certain cases, where the same variable is defined in multiple inherited base profiles. I think we could drop that feature, especially since we already can load multiple dotfiles folders like so:
dotfiles:
We also could think about introducing the concept of "components" which are a set of paths and variables (like profiles but cannot be deployed on their own). Also, they could accept parameters. At this point, we are arriving at a nice general-purpose file-based templating system that might be used for other use cases than dotfiles as well (like: https://github.com/michidk/multiform).
The text was updated successfully, but these errors were encountered: