Skip to content
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

add mb support for split function, better replace support #1445

Closed
wants to merge 1 commit into from
Closed

add mb support for split function, better replace support #1445

wants to merge 1 commit into from

Conversation

1emming
Copy link
Contributor

@1emming 1emming commented Jul 11, 2014

This adds multibyte character support for the spilt function.
For example:
"éÄ,éÄ,Ä,Ä,Ä,Ä,Ä,Äほ"|split(',', 3)
Into:
Array
0 =>éÄ
1 =>éÄ
2 =>Ä,Ä,Ä,Ä,Ä,Äほ

And {% set foo = "éÄéÄÄ"|split('', 3) %}
Into:
Array
0 =>éÄé
1 =>ÄÄ

Fix for the replace filter.
When using the current replace filter like this:
{{ "I like %this% and %that%."|replace('%this%', 'foo') }}
The result is:
"I like fooisf and fooaof."

With this fix the result is:
"I like foo and %that%."

And added tests for this.

@stof
Copy link
Member

stof commented Jul 11, 2014

The replace filter passing 2 strings is a letter by letter replacement. The change you made is a BC break and so cannot be accepted (it will break existing projects).

@stof
Copy link
Member

stof commented Jul 11, 2014

btw, given that you do 2 unrelated changes, they should not be in the same PR (and even less in the same commit)

@1emming 1emming closed this Jul 11, 2014
@1emming
Copy link
Contributor Author

1emming commented Jul 11, 2014

You are right. I've splitted the 'split' part out into another PR.
The 'replace' one needs other types of fixes then:
1 - the behaviour is not documented, so it requires that people know it is build on PHP's strstr
2 - it does not support multi byte stuff, for example: {{ "I like %this% and %that%."|replace('%this%', 'ÄÄÄ') }}

@1emming 1emming deleted the MB_split_support branch July 11, 2014 16:26
@stof
Copy link
Member

stof commented Jul 11, 2014

@1emming even if it is not documented, it works this way since 3 years, so we need to consider BC

fabpot added a commit that referenced this pull request Sep 6, 2015
…SpacePossum)

This PR was merged into the 1.x branch.

Discussion
----------

Traversable support for 'replace', 'merge' and 'sort'

Add traversable support to:
* replace
* merge
* sort

Other changes:
* Removes some not need checks
* ~~Make unit test skip if it cannot write to cache because of the user running the test~~
* Tests added for traversable support
* Doc updates

Deprecating:
I think the undocumented (and multibyte not supported) use of `replace(string, string)` can be removed in Twig 2.0.
Adding MB support and documentation of the feature was turned down (#1618)
However removing it is a BC break (#1445 (comment))

Commits
-------

2a17303 Mark test skipped if cannot write to cache directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants