-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Switch to using a function to parse booleans #2340
Conversation
include/util.h
Outdated
* toggling is not desired, pass in true for current so that toggling values | ||
* get parsed as not true. | ||
*/ | ||
bool parse_boolean(const char *boolean, const bool current); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bool argument doesn't need to be const here
@@ -123,6 +123,21 @@ uint32_t parse_color(const char *color) { | |||
return res; | |||
} | |||
|
|||
bool parse_boolean(const char *boolean, bool current) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to handle errors? (ie. bool parse_boolean(const char *str, bool current, bool *out)
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this too, but it looks like the function returns false for invalid values and if that matches i3's behavior we should keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make sense to add a comment stating that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about case sensitivity? Shouldn't these use strcasecmp
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make sense to add a comment stating that.
Added
What about case sensitivity? Shouldn't these use
strcasecmp
?
Fixed
Thanks! |
Related to #2336.
This PR adds a function (
parse_boolean
) similar to i3'seval_boolstr
. The onlytrue
value that I added wasenabled
since that was in use for various commands. The rest are identical to i3's.I also added support for toggling values. Currently, toggling is not supported for
input
oroutput
commands (since it would require a change to the way their configs work, which is out of scope for this PR), but should work for anything else that takes a boolean value. Forinput
andoutput
commands,toggle
is interpreted as a non-true value.