Added support for IME input for languages such as Chinese and Japanese. #90
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kylecarbs
approved these changes
Dec 4, 2025
Member
kylecarbs
left a comment
kylecarbs
left a comment
There was a problem hiding this comment.
Very cool. This looks good to me!
There was a problem hiding this comment.
Pull request overview
This PR implements Input Method Editor (IME) support for Chinese, Japanese, and other languages that require composition-based input. The implementation adds event listeners for composition events and prevents duplicate input during the composition process.
Key Changes:
- Added composition event handling (compositionstart, compositionupdate, compositionend) to track IME input state
- Modified keydown handler to ignore keyboard events during composition to prevent duplicate input
- Implemented text node cleanup after composition to prevent visual duplicates in the terminal
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| lib/input-handler.ts | Added composition event listeners and handlers, modified keydown to ignore events during composition, implemented text node cleanup logic |
| lib/input-handler.test.ts | Added comprehensive IME composition tests covering composition sequences, keydown ignoring, and text node cleanup; updated mock infrastructure to support composition events |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/input-handler.test.ts
Outdated
Comment on lines
97
to
99
| preventDefault: mock(() => { }), | ||
| stopPropagation: mock(() => { }), | ||
| }; |
There was a problem hiding this comment.
[nitpick] Inconsistent formatting: same empty arrow function formatting issue { } vs {}. This should match the project's established style.
Comment on lines
+306
to
+402
| describe('IME Composition', () => { | ||
| test('handles composition sequence', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Start composition | ||
| const startEvent = createCompositionEvent('compositionstart', ''); | ||
| container.dispatchEvent(startEvent); | ||
|
|
||
| // Update composition (typing) | ||
| const updateEvent1 = createCompositionEvent('compositionupdate', 'n'); | ||
| container.dispatchEvent(updateEvent1); | ||
|
|
||
| // Keydown events during composition should be ignored | ||
| const keyEvent1 = createKeyEvent('KeyN', 'n'); | ||
| Object.defineProperty(keyEvent1, 'isComposing', { value: true }); | ||
| simulateKey(container, keyEvent1); | ||
|
|
||
| // Update composition (more typing) | ||
| const updateEvent2 = createCompositionEvent('compositionupdate', 'ni'); | ||
| container.dispatchEvent(updateEvent2); | ||
|
|
||
| // End composition (commit) | ||
| const endEvent = createCompositionEvent('compositionend', '你好'); | ||
| container.dispatchEvent(endEvent); | ||
|
|
||
| // Should only receive the final committed text | ||
| expect(dataReceived).toEqual(['你好']); | ||
| }); | ||
|
|
||
| test('ignores keydown during composition', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Start composition | ||
| container.dispatchEvent(createCompositionEvent('compositionstart', '')); | ||
|
|
||
| // Simulate keydown with isComposing=true | ||
| const keyEvent = createKeyEvent('KeyA', 'a'); | ||
| Object.defineProperty(keyEvent, 'isComposing', { value: true }); | ||
| simulateKey(container, keyEvent); | ||
|
|
||
| // Simulate keydown with keyCode 229 | ||
| const keyEvent229 = createKeyEvent('KeyB', 'b'); | ||
| Object.defineProperty(keyEvent229, 'keyCode', { value: 229 }); | ||
| simulateKey(container, keyEvent229); | ||
|
|
||
| // Should not receive any data | ||
| expect(dataReceived.length).toBe(0); | ||
|
|
||
| // End composition | ||
| container.dispatchEvent(createCompositionEvent('compositionend', 'a')); | ||
| expect(dataReceived).toEqual(['a']); | ||
| }); | ||
|
|
||
| test('cleans up text nodes in container after composition', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Simulate browser inserting text node during composition | ||
| const textNode = { nodeType: 3, textContent: '你好' } as Node; | ||
| container.appendChild(textNode); | ||
|
|
||
| // Also add a non-text node (e.g. canvas) to ensure it's not removed | ||
| const elementNode = { nodeType: 1, nodeName: 'CANVAS' } as Node; | ||
| container.appendChild(elementNode); | ||
|
|
||
| expect(container.childNodes.length).toBe(2); | ||
|
|
||
| // End composition | ||
| const endEvent = createCompositionEvent('compositionend', '你好'); | ||
| container.dispatchEvent(endEvent); | ||
|
|
||
| // Should have removed the text node but kept the element node | ||
| expect(container.childNodes.length).toBe(1); | ||
| expect(container.childNodes[0]).toBe(elementNode); | ||
| expect(dataReceived).toEqual(['你好']); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The existing dispose removes listeners and marks inactive test should be updated to verify that the new composition event listeners (compositionstart, compositionupdate, compositionend) are also properly removed during disposal. This would ensure complete cleanup of all event listeners.
Comment on lines
+306
to
+402
| describe('IME Composition', () => { | ||
| test('handles composition sequence', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Start composition | ||
| const startEvent = createCompositionEvent('compositionstart', ''); | ||
| container.dispatchEvent(startEvent); | ||
|
|
||
| // Update composition (typing) | ||
| const updateEvent1 = createCompositionEvent('compositionupdate', 'n'); | ||
| container.dispatchEvent(updateEvent1); | ||
|
|
||
| // Keydown events during composition should be ignored | ||
| const keyEvent1 = createKeyEvent('KeyN', 'n'); | ||
| Object.defineProperty(keyEvent1, 'isComposing', { value: true }); | ||
| simulateKey(container, keyEvent1); | ||
|
|
||
| // Update composition (more typing) | ||
| const updateEvent2 = createCompositionEvent('compositionupdate', 'ni'); | ||
| container.dispatchEvent(updateEvent2); | ||
|
|
||
| // End composition (commit) | ||
| const endEvent = createCompositionEvent('compositionend', '你好'); | ||
| container.dispatchEvent(endEvent); | ||
|
|
||
| // Should only receive the final committed text | ||
| expect(dataReceived).toEqual(['你好']); | ||
| }); | ||
|
|
||
| test('ignores keydown during composition', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Start composition | ||
| container.dispatchEvent(createCompositionEvent('compositionstart', '')); | ||
|
|
||
| // Simulate keydown with isComposing=true | ||
| const keyEvent = createKeyEvent('KeyA', 'a'); | ||
| Object.defineProperty(keyEvent, 'isComposing', { value: true }); | ||
| simulateKey(container, keyEvent); | ||
|
|
||
| // Simulate keydown with keyCode 229 | ||
| const keyEvent229 = createKeyEvent('KeyB', 'b'); | ||
| Object.defineProperty(keyEvent229, 'keyCode', { value: 229 }); | ||
| simulateKey(container, keyEvent229); | ||
|
|
||
| // Should not receive any data | ||
| expect(dataReceived.length).toBe(0); | ||
|
|
||
| // End composition | ||
| container.dispatchEvent(createCompositionEvent('compositionend', 'a')); | ||
| expect(dataReceived).toEqual(['a']); | ||
| }); | ||
|
|
||
| test('cleans up text nodes in container after composition', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Simulate browser inserting text node during composition | ||
| const textNode = { nodeType: 3, textContent: '你好' } as Node; | ||
| container.appendChild(textNode); | ||
|
|
||
| // Also add a non-text node (e.g. canvas) to ensure it's not removed | ||
| const elementNode = { nodeType: 1, nodeName: 'CANVAS' } as Node; | ||
| container.appendChild(elementNode); | ||
|
|
||
| expect(container.childNodes.length).toBe(2); | ||
|
|
||
| // End composition | ||
| const endEvent = createCompositionEvent('compositionend', '你好'); | ||
| container.dispatchEvent(endEvent); | ||
|
|
||
| // Should have removed the text node but kept the element node | ||
| expect(container.childNodes.length).toBe(1); | ||
| expect(container.childNodes[0]).toBe(elementNode); | ||
| expect(dataReceived).toEqual(['你好']); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Edge case not tested: What happens if composition is interrupted (e.g., user clicks away or presses Escape)? The isComposing flag would remain true if compositionend is never fired. Consider adding a test for this scenario or adding a mechanism to reset the composition state when the container loses focus or when dispose is called.
Comment on lines
+306
to
+402
| describe('IME Composition', () => { | ||
| test('handles composition sequence', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Start composition | ||
| const startEvent = createCompositionEvent('compositionstart', ''); | ||
| container.dispatchEvent(startEvent); | ||
|
|
||
| // Update composition (typing) | ||
| const updateEvent1 = createCompositionEvent('compositionupdate', 'n'); | ||
| container.dispatchEvent(updateEvent1); | ||
|
|
||
| // Keydown events during composition should be ignored | ||
| const keyEvent1 = createKeyEvent('KeyN', 'n'); | ||
| Object.defineProperty(keyEvent1, 'isComposing', { value: true }); | ||
| simulateKey(container, keyEvent1); | ||
|
|
||
| // Update composition (more typing) | ||
| const updateEvent2 = createCompositionEvent('compositionupdate', 'ni'); | ||
| container.dispatchEvent(updateEvent2); | ||
|
|
||
| // End composition (commit) | ||
| const endEvent = createCompositionEvent('compositionend', '你好'); | ||
| container.dispatchEvent(endEvent); | ||
|
|
||
| // Should only receive the final committed text | ||
| expect(dataReceived).toEqual(['你好']); | ||
| }); | ||
|
|
||
| test('ignores keydown during composition', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Start composition | ||
| container.dispatchEvent(createCompositionEvent('compositionstart', '')); | ||
|
|
||
| // Simulate keydown with isComposing=true | ||
| const keyEvent = createKeyEvent('KeyA', 'a'); | ||
| Object.defineProperty(keyEvent, 'isComposing', { value: true }); | ||
| simulateKey(container, keyEvent); | ||
|
|
||
| // Simulate keydown with keyCode 229 | ||
| const keyEvent229 = createKeyEvent('KeyB', 'b'); | ||
| Object.defineProperty(keyEvent229, 'keyCode', { value: 229 }); | ||
| simulateKey(container, keyEvent229); | ||
|
|
||
| // Should not receive any data | ||
| expect(dataReceived.length).toBe(0); | ||
|
|
||
| // End composition | ||
| container.dispatchEvent(createCompositionEvent('compositionend', 'a')); | ||
| expect(dataReceived).toEqual(['a']); | ||
| }); | ||
|
|
||
| test('cleans up text nodes in container after composition', () => { | ||
| const handler = new InputHandler( | ||
| ghostty, | ||
| container as any, | ||
| (data) => dataReceived.push(data), | ||
| () => { | ||
| bellCalled = true; | ||
| } | ||
| ); | ||
|
|
||
| // Simulate browser inserting text node during composition | ||
| const textNode = { nodeType: 3, textContent: '你好' } as Node; | ||
| container.appendChild(textNode); | ||
|
|
||
| // Also add a non-text node (e.g. canvas) to ensure it's not removed | ||
| const elementNode = { nodeType: 1, nodeName: 'CANVAS' } as Node; | ||
| container.appendChild(elementNode); | ||
|
|
||
| expect(container.childNodes.length).toBe(2); | ||
|
|
||
| // End composition | ||
| const endEvent = createCompositionEvent('compositionend', '你好'); | ||
| container.dispatchEvent(endEvent); | ||
|
|
||
| // Should have removed the text node but kept the element node | ||
| expect(container.childNodes.length).toBe(1); | ||
| expect(container.childNodes[0]).toBe(elementNode); | ||
| expect(dataReceived).toEqual(['你好']); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage: The test suite doesn't cover the case where compositionend fires with empty or null data (e.g., when composition is cancelled). The implementation checks if (data && data.length > 0) before calling the callback, but this path isn't tested. Add a test to verify that cancelling composition doesn't send any data to the terminal.
lib/input-handler.ts
Outdated
Comment on lines
247
to
250
| this.compositionUpdateListener = this.handleCompositionUpdate.bind(this); | ||
| this.container.addEventListener( | ||
| 'compositionupdate', | ||
| this.compositionUpdateListener |
There was a problem hiding this comment.
[nitpick] The compositionupdate event handler is registered but doesn't perform any operations beyond the disposal check. Consider whether this event listener is necessary, or document why it's included for future extensibility. If it's kept for potential future use, the comment on lines 537-539 should be more explicit about this being a placeholder for future enhancements.
Comment on lines
+312
to
+313
| // Ignore keydown events during composition | ||
| // Note: Some browsers send keyCode 229 for all keys during composition |
There was a problem hiding this comment.
[nitpick] The condition checks three separate ways to detect composition: this.isComposing (internal state), event.isComposing (standard property), and event.keyCode === 229 (legacy approach). While this provides good cross-browser compatibility, the comment should clarify that keyCode 229 is a legacy fallback for older browsers, as keyCode is deprecated. Consider if all three checks are still necessary for the target browsers.
Suggested change
| // Ignore keydown events during composition | |
| // Note: Some browsers send keyCode 229 for all keys during composition | |
| // Ignore keydown events during composition. | |
| // We check three ways for cross-browser compatibility: | |
| // - this.isComposing: internal state set by composition events | |
| // - event.isComposing: standard property supported by modern browsers | |
| // - event.keyCode === 229: legacy fallback for older browsers (e.g., IE/Edge), as keyCode is deprecated |
|
|
||
| describe('IME Composition', () => { | ||
| test('handles composition sequence', () => { | ||
| const handler = new InputHandler( |
There was a problem hiding this comment.
Unused variable handler.
Suggested change
| const handler = new InputHandler( | |
| new InputHandler( |
| }); | ||
|
|
||
| test('ignores keydown during composition', () => { | ||
| const handler = new InputHandler( |
There was a problem hiding this comment.
Unused variable handler.
Member
|
@Leask fmt is failing but the rest looks good to me. Fix that up and I'll merge! |
Contributor
Author
|
@kylecarbs fixed! 😄 Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initial support for Chinese, Japanese, and other input methods has been implemented. This is a preliminary version that may require further optimization, but it stably achieves the target functionality without negative impacts.
Compared to before the modification, input was not possible:
This is the revised effect, which meets expectations: