A minor TSF refactoring by lhecker · Pull Request #17067 · microsoft/terminal

Leonard Hecker (lhecker) · 2024-04-16T14:21:27Z

Next in the popular series of minor refactorings:
Out with the old, in with the new!

This PR removes all of the existing TSF code, both for conhost and
Windows Terminal. conhost's TSF implementation was awful:
It allocated an entire text buffer per line of input.
Additionally, its implementation spanned a whopping 40 files and
almost 5000 lines of code. Windows Terminal's implementation was
absolutely fine in comparison, but it was user unfriendly due to
two reasons: Its usage of the CoreTextServices WinRT API indirectly
meant that it used a non-transitory TSF document, which is not the
right choice for a terminal. A TF_SS_TRANSITORY document (-context)
indicates to TSF that it cannot undo a previously completed composition
which is exactly what we need: Once composition has completed we send
the result to the shell and we cannot undo this later on.
The WinRT API does not allow us to use TF_SS_TRANSITORY and so it's
unsuitable for our application. Additionally, the implementation used
XAML to render the composition instead of being part of our text
renderer, which resulted in the text looking weird and hard to read.

The new implementation spans just 8 files and is ~1000 lines which
should make it significantly easier to maintain. The architecture is
not particularly great, but it's certainly better than what we had.
The implementation is almost entirely identical between both conhost
and Windows Terminal and thus they both also behave identical.
It fixes an uncountable number of subtle bugs in the conhost TSF
implementation, as it failed to check for status codes after calls.
It also adds several new features, like support for wavy underlines
(as used by the Japanese IME), dashed underlines (the default for
various languages now, like Vietnamese), colored underlines,
colored foreground/background controlled by the IME, and more!

I have tried to replicate the following issues and have a high
confidence that they're resolved now:
Closes #1304
Closes #3730
Closes #4052
Closes #5007 (as it is not applicable anymore)
Closes #5110
Closes #6186
Closes #6192
Closes #13805
Closes #14349
Closes #14407
Closes #16180

For the following issues I'm not entirely sure if it'll fix it,
but I suspect it's somewhat likely:
#13681
#16305
#16817

Lastly, there's one remaining bug that I don't know how to resolve.
However, that issue also plagues conhost and Windows Terminal
right now, so it's at least not a regression:

Press Win+. (emoji picker) and close it
Move the window around
Press Win+.

This will open the emoji picker at the old window location.
It also occurs when the cursor moves within the window.
While this is super annoying, I could not find a way to fix it.

Validation Steps Performed

See the above closed issues
Use Vietnamese Telex and type "xin choaf"
Results in "xin chào" ✅
Use the MS Japanese IME and press Alt+`
Toggles between the last 2 modes ✅
Use the MS Japanese IME, type "kyouhaishaheiku", and press Space
- The text is converted, underlined and the first part is
  doubly underlined ✅
- Left/Right moves between the 3 segments ✅
- Home/End moves between start/end ✅
- Esc puts a wavy line under the current segment ✅
Use the Korean IME, type "gksgks"
This results in "한한" ✅
Use the Korean IME, type "gks", and press Right Ctrl
Opens a popup which allows you to navigate with Arrow/Tab keys ✅

Mike Griese (zadjii-msft) · 2024-04-16T14:25:48Z

i dunno though, is fixing 11 issues in one PR enough to be considered "minor"? this is a trivial refactoring at best 😝

Michael Niksa (miniksa) · 2024-04-17T00:15:13Z

You're a champion, Leonard Hecker (@lhecker)

Carlos Zamora (carlos-zamora)

Most of the PR is deleted files and deleting references to the deleted stuff, so that's nice.

The most complicated part is src/tsf/Implementation.cpp, but a good amount of the complexity there is that it's a lot of COM.

That said, looks great!

Carlos Zamora (carlos-zamora) · 2024-04-17T18:18:56Z

+                    .columnLimit = buffer.GetRowByOffset(line.top).GetReadableColumnCount(),
+                };
+
+                state.text = text.substr(0, _pData->activeComposition.cursorPos);


Suggested change

.columnLimit = buffer.GetRowByOffset(line.top).GetReadableColumnCount(),

};

state.text = text.substr(0, _pData->activeComposition.cursorPos);

.columnLimit = buffer.GetRowByOffset(line.top).GetReadableColumnCount(),

.text = text.substr(0, _pData->activeComposition.cursorPos),

};

Curious, any particular reason you didn't initialize it here with columnLimit?

I did it so that the two ReplaceText() calls look somewhat symmetric. I felt like it's easier to read/understand that way.

Carlos Zamora (carlos-zamora) · 2024-04-17T18:26:58Z

-    // 3. Paint overlays that reside above the text buffer
-    _PaintOverlays(pEngine);
-
    // 4. Paint Selection


Suggested change

// 4. Paint Selection

// 3. Paint Selection

and update the other numbers below

I'd prefer not changing this code right now, because I'd like to remove it in the near-ish term and changing it now would only disturb the git blame until then IMHO.

Dustin L. Howett (DHowett) · 2024-04-17T21:20:56Z

    }

-    // Free all resources used by this thread
-    DeactivateTextServices();


are there any resources we need to free in the new code?

Ideally, yes. However, this code is currently effectively unreachable, because we TerminateProcess in the ServiceLocator on shutdown. A better location for shutting down TSF is in the destructor of Window but I figured it's fine if we just don't. After all, we also got the delete renderer calls commented out there.

Document the design decisions inherited from Windows Terminal's TSF rewrite by Leonard Hecker (microsoft/terminal#17067). The header now explains why direct TSF was chosen over XAML TextBox IME, the EditSessionProxy pattern, range walking with GUID_PROP_COMPOSING, and callback-driven output architecture. Closes #112 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

When an arrow key is pressed during an active Korean IME composition, _KeyHandler was forwarding the key to the PTY immediately via _TrySendKeyEvent. This moved the cursor before the async TSF edit session had a chance to commit the finalized text, causing the committed character to land at the wrong position. The fix adds a guard that mirrors the existing behavior in the conhost v1 key handler (src/interactivity/win32/windowio.cpp lines 361-364): suppress all key events from being forwarded to the PTY while TSF has an active composition. The IME re-injects navigation and confirmation keys after the composition ends, at which point HasActiveComposition() returns false and they are forwarded normally. Historical note: this guard was not needed before PR microsoft#17067 (v1.22) because the old TSF implementation used WinRT CoreTextServices via XAML (TSFInputControl.xaml/.cpp). In that model the XAML framework handled composition key events before they reached _KeyHandler, so _KeyHandler never received them during composition. The new custom Win32 TSF context introduced in microsoft#17067 no longer intercepts keys at the XAML level, making _KeyHandler responsible for this check. The bug was not visible in v1.22-v1.23 for Korean because the Korean IME was excluded from the TSF path by TF_TMAE_UIELEMENTENABLEDONLY. It became visible in v1.24 when PR microsoft#19738 removed that flag, causing the Korean IME to activate through TSF for the first time. Closes microsoft#20038

Fix a regression introduced in v1.24 where pressing an arrow key during Korean IME composition caused the committed character to be inserted at the wrong cursor position. Before #19738, the Korean IME activated through IMM32 (excluded from TSF by `TF_TMAE_UIELEMENTENABLEDONLY`) and was not affected by `TermControl::_KeyHandler` forwarding keys to the PTY during composition. After #19738, the Korean IME activates through TSF, making a missing composition guard in `_KeyHandler` visible as a bug. The sequence of events that causes the bug: 1. User presses Left arrow during active Korean IME composition (e.g. composing `가`). 2. `_KeyHandler` calls `_TrySendKeyEvent(VK_LEFT)` which enqueues `\x1b[D` to the PTY input queue. The cursor moves. 3. TSF processes the key. The Korean IME sees the arrow and ends the composition. 4. `OnEndComposition` schedules `_doCompositionUpdate` with `TF_ES_ASYNC`. 5. The async session fires, reads finalized text `가`, calls `HandleOutput("가")`. 6. PTY processes `[\x1b[D, "가"]`: cursor moves left first, then `가` is inserted at the wrong (already-moved) position. The fix adds a guard before `_TrySendKeyEvent`, which mirrors the existing behavior in conhost (windowio.cpp). When TSF has an active composition, key events are not converted into input. The Korean IME re-injects navigation and confirmation keys after the composition ends, at which point `HasActiveComposition()` returns false and they are forwarded normally. **Historical note:** This guard was not needed before PR #17067 (v1.22) because the old implementation used WinRT `CoreTextServices` via XAML (`TSFInputControl.xaml`). The XAML framework intercepted composition key events before `_KeyHandler`. The new custom Win32 TSF context in #17067 no longer does this. The bug was latent from v1.22 but only became visible for Korean in v1.24 when #19738 removed `TF_TMAE_UIELEMENTENABLEDONLY`. ## Validation Steps Performed 1. Open Windows Terminal with Korean IME (Dubeolsik layout). 2. Type `rk` to begin composing `가` (composition active, syllable not yet committed). 3. Press the Left arrow key. 4. Before fix: `가` is inserted one cell to the left of the intended position. 5. After fix: `가` is inserted at the correct position, then cursor moves left. Also verified: - Normal Korean text input (typing without arrow keys) still works correctly. - Arrow key navigation when no composition is active still works correctly. - English and other IME input is not affected. Closes #20038 Refs #19738

Fix a regression introduced in v1.24 where pressing an arrow key during Korean IME composition caused the committed character to be inserted at the wrong cursor position. Before #19738, the Korean IME activated through IMM32 (excluded from TSF by `TF_TMAE_UIELEMENTENABLEDONLY`) and was not affected by `TermControl::_KeyHandler` forwarding keys to the PTY during composition. After #19738, the Korean IME activates through TSF, making a missing composition guard in `_KeyHandler` visible as a bug. The sequence of events that causes the bug: 1. User presses Left arrow during active Korean IME composition (e.g. composing `가`). 2. `_KeyHandler` calls `_TrySendKeyEvent(VK_LEFT)` which enqueues `\x1b[D` to the PTY input queue. The cursor moves. 3. TSF processes the key. The Korean IME sees the arrow and ends the composition. 4. `OnEndComposition` schedules `_doCompositionUpdate` with `TF_ES_ASYNC`. 5. The async session fires, reads finalized text `가`, calls `HandleOutput("가")`. 6. PTY processes `[\x1b[D, "가"]`: cursor moves left first, then `가` is inserted at the wrong (already-moved) position. The fix adds a guard before `_TrySendKeyEvent`, which mirrors the existing behavior in conhost (windowio.cpp). When TSF has an active composition, key events are not converted into input. The Korean IME re-injects navigation and confirmation keys after the composition ends, at which point `HasActiveComposition()` returns false and they are forwarded normally. **Historical note:** This guard was not needed before PR #17067 (v1.22) because the old implementation used WinRT `CoreTextServices` via XAML (`TSFInputControl.xaml`). The XAML framework intercepted composition key events before `_KeyHandler`. The new custom Win32 TSF context in #17067 no longer does this. The bug was latent from v1.22 but only became visible for Korean in v1.24 when #19738 removed `TF_TMAE_UIELEMENTENABLEDONLY`. ## Validation Steps Performed 1. Open Windows Terminal with Korean IME (Dubeolsik layout). 2. Type `rk` to begin composing `가` (composition active, syllable not yet committed). 3. Press the Left arrow key. 4. Before fix: `가` is inserted one cell to the left of the intended position. 5. After fix: `가` is inserted at the correct position, then cursor moves left. Also verified: - Normal Korean text input (typing without arrow keys) still works correctly. - Arrow key navigation when no composition is active still works correctly. - English and other IME input is not affected. Closes #20038 Refs #19738 (cherry picked from commit 14f4271) Service-Card-Id: PVTI_lADOAF3p4s4BQX0-zgo0JpM Service-Version: 1.25

Fix a regression introduced in v1.24 where pressing an arrow key during Korean IME composition caused the committed character to be inserted at the wrong cursor position. Before #19738, the Korean IME activated through IMM32 (excluded from TSF by `TF_TMAE_UIELEMENTENABLEDONLY`) and was not affected by `TermControl::_KeyHandler` forwarding keys to the PTY during composition. After #19738, the Korean IME activates through TSF, making a missing composition guard in `_KeyHandler` visible as a bug. The sequence of events that causes the bug: 1. User presses Left arrow during active Korean IME composition (e.g. composing `가`). 2. `_KeyHandler` calls `_TrySendKeyEvent(VK_LEFT)` which enqueues `\x1b[D` to the PTY input queue. The cursor moves. 3. TSF processes the key. The Korean IME sees the arrow and ends the composition. 4. `OnEndComposition` schedules `_doCompositionUpdate` with `TF_ES_ASYNC`. 5. The async session fires, reads finalized text `가`, calls `HandleOutput("가")`. 6. PTY processes `[\x1b[D, "가"]`: cursor moves left first, then `가` is inserted at the wrong (already-moved) position. The fix adds a guard before `_TrySendKeyEvent`, which mirrors the existing behavior in conhost (windowio.cpp). When TSF has an active composition, key events are not converted into input. The Korean IME re-injects navigation and confirmation keys after the composition ends, at which point `HasActiveComposition()` returns false and they are forwarded normally. **Historical note:** This guard was not needed before PR #17067 (v1.22) because the old implementation used WinRT `CoreTextServices` via XAML (`TSFInputControl.xaml`). The XAML framework intercepted composition key events before `_KeyHandler`. The new custom Win32 TSF context in #17067 no longer does this. The bug was latent from v1.22 but only became visible for Korean in v1.24 when #19738 removed `TF_TMAE_UIELEMENTENABLEDONLY`. ## Validation Steps Performed 1. Open Windows Terminal with Korean IME (Dubeolsik layout). 2. Type `rk` to begin composing `가` (composition active, syllable not yet committed). 3. Press the Left arrow key. 4. Before fix: `가` is inserted one cell to the left of the intended position. 5. After fix: `가` is inserted at the correct position, then cursor moves left. Also verified: - Normal Korean text input (typing without arrow keys) still works correctly. - Arrow key navigation when no composition is active still works correctly. - English and other IME input is not affected. Closes #20038 Refs #19738 (cherry picked from commit 14f4271) Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgo0JpQ Service-Version: 1.24

A minor TSF refactoring

02ad1c6

Leonard Hecker (lhecker) added 3 commits April 16, 2024 16:41

Fix build failure, Improve FindWindowOfActiveTSF

c35f388

Fix build, Prevent flickering

f1ad606

More fixes

4125cc2

Use the color at the point of insertion

f3e5054

Carlos Zamora (carlos-zamora) approved these changes Apr 17, 2024

View reviewed changes

Partially address feedback

771bbc6

Dustin L. Howett (DHowett) reviewed Apr 17, 2024

View reviewed changes

Address feedback

fee7a54

Dustin L. Howett (DHowett) approved these changes Apr 18, 2024

View reviewed changes

Leonard Hecker (lhecker) added this pull request to the merge queue Apr 18, 2024

Merged via the queue into main with commit 4e7b63c Apr 18, 2024

Leonard Hecker (lhecker) deleted the dev/lhecker/8000-conime branch April 18, 2024 18:18

Mike Griese (zadjii-msft) mentioned this pull request Apr 19, 2024

[1.21.1091 Canary] Terminal crashes on launch #17089

Closed

James Holderness (j4james) mentioned this pull request May 28, 2024

Alt + numpad combination does not work anymore #17327

Closed

Mike Griese (zadjii-msft) mentioned this pull request Aug 29, 2024

Has the automatic switching input method function been canceled? #17816

Closed

Leonard Hecker (lhecker) mentioned this pull request Jul 9, 2025

the first key pressing is sometimes ignored when writing with Japanese IME. #11521

Closed

加村有司 (YuujiKamura) mentioned this pull request Mar 17, 2026

TSF実装の帰属表記 — Windows Terminal PR #17067 由来の設計判断を明記 YuujiKamura/ghostty#112

Closed

jason (drvoss) mentioned this pull request Mar 31, 2026

Korean IME (v1.24+): Arrow key during composition inserts character at wrong position #20038

Closed

jason (drvoss) mentioned this pull request Mar 31, 2026

Fix Korean IME arrow key inserting character at wrong position #20039

Merged

1 of 4 tasks

kazuki (i999rri) mentioned this pull request Apr 6, 2026

WinUI 3: IME input not working (Japanese/Chinese/Korean) / WinUI 3でIME入力が動かない i999rri/GhosttyWin32#16

Closed

watanabe (wakamenod) mentioned this pull request Nov 7, 2026

Windowing without winit: ADR 0070 and stage 1 wakamenod/oso#51

Merged

Conversation

Leonard Hecker (lhecker) commented Apr 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Validation Steps Performed

Uh oh!

Mike Griese (zadjii-msft) commented Apr 16, 2024

Uh oh!

Michael Niksa (miniksa) commented Apr 17, 2024

Uh oh!

Carlos Zamora (carlos-zamora) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Carlos Zamora (carlos-zamora) Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Leonard Hecker (lhecker) Apr 18, 2024

Choose a reason for hiding this comment

Uh oh!

Carlos Zamora (carlos-zamora) Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Carlos Zamora (carlos-zamora) Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Leonard Hecker (lhecker) Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dustin L. Howett (DHowett) Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Leonard Hecker (lhecker) Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Leonard Hecker (lhecker) commented Apr 16, 2024 •

edited

Loading

Leonard Hecker (lhecker) Apr 18, 2024 •

edited

Loading