Trim invisible characters at string edges

[meh7an]

No description provided.

[CLAassistant]

All committers have signed the CLA.

[ilya-fedin]

looks like it's going to break lots of language scripts

[ilya-fedin]

Thanks! The idea is right and replacing the ad-hoc ch == QChar(8203) with a named predicate is an improvement. But the list itself needs some trimming.

0x1160 must not be trimmed — it is part of jamo composition

U+1160 HANGUL JUNGSEONG FILLER (category Lo) is not an "invisible character": per the standard (3.12, Conjoining Jamo Behavior) it fills the vacant position of an incomplete syllable block, so an isolated leading consonant is written as U+1100 U+1160 ("ᄀᅠ"). A message ending with such a block loses the filler after trimming and shapes differently (a bare choseong instead of a syllable). Its counterpart U+115F CHOSEONG FILLER (U+115F U+1161 = an isolated vowel) is absent from the list, which is correct — but then 1160 should go as well. Safe here are only the compatibility fillers 0x3164 and 0xFFA0, which take no part in composition.

0x2800 breaks a real use case

Braille pattern blank is widely used exactly as an indent that does not get eaten, in braille/ASCII art. And IsTrimmed() is not display-only: TextUtilities::Trim() in ui/text/text_entity.cpp runs on the outgoing text before sending, so leading/trailing blanks of the art get silently cut on the sender side, while the server and other clients keep them — a divergence plus mangled art. On top of that, HasSendText() in tdesktop (Telegram/SourceFiles/chat_helpers/message_field.cpp) will stop treating an all-blank message as text and block sending it. I'd drop 0x2800; if the goal is empty names, that is a different predicate in a different place.

0x17B4/0x17B5 are Mn — at the end of a string they belong to the previous letter

A leading occurrence is meaningless and safe to strip, but a trailing AQ/AA is the vowel of the last consonant, and removing it changes the word. There is a precedent for trimming trailing marks (IsBad() already trims VS1–VS15, 65024–65038), so it is your call, but it should be a deliberate one rather than "along with ZWSP". Also note both already match IsDiacritic(), so the meaning gets duplicated across two predicates.

The Cf group is better not spelled out by hand

0x200B/200C/200D/2060/FEFF are all Other_Format; a single ch.category() == QChar::Other_Format covers them, and also U+180E, U+2061–2064, U+200E/200F, U+061C and U+00AD, and it does not go stale as Qt updates its Unicode tables. Surrogates have category Cs, so tag characters U+E0020–E007F (flags) are not affected. The catch is that this widens behaviour (soft hyphen, LRM/RLM) — if that is not wanted, the comment should at least explain why exactly these five out of Cf.

Smaller things

• IsBlankLike is exported in ui/text/text.h, but there are no users outside text.cpp — no need to widen the public API, a static function next to it (or just the cases inlined into IsTrimmed()) is enough.
• The six-line comment is out of style for the file (neighbouring predicates have a one-liner or nothing), and "that predicate marks word boundaries" is inaccurate: word boundaries are IsWordSeparator(), while IsSpace() feeds replaceWithSpace in ui/text/text_block_parser.cpp, i.e. the character gets painted as a space. The conclusion ("these must not go into IsSpace()") is right, the reasoning is not.
• The PR title "Treat invisible characters as spaces" contradicts both the code and the comment itself (must never reach IsSpace()). The commit headline "Trim invisible characters at string edges" is the accurate one — could you rename the PR?
• "safe to strip at the edges of a string, where they join nothing" does not describe all the callers: IsTrimmed() is also used per-character in the middle of the text (HasSendText(), the IV markdown parsing, compose_ai_button_factory.cpp). The semantics of "no meaningful characters" match there, but the wording should be fixed so that the next person does not assume the predicate is edges-only.

[ilya-fedin]

Thanks for the update — the hangul jungseong filler, the braille blank and the khmer inherent vowels are gone, the helper is file-local now, and the comment is both shorter and accurate. Two things below, and one retraction of my own previous suggestion.

0x200D is not blank — it changes the shape of the character next to it

I should have caught this in the first pass. ZWJ is a joining control, and at a string edge it acts on the neighbour that is still there. Shaped with HarfBuzz 13.2.1 against Noto Naskh Arabic / Noto Sans Malayalam / Noto Sans Devanagari:

ARABIC "می"      plain  [uni0649.fina | uni0645.init]
                 +ZWJ   [twodotshorizontalbelowar | uni066E.medi.wide | uni0645.init]
ARABIC "ی" alone plain  [uni0649]           <- isolated form
                 ZWJ+   [uni0649.fina]      <- connected form
MALAYALAM "ന്"    plain  [namlym | viramamlym]
                 +ZWJ   [nachillumlym]      <- exactly the glyph of atomic "ൻ" U+0D7B
DEVANAGARI "क्"   plain  [uni0915 | uni094D]
                 +ZWJ   [uni0915094D]       <- half-form ligature

So a leading or trailing ZWJ switches an Arabic/Persian letter between its isolated and its connected form — that is the standard way of writing a letter form or a word fragment, and it lives exactly at the edge. In Malayalam the legacy encoding of a chillu is consonant + virama + U+200D, and a chillu is by definition word-final, i.e. very often message-final: trimming the ZWJ turns "ൻ" into "ന്", which is a different text, not a cosmetic difference. The atomic chillus U+0D7A–0D7F only arrived in Unicode 5.1, so plenty of existing content and older input methods still produce the ZWJ form.

ZWNJ (0x200C) does not move any glyph at an edge in the same test — there is no neighbour on that side to disjoin. So it is defensible on its own. I would still drop it together with ZWJ: it is the same class of character, the gain is zero (ZWSP is already covered for the "invisible message" case), and "string edge" is not always "shaping run edge" — SingleLine() and the dialogs-list previews concatenate a name and the text into one string and shape them together, so a leading ZWNJ that was there precisely to keep two things apart can disappear where it mattered.

0x3164 / 0xFFA0 are graphic letters with a non-zero advance

0x3164 Lo  East_Asian_Width=W (full width)      <compat> 1160
0xFFA0 Lo  East_Asian_Width=H (half width)      <narrow> 3164

They are not zero-width format characters, they are blank glyphs: measured in a font, U+3164 has the same advance as a hangul syllable or an ideographic space, U+FFA0 the same as a regular space. That is why they get used as non-collapsing indentation — the very argument that removed U+2800 from the list applies here unchanged; the only difference is the general category (So vs Lo).

On top of that, U+3164 has a compatibility decomposition to U+1160, the jungseong filler we just agreed must not be trimmed, and U+FFA0 decomposes to U+3164 — so the list would trim a character while preserving its own NFKC form.

There is also a boundary question. Everything IsTrimmed() matches today is non-graphic: Unicode whitespace, the control/format characters of IsBad(), zero-width format characters. Admitting a general-category-Lo letter changes the criterion from "has no graphic representation" to "looks empty to me", and that criterion has no natural stopping point — U+2800, U+115F, U+1160, the various full-width blanks all qualify equally.

Retracting my QChar::Other_Format suggestion

Please disregard that part of my previous comment — you were right to keep an explicit list. I enumerated the class: Cf in Unicode 17 has 170 members, and the blank ones are a small minority. It also contains the prepended concatenation marks (U+0600–0605, U+06DD, U+070F, U+0890, U+0891, U+08E2, U+110BD, U+110CD), which have visible rendering over the digits that follow them — and whose normal position is the start of a string, exactly where a left trim would eat them; the bidi controls and isolates; the deprecated shaping controls U+206A–206F; the paired interlinear annotation characters; the Egyptian hieroglyph quadrat joiners U+13430–1343F; the Duployan overlap controls; the paired musical beam/tie/slur controls; and 97 tag characters. Excluding ~160 of 170 is a whitelist written inside out.

The failure modes are asymmetric, too: a whitelist that misses a character simply fails to trim something, while a category-based rule that misses one destroys text. And each Unicode update silently enrols new Cf characters into the trimmed set with no change in lib_ui (U+0890/0891 arrived in 14.0, U+08E2 in 9.0, the Egyptian set grew in 15.0). The property that would express this correctly, Prepended_Concatenation_Mark, is not exposed by QChar at all.

Summary

Drop 0x200D (and, I would suggest, 0x200C, 0x3164, 0xFFA0). That leaves 0x200B, 0x2060 and QChar::ByteOrderMark, all three verified inert at an edge — they change no glyph next to them and do not break cursive joining even in the middle of a word. If you want the list to cover its own category consistently, U+180E and U+2061–2064 belong there as well by exactly the same test.

Also, the PR title still says "Treat invisible characters as spaces", which contradicts the code and the comment; the commit headline is the right one.

[ilya-fedin]

Thanks — the joining controls and the hangul fillers are gone and the title is fixed. One item left, and it is the one I suggested myself, so my apologies for the extra round.

0x180E should go too

ArabicShaping.txt (17.0) turns out to state the boundary we arrived at by measurement. Out of everything that was ever on this list, exactly three characters are explicitly listed there — and they are exactly the three we rejected:

180E; MONGOLIAN VOWEL SEPARATOR; U   <- Non_Joining
200C; ZERO WIDTH NON-JOINER;     U
200D; ZERO WIDTH JOINER;         C   <- Join_Causing

Everything else in the current list is unlisted and therefore Joining_Type=Transparent by the file's default rule. So U+180E is a joining control, in the same class as the ZWNJ that just left the list, and shaping confirms it:

ARABIC     "ب" + X + "ب"    plain   [uni066E.fina | uni066E.init]     <- joined
                            +180E   [uni066E | uni066E]               <- joining broken, exactly like ZWNJ
MONGOLIAN  "ᠭᠠᠵᠠᠷ" + "ᠠ"     plain   [... uni1837.R.medi | uni1820.A.fina]
                            +MVS    [... uni1837.R.fina | mvs.narrow | uni1820.Aa.isol]

The Mongolian case is grammar, not decoration: MVS is what separates a detachable suffix vowel from the stem and selects the separate form for it.

And at an edge it is not invisible either. In Noto Sans Mongolian a trailing MVS gets a real glyph with a real advance:

"ᠭᠠᠵᠠᠷ" + MVS   [uni182D.Hx.init=0+788 | uni1820.A.medi=1+284 | uni1835.J.medi=2+530
                | uni1820.A.medi=3+284 | uni1837.R.fina=4+501 | mvs.nominal=5+389]
                                                                 ^^^ a plain space in this font is 260

It only measures as zero-width in fonts that have no glyph for it, where HarfBuzz applies its default-ignorable handling — so the invisibility is a property of the font, not of the character. That is also why it was General_Category=Zs until Unicode 6.3.

The remaining seven (0x200B, 0x2060, 0x2061–0x2064, QChar::ByteOrderMark) pass both checks: they leave cursive joining intact even in the middle of a word, and their own advance is zero.

One line worth adding to the comment

It is probably worth recording why the list is hand-written, because the shape of it is not accidental. The original version of this list was, almost character for character, Unicode's Default_Ignorable_Code_Point set (DerivedCoreProperties.txt: 115F..1160, 17B4..17B5, 180E, 200B..200F, 2060..206F, 3164, FE00..FE0F, FEFF, FFA0, …) — and every single character we removed over these rounds belongs to it. Default_Ignorable means "render nothing rather than a tofu box if you cannot handle this", i.e. an instruction to the renderer; it is not a statement that the character can be dropped from the text. Same for General_Category=Cf, which additionally contains the prepended concatenation marks, the bidi controls and the Egyptian quadrat joiners.

So a note along the lines of "hand-picked: neither Default_Ignorable nor Cf is a safe criterion, these are only the characters that are zero-width and Joining_Type=Transparent" would keep the next person from simplifying the list straight back to where it started.

With U+180E dropped I have nothing further on the substance.