Fixes for PHP types #5495
Fixes for PHP types #5495
Conversation
| @@ -972,7 +980,7 @@ public static function safeTemplate( | |||
| public static function short( | |||
| string $string = null, | |||
| int $length = 0, | |||
| string $appendix = '…' | |||
| string|false $appendix = '…' | |||
There was a problem hiding this comment.
Was false as the argument already supported or used before?
There was a problem hiding this comment.
Yes, not in tests but in Str::slug(): https://github.com/getkirby/kirby/blob/v4/develop/src/Toolkit/Str.php#L1114
There was a problem hiding this comment.
@distantnative Should we really officially support this though by adding the type hint for false? If false is passed to an argument that is hinted for string, the false value is converted to an empty string. This can be made explicit in the calling code (here: Str::slug()) by passing an empty string instead of false in the first place.
I feel like supporting false is unnecessary because it doesn't add any benefit over an empty string, but makes the method signature and code more complex.
There was a problem hiding this comment.
Idk, I'm indifferent, don't have any issue with supporting false either
Co-authored-by: Lukas Bestle <lukas@getkirby.com>
Co-authored-by: Lukas Bestle <lukas@getkirby.com>
|
@lukasbestle interestingly 581d10d seems to let the unit tests crash under PHP 8 |
|
@lukasbestle does anything speak against reverting to substring? |
|
I don't think reverting helps us here. We are scraping so close under the memory limit that basically any change will make us go over it. See our memory leak thread in Discord. |
|
Can we then just merge this PR as we need to fix the test/memory issue separately? |
Some type-related fixes that came up when I tried strict typing.