Skip to content
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

Fail when invalid values are provided to rule arguments. #352

Closed

Conversation

askreet
Copy link

@askreet askreet commented Dec 23, 2019

Inspired by #350.

@askreet
Copy link
Author

askreet commented Dec 23, 2019

Now properly tells me I am dumb when using arguments incorrectly:

526> yabai -m rule --add title="^Opening" manage=false label=rule1
unknown value 'false' for argument 'manage'

@@ -1764,30 +1769,45 @@ static void handle_domain_rule(FILE *rsp, struct token domain, char *message)
rule->manage = RULE_PROP_ON;
} else if (string_equals(value, ARGUMENT_COMMON_VAL_OFF)) {
rule->manage = RULE_PROP_OFF;
} else {
bad_rule_argument_value(rsp, key, value);
return;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed there is a mix of having an internal value in the object saying it's OK (for regex in many cases) and early return on failure. Not sure which pattern is preferred, can update this to match other pattern, just let me know.

@koekeishiya
Copy link
Owner

I appreciate the effort, but as you mentioned there is actually not a clear way to properly deal with errors regarding invalid arguments as of right now.

Usually I'd agree with something similar to what you did in this PR, but I think I want to rework how rules (and signals) are parsed into something that can validate the value given to key-value pairs, and also whether the key-value pair is required or optional.

@koekeishiya
Copy link
Owner

Didn't bother too much with what I wrote in my previous comment, but solved it in a way I'm fairly ok with for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants