first usages of String component #4514
Conversation
why not add this to our own composer file? |
| @@ -132,7 +133,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int | |||
| $this->verified = in_array((int) $input->getOption('verified'), [0, 1, 2]) ? (int) $input->getOption('verified') : 1; | |||
| $regDate = $input->getOption('regdate') ?? $this->nowUTC; | |||
| $this->range = is_string($regDate) && '>' === $regDate[0]; | |||
| $this->regDate = $this->range ? mb_substr($regDate, 1) : $regDate; | |||
| $this->regDate = $this->range ? (string)s($regDate)->slice(1) : $regDate; | |||
There was a problem hiding this comment.
space after cast or no? here you do not have a space, later you do have a space.
There was a problem hiding this comment.
Spacing for casts were not consistent before already. Improved that in 7a1aa6a
| @@ -88,7 +89,7 @@ public function __construct(array $json = []) | |||
| $this->dependencies = $this->formatDependencies($json); | |||
| $this->shortName = $json['extra']['zikula']['short-name']; | |||
| $this->class = $json['extra']['zikula']['class']; | |||
| $this->namespace = mb_substr($this->class, 0, mb_strrpos($this->class, '\\') + 1); | |||
| $this->namespace = (string) s($this->class)->beforeLast('\\', true); | |||
There was a problem hiding this comment.
space here.
Also, it seems strange to me that a component called String requires to be cast as a string... shouldn't it output a string? I know that it is returning an AbstractSting that is \Stringable, so can we lose the repeated (string) casting?
There was a problem hiding this comment.
The Stringable interface is introduced with PHP 8. Like shown in https://symfony.com/blog/new-in-symfony-5-1-string-improvements#use-stringable-interface the Polyfill provides it already.
This will allow you to use union types such as string|Stringable in the future when you later upgrade to PHP 8.
At the moment our methods expect string for both return types and arguments. So to avoid BC breaks I added the casts in the first step. Changed them to calling __toString() now (a3e4402).
There was a problem hiding this comment.
Please add a ticket to 4.0 to change type hints to \Stringable where possible?
| @@ -73,18 +74,18 @@ public function decode(string $jsonFilePath) | |||
| { | |||
| $base = str_replace('\\', '/', dirname($jsonFilePath)); | |||
| $srcPath = dirname(__DIR__, 4) . '/src/'; | |||
| $base = mb_substr($base, mb_strlen($srcPath)); | |||
| $base = s($base)->slice(mb_strlen($srcPath)); | |||
There was a problem hiding this comment.
now here you do not cast (string). It seems this should be consistent.
There was a problem hiding this comment.
Added __toString() call in bd77d26
I tried that before but it seems to be ignored; probably because |
|
can use use StyleCI to enforce future casts to require the space? |
I mean add the files requirement directly to the composer file |
Done in 32f350e |
Done in 264e51d |
Needs to be changed in distribution, too. |
Description
This PR introduces the String component for some string operations.
Currently the
functions.phpfile is explicitly included to make the helper methods available (u(),b(),s()).This inclusion can be removed again when #4352 is done because then the String component will be explicitly added by composer which adds
"files": [ "Resources/functions.php" ]to the autoloader (link).