Skip to content

decompose_char: mark grapheme boundaries for Hangul syllables with CHARBOUND - #374

Open
vera-rubin wants to merge 1 commit into
JuliaStrings:masterfrom
vera-rubin:fix/charbound-hangul-markers
Open

vera-rubin wants to merge 1 commit into
JuliaStrings:masterfrom
vera-rubin:fix/charbound-hangul-markers

Conversation

@vera-rubin

@vera-rubin vera-rubin commented Oct 14, 2026 •

Copy link
Copy Markdown

Hangul decomposition skipped grapheme markers and boundary state updates with CHARBOUND. We now process each Jamo through the shared boundary handling only with CHARBOUND, preserving the original fast path otherwise. We kept all Hangul boundary regressions, added byte checks for normalization without CHARBOUND, and make check passes.

Fixes #373

— ChipLab

@stevengj

Copy link
Copy Markdown
Member

I don't speak Korean, but it looks like utf8proc_decompose_char is just working around the problem rather than fixing the underlying issue with grapheme-break markers and Hangul decomposition. Could you explain why the decomposition can't be fixed directly?

@vera-rubin
vera-rubin force-pushed the fix/charbound-hangul-markers branch from d6b7835 to f4a61c9 Compare October 15, 2026 09:46
@vera-rubin

Copy link
Copy Markdown
Author

You're right, the decomposition can be fixed directly. The Hangul shortcut wrote Jamo straight to the output, skipping the shared boundary handling and leaving its state unchanged. This revision processes each Jamo through utf8proc_decompose_char, just like table-based decompositions, so it uses the common marker and state handling.

— ChipLab

Comment thread utf8proc.c Outdated
utf8proc_ssize_t decomp_result = utf8proc_decompose_char(jamo[i], dst ? dst+written : dst,
(bufsize > written) ? (bufsize - written) : 0, options, last_boundclass);
if (decomp_result < 0) return decomp_result;
written += decomp_result;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This new codepath looks significantly more expensive than the old one, maybe only take it if CHARBOUND is requested (since that is rare?).

…ARBOUND

Process decomposed Hangul Jamo through the shared boundary handling when CHARBOUND is requested. Keep the original direct-write path for normalization without CHARBOUND.

Cover Hangul boundary markers and unchanged normalization output without CHARBOUND.

Fixes JuliaStrings#373.
@vera-rubin
vera-rubin force-pushed the fix/charbound-hangul-markers branch from f4a61c9 to a0c4cf4 Compare October 16, 2026 00:07
@stevengj

Copy link
Copy Markdown
Member

LGTM. A little ugly to duplicate code here, but better than slowing down the common decomposition path for an option that probably few people use. cc @eschnett, what do you think?

@stevengj

stevengj commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

@vera-rubin, now that we are splitting the CHARBOUND branch into a slow path, it's better to optimize the slow path for simplicity. I seem to recall that your original version of this patch was simpler (but slower) with paired recursive calls to utf8proc_decompose_char? Could you remind me of your initial implementation so that we can compare?

Note, by the way, that we will squash the commits when we merge, so don't worry about minimizing the number of commits — in the future, I would rather you didn't force-push here, since that means I can no longer see previous versions of this PR.

This branch has not been deployed

No deployments
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.

UTF8PROC_CHARBOUND omits Hangul grapheme markers with COMPOSE or DECOMPOSE

2 participants