-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
Fix setters to return correct boolean value #727
Conversation
I noticed we have some other cases here just returning true as well, would it be correct to update all of them to return false instead?
auto parsed_type = ada::scheme::get_scheme_type(input);
bool is_input_special = (parsed_type != ada::scheme::NOT_SPECIAL);
/**
* In the common case, we will immediately recognize a special scheme (e.g.,
*http, https), in which case, we can go really fast.
**/
if (is_input_special) { // fast path!!!
if (has_state_override) {
// If url's scheme is not a special scheme and buffer is a special scheme,
// then return.
if (is_special() != is_input_special) {
return true;
}
// If url includes credentials or has a non-null port, and buffer is
// "file", then return.
if ((has_credentials() || port.has_value()) &&
parsed_type == ada::scheme::type::FILE) {
return true;
}
// If url's scheme is "file" and its host is an empty host, then return.
// An empty host is the empty string.
if (type == ada::scheme::type::FILE && host.has_value() &&
host.value().empty()) {
return true;
}
}
type = parsed_type; |
@CarlosEduR Important work!!! Yes, I think that in the code sample you have found, it should return false in all the error cases. |
…ters-silently-ignored
@CarlosEduR is this ready to review? i see it as draft? |
@anonrig now it is... |
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.
Looks like it improves ada.
@CarlosEduR tests seem to be failing with the most recent changes |
Which tests? |
I am merging. |
Fix: #719