feat: add versioned IME contract - #30
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
IME support is essential for enterprise level — CJK input is non-negotiable for international adoption. The code quality here is high: UTF-8 byte offset validation, comprehensive content purpose/hint enums, clean separation of composition and surrounding text. UTF-8 byte offsets match the cross-platform consensus (Wayland, GTK4, winit all use them).
Our principle: we can be better than enterprise references, but never worse. We study Flutter, GTK4, winit, Wayland — take the best patterns, improve where we can. Some innovations in this PR may genuinely improve on the references. But several points put us below the enterprise baseline. Here's the full analysis.
1. V2 interface pattern — no enterprise precedent
We checked Flutter, GTK4, winit, and Wayland text-input-v3. None use versioned interfaces within a single IME API:
| Framework | How IME evolves |
|---|---|
| Flutter | Adds fields with defaults to TextInputConfiguration struct |
| GTK4 | Adds GObject properties to GtkIMContext |
| Wayland | Bumps entire protocol versions (v1 → v2 → v3) |
| winit | Single ImeRequest enum, capability check at enable time |
The V2 suffix creates permanent maintenance burden (V1 + V2 forever). Pre-v1.0, we can break interfaces freely — the entire point of being pre-v1.0 is that we don't need backward-compatible overlays.
ADR-045 explicitly chose Option A (extend IMEController directly, breaking change) over Option C (new IMEController2). This PR implements what the ADR rejected.
Suggestion: Follow ADR-045 — extend IMEController directly. Pre-v1.0 breaking change is fine. If we adopt V2 now, we're stuck maintaining two parallel interfaces after v1.0.
2. Capability granularity — 9 bits vs winit's 3 groups
winit is the only enterprise framework with explicit capability discovery, using 3 coarse groups: HINT_AND_PURPOSE, CURSOR_AREA, SURROUNDING_TEXT — each mapping to a real platform boundary. This PR goes further with 9 fine-grained bits (Composition, Commit, Cancel, Disabled, DeleteSurrounding, CursorArea, SurroundingText, ContentPurpose, ContentHints).
More granularity can be an advantage — we can be better than references. But it raises a design question: on current platforms, some of these always come together (Composition + Commit + Cancel are atomic — IME is either active or not; ContentPurpose + ContentHints are always paired). Is there a platform scenario where these need to be queried independently?
Question for @besmpl: What's the reasoning behind 9 separate bits instead of winit's 3-group model? Are there specific platform cases where, for example, Composition is supported but Commit is not? If the extra granularity is intentional and forward-looking — that's a valid design choice, and we'd like to understand the rationale.
3. ContentPurposeChat — goes beyond protocol, which is fine
Wayland text-input-v3 defines 14 purposes (0-13, ending at terminal). GTK4 has 11, winit has 11. Chat appears in none of these — but Flutter adds twitter, streetAddress, webSearch beyond the Wayland set for real use cases. Chat (messaging apps, chat widgets) is a legitimate purpose.
This is an area where we can be better than references. Keep Chat — just document that it maps to Normal on platforms without native chat purpose support. No issue here.
4. Cursor duplication — three representations
IMEState.CursorPos (v1) + IMEState.CursorBegin/CursorEnd (new) + IMEComposition.CursorBegin/CursorEnd (new struct). Bridge method Composition() maps between them with fallback. This is confusing — one cursor concept, three representations.
Suggestion: Since we're pre-v1.0, replace CursorPos with CursorBegin/CursorEnd directly in IMEState. No bridge, no fallback, no duplication.
5. Type alias
type IMEEventSource = IMEEventSourceV2 — our project rule: type aliases only for transitional migration. go doc shows = IMEEventSourceV2 instead of proper documentation.
Suggestion: Remove the alias. Use one name.
6. Missing surrounding text limit
Wayland, GTK4, and winit all enforce a 4000-byte maximum on surrounding text. The PR has no stated limit.
Suggestion: Add MaxSurroundingTextBytes = 4000 constant matching the protocol standard.
What's correct and should stay
- UTF-8 byte offsets — matches Wayland, GTK4, winit consensus
- ContentPurpose enum values — Wayland baseline + Chat extension (better than references)
- ContentHint flags — match Wayland's 10 flags
- IMEComposition struct — clean model for composition state
- IMESurroundingText struct — correct model with cursor/anchor offsets
- Validation logic — UTF-8 boundary checks are enterprise-grade
Recommended path forward
- Update ADR-045 with design decisions that evolved (DIP coordinates, struct-based parameters), then align the PR with the updated ADR
- Extend existing interfaces directly — pre-v1.0, no V2 suffix needed. One clean IME contract
- Discuss capability granularity — winit's 3 groups map to platform boundaries. If 9 bits serve a specific purpose, document the rationale
- Add 4000-byte surrounding text limit — match Wayland/GTK4/winit baseline. Chat purpose is fine (we can be better than references)
- Resolve cursor duplication — one representation, no bridge methods
- Remove type alias — real type or single name
IME is the right feature at the right time. The implementation quality is high. The interface design needs alignment with enterprise patterns and our own ADR — then it's ready.
Add the versioned gpucontext IME contract
Summary
Adds the optional gpucontext IME v2 foundation for gogpu#331. The extension
carries UTF-8 byte-offset composition/surrounding-text ranges, candidate cursor
areas, content purposes/hints, cancellation, disabled/delete-surrounding events,
and capability discovery.
Refs gogpu/gogpu#331 and #19.
Compatibility
The existing
EventSource,IMEState, andIMEControllercontracts remainavailable. New
IMEControllerV2,IMEEventSourceV2, andIMECapabilityProviderV2interfaces are optional and can be implementedindependently; legacy implementors do not need new methods.
Verification
go test ./...go vet ./...staticcheck ./...go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest run --timeout=5mGOOS={windows,darwin,linux,js} ... go test -c