Skip to content

Revert "[11.x] Use Str::wrap() instead of nesting Str::start() inside Str::finish()"#54528

Merged
taylorotwell merged 3 commits into
laravel:11.xfrom
shaedrich:revert-pr-53987
Feb 10, 2025
Merged

Revert "[11.x] Use Str::wrap() instead of nesting Str::start() inside Str::finish()"#54528
taylorotwell merged 3 commits into
laravel:11.xfrom
shaedrich:revert-pr-53987

Conversation

@shaedrich

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants