-
-
Notifications
You must be signed in to change notification settings - Fork 152
Allow using null in addText, addHtml and insert; Closes #332 #333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Was it possible to use null with addHtml? |
|
@dg You mean before v4.1? For sure i was using it with both. its because of the if that is there: if (!$text instanceof HtmlStringable) {
$text = htmlspecialchars((string) $text, ENT_NOQUOTES, 'UTF-8');
}EDIT: I got methods mixed up, addHtml goes directly to insert which has this line on top: $child = $child instanceof self ? $child : (string) $child;EDIT2: I just checked issue #1570 that led to this. I don't understand why the null worked for me, since it should have thrown error like stated in the issue. However I am looking at code that does result in |
|
Oh now i see why, I have Html extended and i added this public function insert(?int $index, HtmlStringable|string|null $child, bool $replace = false): static
{
if ($child === null) {
return $this;
}
return parent::insert($index, $child, $replace);
}Ok, what should I do now? Remove that from the pull request? leave it? |
|
I definitely don't want to add support for null. |
|
For insert and addHtml or just insert? |
|
For none of them. |
|
Alright, no point in adding that to addText, i will deal with this in my code. |
|
Hey, I think we're misunderstanding each other. In principle, it would be better to add |
|
No no, i understand and I agree with you, i have bigger issue with the fact that it is in minor version and not major one. //EDIT: This was in my eyes just a stopgap to undo the bc break for me and remove this in v5.0 as the null really does not make sense |
This is reintroducing back option to use null with Html::addText and Html::addHtml after switching from mixed type to narrower one.