Boolean setting support #1879
Boolean setting support #1879
Conversation
|
bosh
force-pushed
the
boolean_setting_support
branch
from
3289fa1 to
46d794f
Compare
bosh
force-pushed
the
boolean_setting_support
branch
from
46d794f to
52bb434
Compare
|
This seems like a nice improvement. The only reason I know of why booleans weren't previously supported is that they are literally indifferent from 0 and 1, so there's nothing a boolean could do that couldn't be done with an int. But also, no reason not to have booleans. I'm a little concerned that the other values will create some "magic" behavior. I realize that this is part of MPF canon, allowing "yes" and "t" to stand for true, et cetera. Something about it just rubs me wrong, but I'm not strongly opinionated enough to push for removing it. |
|
Yeah I agree that the extra "true" parsing values are awkward. It's through this that I became aware of my expectations of YAML parsing rules are from Ruby in particular, which is subtly different than Python's standard. I do think it's weird that True vs true is okay in many places, but config fields that expect template_booleans specifically require True (true has an explicit check and raise in placeholder_manager.py#_eval_name (line 756). I still haven't gotten around to testing the boolean setting by hand to make sure the service menu etc work fine with it ... maybe this weekend. |
bosh
force-pushed
the
boolean_setting_support
branch
from
52bb434 to
f8ec439
Compare
|
bosh
force-pushed
the
boolean_setting_support
branch
from
f8ec439 to
c3662bd
Compare
bosh
force-pushed
the
boolean_setting_support
branch
from
c3662bd to
9d1fd2c
Compare
bosh
force-pushed
the
boolean_setting_support
branch
from
9d1fd2c to
18c53e7
Compare
|
bosh
force-pushed
the
boolean_setting_support
branch
from
18c53e7 to
f1bb07c
Compare
|
values that cast to lowercase strings matching true/t/yes/enable/on or the number 1 will all be true, all other values will be treated as false
bosh
force-pushed
the
boolean_setting_support
branch
from
f1bb07c to
bb1a340
Compare
|
2 participants
Add support to for boolean settings. There's no real reason why you couldnt use a more complex type for a boolean setting, and you would be more prepared if you have to add a third (or more) option in the future -- HOWEVER! I think bool settings are still useful because you should be able to embed them in other templates and logic checks more simply than the other settings types.
Tests include a backfill for float support, and both the cases of true and false defaults (just in case the yaml-to-python conversion could have an issue with e.g. any non-empty-string converting as truthy.
I still need to test the boolean setting in the settings management interface, and also embedded in other templates, but I'm posting now because it's close enough to be worth comment already.