Casting TableCell value to string. #21430

Closed
wants to merge 5 commits into
from

Projects

None yet

6 participants

@jaydiablo
Contributor
Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #21429
License MIT
Doc PR

PHP throws a catchable fatal error when the value from this method is
used in strstr in the Table class. This fixes the error by casting to a string before returning the value.

@jaydiablo jaydiablo Casting TableCell value to string.
PHP throws a catchable fatal error when the value from this method is
used in strstr in the Table class.
dddd749
@jaydiablo jaydiablo long array syntax
6c0764a
@xabbuh
Member
xabbuh commented Jan 27, 2017

Taking it strictly the current behaviour is not wing as the TableCell constructor documents that it expects a string. In the other hand does this change not do any harm. So +0 from me on the proposed change.

@chalasr
Contributor
chalasr commented Jan 27, 2017

Looks like a feature to me, it is the expected behavior on 2.7.

jaydiablo added some commits Jan 27, 2017
@jaydiablo jaydiablo Moving cast to constructor, updating docblock
f5187c5
@jaydiablo jaydiablo Switching docblock back to just "string"
Mis-read the comment by @stof in #21429
c7a902a
@nicolas-grekas
Member

👍 on 2.7, minor tweak to me

@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Jan 29, 2017
@fabpot
Member
fabpot commented Jan 30, 2017 edited

I would only cast for ints then.

@xabbuh
Member
xabbuh commented Feb 3, 2017

@jaydiablo Can you make the change @fabpot asked for? :)

@jaydiablo
Contributor

Sure, what about other numeric values that aren't ints? Like a float.

Should we wrap the cast in something like:

if (is_numeric($value) && !is_string($value)) {

?

@fabpot
Member
fabpot commented Feb 3, 2017

My point was that I don't want to have a cast to a string for objects for instance.

@jaydiablo jaydiablo Only casting to string when value is numeric and not already a string
bcdf9fc
@fabpot
Member
fabpot commented Feb 3, 2017

Thank you @jaydiablo.

@fabpot fabpot added a commit that referenced this pull request Feb 3, 2017
@fabpot fabpot bug #21430 Casting TableCell value to string. (jaydiablo)
This PR was squashed before being merged into the 2.7 branch (closes #21430).

Discussion
----------

Casting TableCell value to string.

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21429
| License       | MIT
| Doc PR        |

PHP throws a catchable fatal error when the value from this method is
used in strstr in the Table class. This fixes the error by casting to a string before returning the value.

Commits
-------

1e5707f Casting TableCell value to string.
f0d13f4
@fabpot fabpot closed this Feb 3, 2017
@jaydiablo jaydiablo deleted the jaydiablo:console-tablecell-numeric-fix branch Feb 4, 2017
This was referenced Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment