[ChrisAbner]

RFC: https://wiki.php.net/rfc/grapheme_str_pad

Adds grapheme_str_pad() to ext/intl, the grapheme cluster counterpart of str_pad() and mb_str_pad().

function grapheme_str_pad(string $string, int $length, string $pad_string = " ", int $pad_type = STR_PAD_RIGHT): string|false {}

• The target length and the input length are counted in grapheme clusters, like grapheme_strlen().
• The padding string is repeated and its last copy is only cut on a grapheme cluster boundary.
• Errors follow mb_str_pad() (ValueError for an empty padding string or an invalid $pad_type, no error when nothing needs padding). Invalid UTF-8 returns false and sets the intl error, like grapheme_substr().
• ASCII input without "\r\n" takes a fast path and gives the same result as str_pad().

Tests: ext/intl/tests/grapheme_str_pad_basic.phpt (NFC/NFD, combining dakuten, Devanagari, emoji with modifier and ZWJ sequences, cuts of the padding string, "\r\n"), grapheme_str_pad_errors.phpt, grapheme_str_pad_oom.phpt and grapheme_str_pad_overflow.phpt.

[Sjord]

Should this be one of the fallible zend string functions? So that it respects the allowed memory size?

• str_pad("a", PHP_INT_MAX, "b") gives Fatal error: Allowed memory size of 134217728 bytes exhausted.
• grapheme_str_pad("a", PHP_INT_MAX, "b") gives Hard memory limit of 268435456 bytes exhausted.

[ChrisAbner]

Good catch, thanks. Fixed in f397ad9: it now uses zend_string_safe_falloc() with RETURN_OOM(), like str_pad() and mb_str_pad(), so your example hits the regular memory_limit error. I added grapheme_str_pad_oom.phpt for it.

[devnexen]

Hi @ChrisAbner, the RFC says invalid UTF-8 in $pad_string returns false, but it is only checked when padding is needed:

var_dump(grapheme_str_pad("abc", 2, "\xff")); // string(3) "abc"

it is fine behaviour wise I think, it matches the empty $pad_string case, but it might deserve a mention in the RFC and a test wdyt ?

[ChrisAbner]

Thanks, agreed. Like mb_str_pad(), neither $pad_string nor $pad_type is checked when no padding is needed. I've updated the RFC to say so and added a test for both cases in 829161e.

[devnexen]

I ll have a better overall look later but I think you need to check its status here.

[ChrisAbner]

Done in 333837b.
It now checks the status of ubrk_setUText(). If it fails, the function sets the intl error, closes the iterator and the UText, and returns false.
Later, 48b7896 moved the counting to UTF-16, like grapheme_strlen(), so the same check is now on ubrk_setText().

[ChrisAbner]

Pushed three more commits besides 333837b.

ed4eee9 adds a test for the two "String size overflow" checks. They had no test before.

f164108 clears the intl error at the start of the function. normalizer_normalize() and transliterator_transliterate() already do this. No grapheme_* function does it yet.

48b7896 counts grapheme clusters through UTF-16 with intl_convert_utf8_to_utf16(), like grapheme_strlen(), instead of validating with U8_NEXT() and walking a UTF-8 UText. The status check stays on ubrk_setText(). In a release build it is about a third faster on large non-ASCII strings.

About f164108: I can drop it if you prefer consistency with the other grapheme functions. If it looks fine, I can apply the same change to the other grapheme functions in a separate PR.

[devnexen]

A fragment can merge at a join, as in any concatenation, for example a padding string that starts with a combining mark or U+200D ZERO WIDTH JOINER, a lone regional indicator, Hangul conjoining jamo, a $string that starts with a combining mark, or a “\r” next to a “\n”. The result then has fewer grapheme clusters than $length, just as grapheme_strlen($a . $b) can be less than grapheme_strlen($a) + grapheme_strlen($b)

I think deserve some test coverage.

[ChrisAbner]

Added in e197470: one test per merge case listed in the RFC. The results are the same with every ICU version I tried, from 57 to 76. I also added 371d81c, which checks that invalid input is rejected even when no padding is needed. Happy to add more cases if you think of others. These are edge cases, but they should be covered.

[devnexen]

nit:

...
} catch (\Throwable $e) {
     $e::class . ': ' . $e->getMessage() . "\n";
}

[devnexen]

apart of the nit, I think it s good to go (or at least nearly) but will give time for other reviews before I merge.