I don't see a big problem accepting this. The change is really minor, and makes sense.
Dmitry.
Hey all,
I know this is a little late in the process, but it's something I've noticed while prepping some content around 7.1.
This RFC: https://wiki.php.net/rfc/too_few_args
Passed, and has been implemented, but I feel that throwing an \Error
exception is a mistake. I think we should another more concrete exception class for this error:
\TooFewArgumentsError extends \Error
A use case where this may trivially occur is where you are using argument unpacking and the unpacked array is too small. Writing this, just looks bad:
try {
foo(… $args);
} catch (\Error $e) { }
compared:
try {
foo(… $args);
} catch (\TooFewArgumentsError $e) { }
Thoughts? Dmitry?
Given the tiny change this is, and that is backwards compatible with the original RFC, I would like to add this to 7.1 for beta3.
I think I can make this change myself.
- Davey
Morning all,
Good idea, +1 for just doing it ...
Cheers
Joe
I don't see a big problem accepting this. The change is really minor, and
makes sense.Dmitry.
Hey all,
I know this is a little late in the process, but it's something I've
noticed while prepping some content around 7.1.This RFC: https://wiki.php.net/rfc/too_few_args
Passed, and has been implemented, but I feel that throwing an
\Error
exception is a mistake. I think we should another more concrete exception
class for this error:
\TooFewArgumentsError extends \Error
A use case where this may trivially occur is where you are using argument
unpacking and the unpacked array is too small. Writing this, just looks bad:try {
foo(… $args);
} catch (\Error $e) { }compared:
try {
foo(… $args);
} catch (\TooFewArgumentsError $e) { }Thoughts? Dmitry?
Given the tiny change this is, and that is backwards compatible with the
original RFC, I would like to add this to 7.1 for beta3.I think I can make this change myself.
- Davey
Dmitry,
I've tried to implement this myself, and it seems to work fine, but I end
up with some weird (unrelated?) test failures after:
https://github.com/php/php-src/compare/master...dshafik:too-few-args-exception
Example test failure:
========DIFF========
001+ Warning: file_get_contents(http://localhost:8964): failed to open
stream: Connection refused in /Users/dshafik/src/
php.net/php-src/tests/basic/bug67198.php on line 29
002+ bool(false)
001- string(4) "PASS"
002- string(4) "PASS"
003+
004+ Warning: file_get_contents(http://localhost:8964): failed to open
stream: Connection refused in /Users/dshafik/src/
php.net/php-src/tests/basic/bug67198.php on line 30
005+ bool(false)
========DONE========
FAIL php://input is empty when enable_post_data_reading=Off
[tests/basic/bug67198.phpt]
and:
========DIFF========
001+ Warning: Declaration of melt\core\DateTime::createFromFormat($format,
$time, ?melt\core\DateTimeZone $timezone = NULL) should be compatible with
DateTime::createFromFormat($format, $time, $object = NULL) in
/Users/dshafik/src/php.net/php-src/ext/date/tests/bug55407.php on line 7
========DONE========
List of failed tests:
=====================================================================
FAILED TEST SUMMARY
Test posix_getgrgid()
function : basic functionality
[ext/posix/tests/posix_getgrgid_basic.phpt]
Test chdir()
function : basic functionality
[ext/standard/tests/dir/chdir_basic-win32-mb.phpt]
Test chdir()
function : usage variations - relative paths
[ext/standard/tests/dir/chdir_variation2-win32-mb.phpt]
Test getcwd()
function : basic functionality
[ext/standard/tests/dir/getcwd_basic-win32-mb.phpt]
Test readdir()
function : usage variations - sub-directories
[ext/standard/tests/dir/readdir_variation3-win32-mb.phpt]
Test readdir()
function : usage variations - different file names
[ext/standard/tests/dir/readdir_variation4-win32-mb.phpt]
Test readdir()
function : usage variations - operate on previously opened
directory [ext/standard/tests/dir/readdir_variation6-win32-mb.phpt]
Test rewinddir()
function : basic functionality
[ext/standard/tests/dir/rewinddir_basic-win32-mb.phpt]
Test scandir()
function : basic functionality
[ext/standard/tests/dir/scandir_basic-win32-mb.phpt]
Test scandir()
function : usage variations - different sorting constants
[ext/standard/tests/dir/scandir_variation10-win32-mb.phpt]
Test scandir()
function : usage variations - different relative paths
[ext/standard/tests/dir/scandir_variation4-win32-mb.phpt]
Test scandir()
function : usage variations - different file names
[ext/standard/tests/dir/scandir_variation8-win32-mb.phpt]
Test scandir()
function : usage variations - different ints as
$sorting_order arg [ext/standard/tests/dir/scandir_variation9-win32-mb.phpt]
file_get_contents()
test using offset parameter out of range
[ext/standard/tests/file/file_get_contents_error001.phpt]
Test lstat()
and stat()
functions: usage variations - file opened using w
and r mode [ext/standard/tests/file/lstat_stat_variation13.phpt]
Check cli_process_title support on Unix
[sapi/cli/tests/cli_process_title_unix.phpt]
I can't see how my change has affected this at all; but maybe I missed
something?
Could you review and see if you spot anything?
Thanks,
Morning all,
Good idea, +1 for just doing it ...
Cheers
JoeI don't see a big problem accepting this. The change is really minor, and
makes sense.Dmitry.
Hey all,
I know this is a little late in the process, but it's something I've
noticed while prepping some content around 7.1.This RFC: https://wiki.php.net/rfc/too_few_args
Passed, and has been implemented, but I feel that throwing an
\Error
exception is a mistake. I think we should another more concrete exception
class for this error:
\TooFewArgumentsError extends \Error
A use case where this may trivially occur is where you are using argument
unpacking and the unpacked array is too small. Writing this, just looks bad:try {
foo(… $args);
} catch (\Error $e) { }compared:
try {
foo(… $args);
} catch (\TooFewArgumentsError $e) { }Thoughts? Dmitry?
Given the tiny change this is, and that is backwards compatible with the
original RFC, I would like to add this to 7.1 for beta3.I think I can make this change myself.
- Davey