Skip to content

Conversation

@shaedrich
Copy link
Contributor

#53987 potentially changes the output of Illuminate\Mail\Mailables\Headers::referencesString()

As @tontonsb explained in #53987 (comment), previously, already wrapped strings would not be wrapped again, but now, they would, resulting in two leading and two trailing angle brackets.

This deviation wasn't discovered by a test, so I added one.

@shaedrich shaedrich changed the title Revert #53987 Revert "[11.x] Use Str::wrap() instead of nesting Str::start() inside Str::finish()" Feb 8, 2025
{
return (new Collection($this->references))
->map(fn ($messageId) => Str::wrap($messageId, '<', '>'))
->map(fn ($messageId) => Str::of($messageId)->start('<')->finish('>'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not reversing, this is adding a Stringable instance overhead on each $references element.

Previous code used nested calls to Str::start() and Str::finish()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it reverses the functionality which is now asserted by the added test, not necessarily the syntax, which is more readable imo.

@taylorotwell taylorotwell merged commit 814054e into laravel:11.x Feb 10, 2025
44 checks passed
@shaedrich shaedrich deleted the revert-pr-53987 branch February 10, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants