Skip to content

[5.3] Make str_replace_array() not interpret needle as a regex#12259

Merged
taylorotwell merged 3 commits into
laravel:masterfrom
vlakoff:str-replace-array
Feb 16, 2016
Merged

[5.3] Make str_replace_array() not interpret needle as a regex#12259
taylorotwell merged 3 commits into
laravel:masterfrom
vlakoff:str-replace-array

Conversation

@vlakoff
Copy link
Copy Markdown
Contributor

@vlakoff vlakoff commented Feb 11, 2016

  • Targetting 5.3 as it's a breaking change. See vlakoff@3db6f2a.
  • Also adding a Str::replaceArray method for codebase consistency.

/**
* Replace a given value in the string sequentially with an array.
*
* @param string $search
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please don't align.

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.

This would reduce consistency with the rest of Str.php and helpers.php files.
I think it would be better to change code style in whole files at once, and in a dedicated commit.

@taylorotwell
Copy link
Copy Markdown
Member

Needs full description of purpose of this change, what it does, what it fixes, purpose for existing, etc.

@vlakoff
Copy link
Copy Markdown
Contributor Author

vlakoff commented Feb 14, 2016

My guess is that when you implemented this function (in ab0fa95) you just thought "it's handled as a regex, but what's the big deal?".

There was a similar situation with Str::is, fixed in a294377.

There is kind of a convention, that PHP functions str_*() deal with strings, preg_*() deal with regexes. I think deviating from this convention is rather confusing.

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