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

Attempt fixing repetitive chars in Japanese IME #15938

Closed
wants to merge 1 commit into from
Closed

Attempt fixing repetitive chars in Japanese IME #15938

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
41 changes: 24 additions & 17 deletions src/cascadia/TerminalControl/TSFInputControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,15 +310,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - <none>
void TSFInputControl::_textRequestedHandler(CoreTextEditContext sender, const CoreTextTextRequestedEventArgs& args)
{
try
{
const auto range = args.Request().Range();
const auto text = _inputBuffer.substr(
range.StartCaretPosition,
range.EndCaretPosition - range.StartCaretPosition);
args.Request().Text(text);
}
CATCH_LOG();
const auto request = args.Request();
const auto range = request.Range();
const auto [beg, len] = _convertRangeIntoBeginAndLength(range);
const auto text = std::wstring_view{ _inputBuffer }.substr(beg, len);
request.Text(text);
}

// Method Description:
Expand Down Expand Up @@ -366,14 +362,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
const auto incomingText = args.Text();
const auto range = args.Range();

_inputBuffer = _inputBuffer.replace(
range.StartCaretPosition,
range.EndCaretPosition - range.StartCaretPosition,
incomingText);
const auto [beg, len] = _convertRangeIntoBeginAndLength(range);
_inputBuffer.replace(beg, len, incomingText);
Copy link
Member Author

Choose a reason for hiding this comment

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

replace() returns a reference to *this, so assigning it to itself is not necessary.
(https://en.cppreference.com/w/cpp/string/basic_string/replace)

_selection = args.NewSelection();
// GH#5054: Pressing backspace might move the caret before the _activeTextStart.
_activeTextStart = std::min(_activeTextStart, _inputBuffer.size());
_activeTextStart = std::min(_activeTextStart, beg);
Copy link
Member Author

Choose a reason for hiding this comment

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

This reverts part of ed800dc. It still seems to work.


// Emojis/Kaomojis/Symbols chosen through the IME without starting composition
// will be sent straight through to the terminal.
Expand All @@ -384,7 +377,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
else
{
Canvas().Visibility(Visibility::Visible);
const auto text = _inputBuffer.substr(_activeTextStart);
const auto text = std::wstring_view{ _inputBuffer }.substr(_activeTextStart);
TextBlock().Text(text);
}

Expand All @@ -400,9 +393,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
}

// CoreTextRange uses int32 start/end indices whereas _inputBuffer uses unsigned size_t
// for a start index and length. This method safely converts the former to the latter,
// while making sure that out-of-range indices are clamped to the size of _inputBuffer.
std::pair<size_t, size_t> TSFInputControl::_convertRangeIntoBeginAndLength(CoreTextRange range) const noexcept
{
auto beg = gsl::narrow_cast<size_t>(std::max(0, range.StartCaretPosition));
auto end = gsl::narrow_cast<size_t>(std::max(0, range.EndCaretPosition));

end = std::min(end, _inputBuffer.size());
beg = std::min(beg, end);

return { beg, end - beg };
}
Comment on lines +399 to +408
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI the ::base::ClampMin, etc., code in this class prior to 2c922e1 was not entirely correct either. For instance, it would pass range.StartCaretPosition to replace() directly.

Code like this:

const auto textEnd = ::base::ClampMin<size_t>(range.EndCaretPosition, _inputBuffer.length());
const auto length = ::base::ClampSub<size_t>(textEnd, range.StartCaretPosition);

is also not correct, since the T in ::base::ClampSub<T> acts just like the T in std::min<T>: It casts all parameters to <T> and then clamps the final value. That is, if you do ClampSub<size_t>(123, -1) it is the same as ClampSub(size_t(123), size_t(-1)) which is 18446744073709551615. In other words, a negative StartCaretPosition will result in length == SIZE_T_MAX (substr() doesn't really care, but... eh).


void TSFInputControl::_SendAndClearText()
{
const auto text = _inputBuffer.substr(_activeTextStart);
const auto text = std::wstring_view{ _inputBuffer }.substr(_activeTextStart);
if (text.empty())
{
return;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/TSFInputControl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _textUpdatingHandler(winrt::Windows::UI::Text::Core::CoreTextEditContext sender, const winrt::Windows::UI::Text::Core::CoreTextTextUpdatingEventArgs& args);
void _formatUpdatingHandler(winrt::Windows::UI::Text::Core::CoreTextEditContext sender, const winrt::Windows::UI::Text::Core::CoreTextFormatUpdatingEventArgs& args);

std::pair<size_t, size_t> _convertRangeIntoBeginAndLength(winrt::Windows::UI::Text::Core::CoreTextRange range) const noexcept;
void _SendAndClearText();
void _RedrawCanvas();

Expand Down