Skip to content

Conversation

IMSoP
Copy link
Contributor

@IMSoP IMSoP commented Aug 13, 2014

Error messages mentioning "long" and "double" are confusing to users, as these are not the names used for the types in the manual or userland functions; they should refer to "integer" and "float", respectively.

This PR updates all the appropriate messages, and all PHPT tests expecting them (the tests make up the bulk of the changes).

I would consider this a bug fix, although there may be tools out there relying on the current wording. In particular, I noticed that echoing a whole class using reflection used the "wrong" terms (see changes to ext/reflection/tests/bug29986.phpt).

I've never submitted a pull request before, and made this change a while ago and then forgot about it, so let me know if I've not followed the correct process.

IMSoP added 5 commits August 13, 2014 14:02
…ouble'->'float'

Fixes type names at Y in "X expects parameter A to be Y, but Z given" messages
to be the types that PHP programmers would expect, rather than C types.
Fixes the second half of invalid parameter messages ending "... expected,
but double given".
Doesn't affect userland gettype() function, as that is defined independently.
Also affects some Reflection output, e.g. summary of class constants.
Other uses are mostly related to typehints, so cannot fall into this case.
Includes necessary test changes to match.
@cjbj
Copy link
Contributor

cjbj commented Aug 13, 2014

There is ongoing discussion about underlying type changes in PHP. See the branch https://github.com/php/php-src/tree/str_size_and_int64
Perhaps you can combine efforts?

@hikari-no-yume
Copy link
Contributor

I did this and got it merged (see #955), so this should probably be closed.

@php-pulls
Copy link

Comment on behalf of ajf at php.net:

Closed as #955 was already merged and makes this redundant.

@php-pulls php-pulls closed this Dec 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants