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

[shaedrich]

Btw, is there a reason, why the null coalescing assignment operator is used instead of just the null coalescing operator?

framework/src/Illuminate/Support/Str.php

Lines 441 to 452 in 5d81b45

	/**
	* Wrap the string with the given strings.
	*
	* @param string $value
	* @param string $before
	* @param string|null $after
	* @return string
	*/
	public static function wrap($value, $before, $after = null)
	{
	return $before.$value.($after ??= $before);
	}

[sydgren]

Btw, is there a reason, why the null coalescing assignment operator is used instead of just the null coalescing operator?

framework/src/Illuminate/Support/Str.php

Lines 441 to 452 in 5d81b45

	/**
	* Wrap the string with the given strings.
	*
	* @param string $value
	* @param string $before
	* @param string|null $after
	* @return string
	*/
	public static function wrap($value, $before, $after = null)
	{
	return $before.$value.($after ??= $before);
	}

It does indeed seem unnecessary to reassign the $after variable.

I guess it's some leftovers from a previous change. Perhaps $after ??= $before has been on its own line above the return statement?

[shaedrich]

Would make sense, but—I checked it real quick—nope, it's always been this way (since 8ea50bc) 🤔

@stevebauman Is there a reason behind that, that we are missing or can the equal sign be removed?

[tontonsb]

Hey @shaedrich isn't wrap a bit different in that it always adds the characters instead of leaving things like <my-reference> intact?

[shaedrich]

@tontonsb Oh, you are right 🤦🏻 👍🏻 Shouldn't there be a method for that? But wrap() is already taken as a name—or would you add a flag to wrap()? 🤔

[tontonsb]

@shaedrich unfourtunately I don't have a nice solution for this.

In other places (collections and Arr) wrap is already idempotent. I would've assumed the same about Str::wrap if you hadn't highlighted the method in this PR. So wrap seems like the correct name for that.. But the current API is already complex enough without a fourth param...

I looked into the history and found surround. But I'd find it intuitive to have names be somewhat like this:

Str::wrap('\_(ツ)_/', '¯'); // ¯\_(ツ)_/¯
Str::wrap('¯\_(ツ)_/¯', '¯'); // ¯\_(ツ)_/¯
Str::surround('¯\_(ツ)_/¯', '¯'); // ¯¯\_(ツ)_/¯¯

But it doesn't seem appropriate to move the existing behaviour to another name...

[stevebauman]

@shaedrich Apologies for the late reply

I wasn't the one to initially PR Str::wrap -- it was added initially here:

https://github.com/laravel/framework/pull/41455/files#diff-3e127621d600522d2ea38948b7f7dc80f875966049aba26391c901efe0eb59d4R1004

And then I moved it here:

https://github.com/laravel/framework/pull/44207/files#diff-262e10b3591ac74c9157a3e732ee544209ea224da8af4844ebacbb1e8e2713d4R350