segmenter: add OffsetInBytes and LengthInBytes to Grapheme, Line, and Word

[hajimehoshi]

Track UTF-8 byte positions alongside rune positions in the attribute iterator, and expose them as OffsetInBytes and LengthInBytes fields on Grapheme, Line, and Word structs. This allows users to efficiently extract segments from byte slices or strings without O(n) conversion.

Fixes #240

[Copilot]

Pull request overview

This PR extends the segmenter package API to expose UTF-8 byte offsets/lengths for each produced segment, enabling callers to slice the original string/[]byte input without converting to []rune first (Fixes #240).

Changes:

• Track UTF-8 byte position alongside rune position inside the internal attributeIterator.
• Expose OffsetInBytes and LengthInBytes on Grapheme, Line, and Word.
• Add a test validating that byte slicing via the new fields matches the segment Text for several UTF-8 inputs across all init modes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
segmenter/segmenter.go	Adds internal byte-position tracking and exposes byte offset/length on segment structs.
segmenter/segmenter_test.go	Adds coverage ensuring byte offsets/lengths reproduce the expected substring for graphemes/lines/words.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[hajimehoshi]

I think it's ready. Please take a look, thanks

[benoitkugler]

Thank you for this contribution.

I'm not a big fan however of the additional complexity incurred by invalid utf8 handling.

Have you concrete use case you must support ? Could we instead require valid utf8 is passed to the segmenter ? That would align more closely to the scope of the library (implicitly defined by the use of rune slices).

[hajimehoshi]

Have you concrete use case you must support ?

In Guigui (a GUI framework made with Ebitengine), I need to implement a text auto-wrapping, and this requires a segmenter. Now I am using uniseg, but this is relatively inactive.

Could we instead require valid utf8 is passed to the segmenter ?

Yeah, I think this makes sense.

I think there are some options, but the simplest way is let it panic:

1. Panic at InitWithString and InitWithBytes when the source includes an invalid sequence. (BTW, if the source includes U+FFFD as a valid sequence intentionally, this should be treated correctly, so we should distinguish them...)
2. Panic at (*attributeIterator).next if the rune is an invalid rune (e.g. U+FFFF, which never exists)

This enables to remove some checks like invalidUTF8Indices from the current change.

Another option is just silently ignore such runes, but this sounds a little risky. What do you think?

[benoitkugler]

What about returning an error from InitWithString and InitWithBytes ? And then panic in (*attributeIterator).next

[hajimehoshi]

What about returning an error from InitWithString and InitWithBytes ?

I slightly prefer panics since the condition when to panic is very clear and a user can check before invoking InitWithString/Bytes. Also this would break a comaptibility with v0.3.4. But if you want to change the signature, I'm fine to do so.

[hajimehoshi]

Also, if InitWithString and InitWithBytes can return an error, should Init also return an error?

[benoitkugler]

Ah, my mistake, I've overlooked the fact that InitWithString and InitWithBytes are already defined in v0.3.4.

I'm in favor of the panic then.

Whats @andydotxyz @whereswaldon think about this question ?

[andydotxyz]

Please don't (ever) panic in a library. Unless it is truly an unrecoverable fatal flaw in the input.
Displaying unicode incorrectly is not one of those things.

We could log it aggressively if returning an error is not an option.

[hajimehoshi]

So if we don't panic, don't return errors, and don't handle invalid sequences in the current way, the only way is to just ignore such sequences or log them, and the result of *InBytes would be undefined for such sequences or runes.

[benoitkugler]

Perhaps this case justifies an API change ?
We only added the new InitXXX functions really recently, @hajimehoshi is probably the only consumer so far.

[benoitkugler]

Also, if InitWithString and InitWithBytes can return an error, should Init also return an error?

Only InitWithString and InitWithBytes would return an error, Init would not.

[hajimehoshi]

Perhaps this case justifies an API change ?
We only added the new InitXXX functions really recently, @hajimehoshi is probably the only consumer so far.

True. Almost nobody uses the API. I've not used these yet neither.

Only InitWithString and InitWithBytes would return an error, Init would not.

It's ok, but I feel like this is inconsistent. An invalid rune like U+FFFF can come to Init, right?

[benoitkugler]

It's ok, but I feel like this is inconsistent. An invalid rune like U+FFFF can come to Init, right?

Yes, you're right. So we would have an undefined behavior in case someone uses Input with invalid runes and String/Bytes iterators. This is not the proper way to use the Segmenter, so I'm fine with keeping this edge case (for simplicity and performance in the common case).

[hajimehoshi]

Updated this PR.

[hajimehoshi]

An invalid rune like U+FFFF can come to Init, right?

This was my understanding but U+FFFF is treated as a one valid rune in Go. An invalid rune is for example U+7FFFFFFF. https://go.dev/play/p/-Axax5XcldD

[benoitkugler]

Thank you for the simplification, I'm really happy with this change !

[whereswaldon]

Thank you for implementing this. Looks reasonable to me!