Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issues with Japanese & Vietnamese IME #13678

Merged
1 commit merged into from
Aug 11, 2022
Merged

Fix issues with Japanese & Vietnamese IME #13678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/actions/spelling/expect/expect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ antialias
antialiasing
ANull
anycpu
AOn
APARTMENTTHREADED
APCs
api
Expand Down Expand Up @@ -80,7 +79,6 @@ ASingle
asm
asmv
asmx
AStomps
ASYNCWINDOWPOS
atch
ATest
Expand Down Expand Up @@ -225,14 +223,14 @@ CFuzz
cgscrn
chafa
changelist
chaof
charinfo
charset
CHARSETINFO
chcp
checkbox
checkboxes
chh
Childitem
chk
chrono
CHT
Expand Down Expand Up @@ -2756,6 +2754,9 @@ xes
xff
XFile
XFORM
xin
xinchaof
xinxinchaof
XManifest
XMath
XMFLOAT
Expand Down
4 changes: 1 addition & 3 deletions src/cascadia/TerminalControl/TSFInputControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_inputBuffer.clear();
_selection = {};
_activeTextStart = 0;
_editContext.NotifyFocusLeave();
_editContext.NotifyTextChanged({ 0, INT32_MAX }, 0, _selection);
_editContext.NotifyFocusEnter();
TextBlock().Text({});
}
}
Expand Down Expand Up @@ -375,7 +373,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
incomingText);
_selection = args.NewSelection();
// GH#5054: Pressing backspace might move the caret before the _activeTextStart.
_activeTextStart = ::base::ClampMin(_activeTextStart, ::base::ClampedNumeric<size_t>(range.StartCaretPosition));
_activeTextStart = std::min(_activeTextStart, _inputBuffer.size());

// Emojis/Kaomojis/Symbols chosen through the IME without starting composition
// will be sent straight through to the terminal.
Expand Down
19 changes: 13 additions & 6 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return;
}

// GH#11479: TSF wants to be notified of any character input via ICoreTextEditContext::NotifyTextChanged().
// TSF is built and tested around the idea that you inform it of any text changes that happen
// when it doesn't currently compose characters. For instance writing "xin chaof" with the
// Vietnamese IME should produce "xin chào". After writing "xin" it'll emit a composition
// completion event and we'll write "xin" to the shell. It now has no input focus and won't know
// about the whitespace. If you then write "chaof", it'll emit another composition completion
// event for "xinchaof" and the resulting output in the shell will finally read "xinxinchaof".
// A composition completion event technically doesn't mean that the completed text is now
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this one is correct. IME can change the text in the edit control as it sees fit, you can think of something like autocorrection.
So with the current implementation (before this PR), IME will see a big lump of text without any word delimitation (spaces, dot, comma ...) as they're not notified back to TSF, limiting the ability to provide more advanced text edit functionality that IME can provide.

I have a question regarding the text edit field, is the current command line where the user can input text is not considered a text edit field? And only the composition is considered as a text edit field and has text editing events notified back to TSF?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question regarding the text edit field, is the current command line where the user can input text is not considered a text edit field?

This part is complicated. Right now, we are using an "overlay" text field because we have not connected the command line buffer to the TSF. This is because of how Terminal sees the screen:

0 1 2 3 4 5 6
C : \ > H i .

All of the input and output goes into the same character grid. Terminal does not know that 3 is part of the prompt, and 4 is part of the user input! If we naively sent all that data to TSF, it would look like the user could edit the C:\> part.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the input and output goes into the same character grid. Terminal does not know that 3 is part of the prompt, and 4 is part of the user input! If we naively sent all that data to TSF, it would look like the user could edit the C:\> part.

I see, thank you.
On another hand, I think that we can probably just pass that to the TSF, since the text modification should be depended on the text cursor position as well. And even if the TSF's input processors think it can change the text, if the application doesn't change the text then the input processors should follow.

// immutable after all. We could (and probably should) inform TSF of any input changes,
// but we technically aren't a text input field. The immediate solution was
// to simply force TSF to clear its text whenever we have input focus.
TSFInputControl().ClearBuffer();

_HidePointerCursorHandlers(*this, nullptr);

const auto ch = e.Character();
Expand Down Expand Up @@ -1182,12 +1195,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
const auto window = CoreWindow::GetForCurrentThread();

if (vkey == VK_ESCAPE ||
vkey == VK_RETURN)
{
TSFInputControl().ClearBuffer();
}

// If the terminal translated the key, mark the event as handled.
// This will prevent the system from trying to get the character out
// of it and sending us a CharacterReceived event.
Expand Down