trim support for multibyte spaces / mb_trim()

[8ctopus]

Description

Hello!

the standard trim() function has a major flaw as it does not cleanup multibyte spaces.
I see two options to this problem:

• implement mb_trim()
• add support for multibyte spaces to the existing function

Code example in php 8.1.8

// does not work
echo var_dump(trim('小野　')); // string(9) "小野　"

// temporary solution
echo var_dump(preg_replace("/^\s+|\s+$/u", "", "小野　")); // string(6) "小野"

[cmb69]

• add support for multibyte spaces to the existing function

That is not an option, since trim() is supposed to work with bytes, so changing this would cause an unnecessary BC break.

If it was only about UTF-8 encoded strings, I'd say using PCRE is fine, but that wouldn't work for other multibyte encodings, so introducing mb_trim() might be a sensible improvement.

@alexdowad, thoughts?

[alexdowad]

Thanks for the question, @cmb69. Here are a few thoughts:

• I see your point about working with other multi-byte encodings. We have a whole library (mbstring) of text-processing functions which work on a variety of encodings, so making this one also work with all supported text encodings would have the advantage of consistency with the existing PHP standard library.
• On the other hand, I have had experiences on other FOSS projects before where I generalized certain features, just out of principle... only to discover later that a special case was all that the users needed, and nobody was using the full generality of the feature. It only created an implementation and maintenance burden.
• Using existing code in mbstring, making this feature generalize to all supported text encodings is not difficult. It would only take a small amount of code.

Please let me know what you want!

[cmb69]

Well, there is https://bugs.php.net/bug.php?id=23501 (only recently been closed), and some duplicates of that, so probably the functionality is useful for some.

[alexdowad]

Well, there is https://bugs.php.net/bug.php?id=23501 (only recently been closed), and some duplicates of that, so probably the functionality is useful for some.

OK, sounds good to me!

I am ready to implement mb_trim, but would like to ask @8ctopus: Are you able to provide a list of all the Unicode codepoints which should be trimmed from the start/end of the input string?

For @cmb69, if this is merged, might you be in a position to add it to the PHP docs?

Any comments from other developers? @nikic @kamil-tekiela @Girgias

[cmb69]

if this is merged, might you be in a position to add it to the PHP docs?

Yes. This might require an RFC, though. But maybe start with a PR; we can go from there. And since it's too late for PHP 8.2 anyway, it's not urgent anyway. :)

[nikic]

Adding mb_trim sounds reasonable to me. Trimming does not reduce to a single-byte implementation for self-synchronizing encodings, so having an mbstring implementation adds actual value.

[KapitanOczywisty]

@alexdowad Probably mb_trim should remove the same characters as regex \s, with addition of \0:

Same as trim:

• U+0009 \t
• U+000A \n
• U+000B \v
• U+000D \r
• U+0020 SPACE (also in Separator category)

Not removed in trim

• U+000C \f, it's not removed in trim() by default, probably it wasn't common enough, but it's ok for mb_trim

Removed in trim, but not included by \s

• U+0000 \0

whole Separator Z category (19 codepoints):

• U+0020 SPACE
• U+00A0 NO-BREAK SPACE
• U+1680 OGHAM SPACE MARK
• U+2000 EN QUAD
• U+2001 EM QUAD
• U+2002 EN SPACE
• U+2003 EM SPACE
• U+2004 THREE-PER-EM SPACE
• U+2005 FOUR-PER-EM SPACE
• U+2006 SIX-PER-EM SPACE
• U+2007 FIGURE SPACE
• U+2008 PUNCTUATION SPACE
• U+2009 THIN SPACE
• U+200A HAIR SPACE
• U+2028 LINE SEPARATOR
• U+2029 PARAGRAPH SEPARATOR
• U+202F NARROW NO-BREAK SPACE
• U+205F MEDIUM MATHEMATICAL SPACE
• U+3000 IDEOGRAPHIC SPACE

Other symbols

• U+0085 NEXT LINE (NEL)
• U+180E MONGOLIAN VOWEL SEPARATOR

[8ctopus]

@alexdowad Looks like @KapitanOczywisty understands it better than I do. I've checked and the Japanese space I was referring to in my example is covered (U+3000 IDEOGRAPHIC SPACE). Thank you all for looking into the matter.

[alexdowad]

if this is merged, might you be in a position to add it to the PHP docs?

Yes. This might require an RFC, though. But maybe start with a PR; we can go from there. And since it's too late for PHP 8.2 anyway, it's not urgent anyway. :)

😆

I'm sure this is not your intention, but the mention of "RFC" makes me reluctant to work on this right now. The reason is because I am still refactoring and optimizing a lot of mbstring code, and if I prepare a PR and it's not quickly merged, then by the time an RFC is prepared and approved, it's almost certain that the patch won't apply and will need to be rewritten.

I would rather write the patch after the RFC process. Or does an RFC require a sample implementation?