Repository navigation
Conversation
Backspace and forward delete should each remove one grapheme cluster — the unit a reader perceives as a character. Both removed less, so a single perceived character took several presses to erase. `backdelete` deleted the whole upstream cluster only for a hard line break or an emoji cluster, and otherwise fell back to `char_indices().next_back()`, i.e. one codepoint. `delete` had no special case at all: it removed the downstream *layout* cluster, which comes from shaping and so tracks the font's glyphs rather than perceived characters — splitting a grapheme wherever the font does not ligate the sequence, and splitting a CRLF pair in two. Presses to erase a single grapheme, before → after: | input | backdelete | delete | | ---------------------------------------- | ---------- | ------ | | `👨👩👧👦` ZWJ family | 7 → 1 | 7 → 1 | | `🇯🇵` regional-indicator flag | 2 → 1 | 2 → 1 | | `❤️` `U+2764 U+FE0F` | 2 → 1 | 2 → 1 | | `👋🏽` waving hand + skin tone | 1 → 1 | 2 → 1 | | `e` + combining acute `U+0301` | 2 → 1 | 2 → 1 | | `각` Hangul jamo `U+1100 U+1161 U+11A8` | 1 → 1 | 3 → 1 | | `CRLF` | 1 → 1 | 2 → 1 | Note the last three rows are not emoji at all, so no amount of refining the emoji check would have fixed them: the operations were reaching for the wrong unit. Segment on grapheme-cluster boundaries instead, using the ICU grapheme segmenter already reachable via `AnalysisDataSources`. The `is_hard_line_break() || is_emoji()` special case then becomes unnecessary and is dropped — both are simply graphemes. Add tests covering each case above in both directions, plus a guard that two separate graphemes still take two presses. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I haven't carefully looked at the backspace logic in Parley recently, but in general it should be finer granularity than grapheme clusters. We had fairly carefully worked out logic in linebender/xilem#303, as far as I'm concerned that's still a good reference. |
|
Well, in the version right before that patch it seemed to just delete by codepoint which can produce very strange results for anything that actually combines them like many emoji or non-trivial characters in non-Latin alphabets do but I am certainly open for suggestions for more refined logic from people actually using some of those languages in their everyday typing. |
|
Actually no, that is not quite true, I might have missed something there, let me re-check. |
|
Welcome! Also, as a stylistic thing, I believe that we don't want to land commits as 'co-authored-by Claude'. You are completely responsible for the LLM's output, and so the co-authored claim isn't correct. |
|
I will do that, my apologies. I just didn't want to hide the LLM involvement. I also did some further tests and comparisons and so far it seems like the code before my commit intends to delete emojis with one delete/backdelete call but fails for some reason (I did a branch in my repo that just tests a lot of cases automatically instead of relying on the manual test of "this all looks horribly wrong with the cursor in the middle of emoji and other characters" that I used before that only covered a few cases. It also looks like the story in terms of what users expect varies from culture to culture, in particular where IME and pre-edit composition of characters is involved deleting individual codepoints isn't really that common (India seems to be an exception). Parley also seems to be just about the only software that deletes CRLF as two individual characters with backdelete/delete. But overall I will still need to investigate a bit further before I can update this. |
|
No worries! I'm glad to see that you're being thorough here. I expect that we'll discuss this in next week's text office hours - that will probably be next Tuesday at 07:00 UTC (I believe, you should double check that on Zulip if you wish to join. We are planning a ~monthly office hours to be a more US friendly time slot). It would be great to have you there if the timezone works. This |
|
I have been digging into this a bit deeper but haven't quite gotten to the point where I want to submit any new PRs. So far I stumbled across (but haven't investigated deeper yet)
I did some (largely LLM using and not suitable for inclusion in the upstream repo) initial research and tests of parley behavior on main...taladar:parley:research/traversal-fact-finding but haven't had the time to verify everything in there manually yet for lack of time to work on parley exclusively instead of the project I intended to use parley in. |
Add an `is_grapheme_start` flag to `CharInfo`, computed in `analyze_text` by threading the ICU4X grapheme segmenter through the same boundary iterator used for word and line segmentation. `shape_item` now derives its font-selection segmentation from this flag instead of re-running the grapheme segmenter per item, so analysis is the single source of truth for grapheme boundaries and net segmentation work is unchanged (one pass over the text). An item's first char is treated as a grapheme start regardless of its flag, matching the previous behavior of segmenting each item's substring independently. The `analysis_data_sources` parameter of `Shaper::shape_item` is now unused and has been removed. Beyond deduplicating the segmentation, this makes UAX linebender#29 boundaries available to everything downstream of analysis (in the spirit of Pango's `PangoLogAttr.is_cursor_position`), which is groundwork for grapheme-aware cursor motion and deletion (linebender#693, linebender#694). No behavior change: all layout output and test snapshots are identical.
Add an `is_grapheme_start` flag to `CharInfo`, computed in `analyze_text` by threading the ICU4X grapheme segmenter through the same boundary iterator used for word and line segmentation. `shape_item` now derives its font-selection segmentation from this flag instead of re-running the grapheme segmenter per item, so analysis is the single source of truth for grapheme boundaries and net segmentation work is unchanged (one pass over the text). An item's first char is treated as a grapheme start regardless of its flag, matching the previous behavior of segmenting each item's substring independently. The `analysis_data_sources` parameter of `Shaper::shape_item` is now unused and has been removed. Beyond deduplicating the segmentation, this makes UAX linebender#29 boundaries available to everything downstream of analysis (in the spirit of Pango's `PangoLogAttr.is_cursor_position`), which is groundwork for grapheme-aware cursor motion and deletion (linebender#693, linebender#694). No behavior change: all layout output and test snapshots are identical.
…er#811) LLM Contributions: Unknown for linebender#620, c-emoji. I used them for review, bootstrapping the new crate to follow https://linebender.org/wiki/formatting-scheme/, and getting/implementing the technique of sticking the properties into our existing property infrastructure, plus rebasing/reordering the series. Adaptation of linebender#620 by @fundon. This uses a state machine based on @chansen's [c-emoji](https://github.com/chansen/c-emoji) to implement detection of Emoji sequences, as described in [tr-51](https://www.unicode.org/reports/tr51/tr51-29.html). This is Unicode 17 Emoji, as opposed to Unicode 18 (releasing tomorrow...), but I don't think the difference has a huge impact for us. The state machine lives in a new crate `parley-emoji`, so as to follow c-emoji's license. We were unable to get its code relicensed under our MIT/Apache 2.0 license. With this, we are *not* completely compliant with Unicode Emoji, but this gives us a significantly better jumping off point. Things I've chosen to defer: - Better testing. I have local changes which generate a suitable subset of Noto Color Emoji, but I think end-to-end testing needs more care. - Also not yet tested using the upstream Unicode data. - Splitting a single grapheme cluster into its 'text presentation/emoji presentation' preferred sections. This can happen in three cases that I'm aware of: - Prepend character then Emoji, all within one grapheme cluster - Non-Emoji modifier base, then Fitzgerald skin tone modifier - ZWJ sequence containing VS15 text variation selectors. - Having the Emoji font in the fallback chain, even if text presentation is preferred. - Thinking through how the 'reject' case should be handled - Doing the proper rendering of malformed tag sequences, which is required by spec, but which as far as I know, no system implements. - [#font-tech > Appearance of unsupported emoji sequences](https://xi.zulipchat.com/#narrow/channel/296640-font-tech/topic/Appearance.20of.20unsupported.20emoji.20sequences/with/623471132). - Verifying that the handling of RI sequences is correct - in particular, the way that three regional indicators in a row merge into grapheme clusters can lead to jank. - Fix the editing behaviour to use this properly (see also linebender#693) - Documenting our level of compliance with the Unicode Emoji spec. - Handling legacy modifier sequences with an intervening VS16. There's a TODO in the code for this. - The 'emoji presentation locale' thing. I don't believe any of those are blocking - this is still a better state than before. Fixes linebender#744. Supersedes linebender#620, linebender#788, linebender#692. cc @kane50613 (wrt linebender#745). **Changelog** > ### Fixed > > - Parley's detection of Emoji is now more robust. --------- Co-authored-by: Fangdun Tsai <fundon@pindash.io> Co-authored-by: Tom Churchman <thomas@churchman.nl>
LLM Contributions: Claude
Backspace and forward delete should each remove one grapheme cluster — the unit a reader perceives as a character. Both removed less, so a single perceived character took several presses to erase.
backdeletedeleted the whole upstream cluster only for a hard line break or an emoji cluster, and otherwise fell back tochar_indices().next_back(), i.e. one codepoint.deletehad no special case at all: it removed the downstream layout cluster, which comes from shaping and so tracks the font's glyphs rather than perceived characters — splitting a grapheme wherever the font does not ligate the sequence, and splitting a CRLF pair in two.Presses to erase a single grapheme, before → after:
👨👩👧👦ZWJ family🇯🇵regional-indicator flag❤️U+2764 U+FE0F👋🏽waving hand + skin tonee+ combining acuteU+0301각Hangul jamoU+1100 U+1161 U+11A8CRLFNote the last three rows are not emoji at all, so no amount of refining the emoji check would have fixed them: the operations were reaching for the wrong unit. Segment on grapheme-cluster boundaries instead, using the ICU grapheme segmenter already reachable via
AnalysisDataSources. Theis_hard_line_break() || is_emoji()special case then becomes unnecessary and is dropped — both are simply graphemes.Add tests covering each case above in both directions, plus a guard that two separate graphemes still take two presses.