luci-base: add uci.get_bool to allow cleanup of app code #7612
Conversation
Any number of apps read boolean values from configuration files, then use
various inconsistent means for checking truth values. The get_bool function
allows app authors to fetch the value without regard for how it is represented
in the config file.
For example, this
let enabled = uci.get('system', 'ntp', 'enable_server');
if (enabled == '1') ...
could become the more natural
let enabled = uci.get_bool('system', 'ntp', 'enable_server');
if (enabled) ...
Signed-off-by: Eric Fahlgren <ericfahlgren@gmail.com>
| get_bool(conf, type, opt) { | ||
| let value = this.get(conf, type, opt); | ||
| if (typeof(value) == 'string') | ||
| return ['1', 'on', 'true', 'yes'].includes(value.toLowerCase()); |
There was a problem hiding this comment.
enabled is a parameter name. I've not seen any instances that use = "enabled", and I don't think its use should be encouraged.
There was a problem hiding this comment.
it may be some feature = 'enabled'. I agree that this should be avoided, just want to have it consistent with UCI. Still not a problem, we may merge the PR.
| let value = this.get(conf, type, opt); | ||
| if (typeof(value) == 'string') | ||
| return ['1', 'on', 'true', 'yes'].includes(value.toLowerCase()); | ||
| return false; |
There was a problem hiding this comment.
a Number 1 will be converted to false
There was a problem hiding this comment.
But get guarantees null, string or stuff we don't care about, so should never see an int.
|
There is Additionally it also would be nice to check the existing JS code to use the function. Interesting in how many places it would be useful. Just a bikeshedding though. From a performance standpoint:
Maybe the regexp will be faster, it may use KMP search, and maybe even easier to understand. The JavaScript itself has Boolan(). |
Yeah, I completely ignored any performance considerations. I figure this is always running on a "big" client with CPU and RAM to spare, so my only real thought was to keep the JS footprint small so it takes up less device space... |
Thanks for pointing that out @stokito! Would be ideal if the js function was consistent with the shell function in accepting |
Any number of apps read boolean values from configuration files, then use various inconsistent means for checking truth values. The get_bool function allows app authors to fetch the value without regard for how it is represented in the config file.
For example, this
could become the more natural