Skip to content

Add ValueError #4682

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

Closed
wants to merge 2 commits into from
Closed

Add ValueError #4682

wants to merge 2 commits into from

Conversation

salathe
Copy link
Contributor

@salathe salathe commented Sep 4, 2019

ValueError is intended to be thrown when a function or method receives an argument that has the right type (incorrect type should throw a TypeError) but an inappropriate value.

ValueError is intended to be thrown when a function or method receives
an argument that has the right type (incorrect type should throw a
TypeError) but an inappropriate value.
Raise a ValueError instead of a plain Error when calling imagecreate()
or imagecreatetruecolor() with too big or small values for the width or
height arguments.
@salathe
Copy link
Contributor Author

salathe commented Sep 4, 2019

If approved, this might be useful for many of the Errors from the big list of E_WARNING to Error PRs that have been merged lately, which really should be more specific than Error.

I added an example from #4648 for imagecreate() and imagecreatetruecolor().

@salathe salathe requested a review from nikic September 4, 2019 21:09
@Girgias
Copy link
Member

Girgias commented Sep 4, 2019

I'm all for it, should we also define some "standard" messages? e.g. for empty arguments or negative values?

@php-pulls
Copy link

Comment on behalf of salathe at php.net:

Merged as bc61997 (add ValueError) and a436796 (raise ValueError from imagecreate/imagecreatetruecolor).

@php-pulls php-pulls closed this Sep 9, 2019
@salathe salathe deleted the valueerror branch September 9, 2019 20:14
php-pulls pushed a commit that referenced this pull request Sep 10, 2019
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.

6 participants