-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Use a common function for "cannot be empty" ValueError #15489
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
Use a common function for "cannot be empty" ValueError #15489
Conversation
|
4f11fa2
to
314410f
Compare
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.
LGTM from mbstring. Thanks.
@@ -14,4 +14,4 @@ try { | |||
} | |||
?> | |||
--EXPECT-- | |||
DOMDocument::loadHTML(): Argument #1 ($source) must not be empty | |||
DOMDocument::loadHTML(): Argument #1 ($source) cannot be empty |
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.
Agreed with Tim
89e35ad
to
c47d283
Compare
It not being empty is already checked by PHP_GETTEXT_DOMAIN_LENGTH_CHECK()
c47d283
to
34b57c9
Compare
Zend/zend_API.c
Outdated
@@ -439,6 +439,11 @@ ZEND_API ZEND_COLD void zend_argument_value_error(uint32_t arg_num, const char * | |||
} | |||
/* }}} */ | |||
|
|||
ZEND_API ZEND_COLD void zend_argument_cannot_be_empty_error(uint32_t arg_num) |
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 function name now no longer matches the message.
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.
Updated in-place in the last commit, which changes the wording from "cannot" to "must not"
34b57c9
to
c823ee4
Compare
Commits should be reviewed individually.
Some extensions were not using the standard wording, so changed it by using the new helper.
There are also some drive-by refactoring that I noticed while changing the code to use the helper.