Skip to content

Switch the format of argument error messages #5211

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

Conversation

kocsismate
Copy link
Member

It turned out that the ...() expects argument #1 ($arg) to be format is not that universal as Argument #1 ($arg) passed to ...(), that's why we should make the switch again.

@kocsismate kocsismate changed the title Use "Argument #1 ($arg) passed to ...()" format for argument error messages Switch the format of argument error messages Feb 26, 2020
@kocsismate kocsismate force-pushed the consistent-error-messages2-fix branch 2 times, most recently from 9631c82 to ae8f531 Compare February 26, 2020 08:46
@nikic
Copy link
Member

nikic commented Feb 26, 2020

As mentioned in chat before, if instead of

Argument #1 ($options) passed to finfo_open() must be of type int, string given

the message is

finfo_open(): Argument #1 ($options) must be of type int, string given

then we will use the same error pattern as php_error_docref(), which is probably our single largest class of error messages (some of which we may convert to exceptions, but not all, or even most).

@kocsismate
Copy link
Member Author

@nikic uh, I completely forgot this (I was in a hurry yesterday)...

I like this pattern much more, because the "argument, function" order is more difficult to understand. I still have my regex so the conversion won't be a problem. :)

@kocsismate kocsismate force-pushed the consistent-error-messages2-fix branch 4 times, most recently from 3890e9d to 563d4a5 Compare February 26, 2020 11:12
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

There is a spurious ext/standard/tests/file/windows_links/bug78862.target file committed.

@kocsismate
Copy link
Member Author

Ehh, I removed it several times ^^ Let's try again one more time.

@kocsismate kocsismate force-pushed the consistent-error-messages2-fix branch from 563d4a5 to 5bdd847 Compare February 26, 2020 13:45
@php-pulls php-pulls closed this in 960318e Feb 26, 2020
@kocsismate kocsismate deleted the consistent-error-messages2-fix branch February 26, 2020 14:03
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