-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
parsing of PC_DISABLE_TUI should be consistent with -t flag #75
Comments
one use case is to address a comment here also, if ''export PC_DISABLE_TUI=''${PC_DISABLE_TUI-${builtins.toJSON (!config.tui)}}'' which would allow user to override the default value by setting |
@F1bonacc1 what do you think about the above? |
Hey @adrian-gierakowski, I always considered it standard practice to use any value in an env variable acting as a boolean. I would prefer not to break the existing behavior and remain backward compatible.
This way you will be able to reset it with something like |
Could you please elaborate, I am not sure what you mean. I believe that if an env var is provided as and alternative to a cli flag, then following the principle of least surprise, one should be able to use the same values with it as with the flag to achieve the same result. Your proposed solution is fine for me, but personally I’d change it to support the same values as the flag, if not now then at least in the next major release. |
Fixed in v1.5.0. |
Defect
Currently all of the following will result in TUI being disabled:
So the only way override
PC_DISABLE_TUI
once it's set is by un-setting the env var, or by adding -t flag.By comparison, when using t flag following result in tui being disabled:
while following results in it being enabled:
anything else (like t=whatever) is an error
Version of
process-compose
:OS environment:
nixos
Steps or code to reproduce the issue:
runt
Expected result:
I would expect the parsing of
PC_DISABLE_TUI
to be consistent with parsing o-t
Btw. I think it's a bit confusing that
PC_DISABLE_TUI
is reverse of-t
. I would deprecatePC_DISABLE_TUI
and addPC_TUI
instead.Actual result:
setting
PC_DISABLE_TUI
to any value is interpreted astrue
The text was updated successfully, but these errors were encountered: