This implements the Grapheme clustering as required by
P1868R2 width: clarifying units of width and precision in std::format
This was omitted in the initial patch, but the paper was marked as completed. This really completes the paper.
Differential D126971
[libc++] Implements Unicode grapheme clustering Mordante on Jun 3 2022, 9:10 AM. Authored by
Details
This implements the Grapheme clustering as required by This was omitted in the initial patch, but the paper was marked as completed. This really completes the paper.
Diff Detail
Event TimelineThere are a very large number of changes, so older changes are hidden. Show Older Changes Comment Actions Use a better way to store the data. Note the code and comment needs more polish. Comment Actions Minor cleanups and addresses @STL_MSFT's review comments while upstreaming some of these changes. Comment Actions Mostly looks good but at least one output doesn't look right which suggests an error in width estimation. Also a few nits inline.
Comment Actions Thanks a lot for your review!
Comment Actions I have some comments. Being a total unicode newbie, this is mostly out of my depth, but the general approach looks sensible to me. I'll have a closer look at https://www.unicode.org/reports/tr29/tr29-35.html#Grapheme_Cluster_Boundaries to provide a more meaningful review, but this basically LGTM via the assumption that the test coverage would catch issues in the implementation.
Comment Actions Thanks for the reviews!
Comment Actions We're seeing failures after this patch https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8808100903143342625/overview could you please take a look? Comment Actions It seems one of the clang-tidy changes flags the current code. I should be able to fix this rather fast. |