<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
This PR will allow TSFInputControl to redraw its Canvas and TextBlock in response to when the Terminal cursor position updates. This will fix the issue where during Korean composition, the first symbol of the next composition will appear on top of the previous composed character. Since the Terminal Cursor updates a lot, I've added some checks to see if the TSFInputControl really needs to redraw. This will also decrease the number of actual redraws since we receive a bunch of `LayoutRequested` events when there's no difference between them.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#4963
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Startup, teardown, CJK IME gibberish testing, making sure the IME block shows up in the right place.
This PR has evolved to encapsulate two related fixes that I can't really
untie anymore.
#2455 - Duplicating a tab that doesn't exist anymore
This was the bug I was originally fixing in #4429.
When the user tries to `duplicateTab` with a profile that doesn't exist
anymore (like might happen after a settings reload), don't crash.
As I was going about adding tests for this, got blocked by the fact that
the Terminal couldn't open _any_ panes while the `TerminalPage` was size
0x0. This had two theoretical solutions:
* Fake the `TerminalPage` into thinking it had a real size in the test -
probably possible, though I'm unsure how it would work in practice.
* Change `Pane`s to not require an `ActualWidth`, `ActualHeight` on
initialization.
Fortuately, the second option was something else that was already on my
backlog of bugs.
#4618 - `wt` command-line can't consistently parse more than one arg
Presently, the Terminal just arbitrarily dispatches a bunch of handlers
to try and handle all the commands provided on the commandline. That's
lead to a bunch of reports that not all the commands will always get
executed, nor will they all get executed in the same order.
This PR also changes the `TerminalPage` to be able to dispatch all the
commands sequentially, all at once in the startup. No longer will there
be a hot second where the commands seem to execute themselves in from of
the user - they'll all happen behind the scenes on startup.
This involved a couple other changes areound the `TerminalPage`
* I had to make sure that panes could be opened at a 0x0 size. Now they
use a star sizing based off the percentage of the parent they're
supposed to consume, so that when the parent _does_ get laid out,
they'll take the appropriate size of that parent.
* I had to do some math ahead of time to try and calculate what a
`SplitState::Automatic` would be evaluated as, despite the fact that
we don't actually know how big the pane will be.
* I had to ensure that `focus-tab` commands appropriately mark a single
tab as focused while we're in startup, without roundtripping to the
Dispatcher thread and back
## References
#4429 - the original PR for #2455#5047 - a follow-up task from discussion in #4429#4953 - a PR for making panes use star sizing, which was immensly
helpful for this PR.
## Detailed Description of the Pull Request / Additional comments
`CascadiaSettings::BuildSettings` can throw if the GUID doesn't exist.
This wraps those calls up with a try/catch.
It also adds a couple tests - a few `SettingsTests` for try/catching
this state. It also adds a XAML-y test in `TabTests` that creates a
`TerminalPage` and then performs som UI-like actions on it. This test
required a minor change to how we generate the new tab dropdown - in the
tests, `Application::Current()` is _not_ a `TerminalApp::App`, so it
doesn't have a `Logic()` to query. So wrap that in a try/catch as well.
While working on these tests, I found that we'd crash pretty agressively
for mysterious reasons if the TestHostApp became focused while the test
was running. This was due to a call in
`TSFInputControl::NotifyFocusEnter` that would callback to
`TSFInputControl::_layoutRequested`, which would crash on setting the
`MaxSize` of the canvas to a negative value. This PR includes a hotfix
for that bug as well.
## Validation Steps Performed
* Manual testing with a _lot_ of commands in a commandline
* run the tests
* Team tested in selfhost
Closes#2455Closes#4618
## Summary of the Pull Request
This PR fixes an out of bounds access when deleting composition during Chinese IME. What's happening is that we're receiving a CompositionCompleted before receiving the TextUpdate to tell us to delete the last character in the composition. This creates two problems for us:
1. The final character gets sent to the Terminal when it should have been deleted.
2. `_activeTextStart` gets set to `_inputBuffer.length()` when sending the character to the terminal, so when the `TextUpdate` comes right after the `CompositionCompleted` event, `_activeTextStart` is out of sync.
This PR fixes the second issue, by updating `_activeTextStart` during a `TextUpdate` in case we run into this issue.
The first issue is trickier to resolve since we assume that if the text server api tells us a composition is completed, we should send what we have. It'll be tracked here: #5110.
At the very least, this PR will let users continue to type in Chinese IME without it breaking, but it will still be annoying to see the first letter of your composition reappear after deleting it.
## PR Checklist
* [x] Closes#5054
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Play around with Chinese IME deleting and composing, and play around with Korean and Japanese IME to see that it still works as expected.
## Summary of the Pull Request
This PR turns on TextWrapping on `TSFInputControl::TextBlock`. Once the TextBlock hits the end of the Terminal window, it will wrap downwards, but the TextBlock will have as much width as it had when composition started. Unfortunately, this means if composition starts right at the end of the Terminal with enough width for just one character, there will be a vertical line of characters down the right side of the Terminal 😅. It's definitely not ideal, and I imagine this won't be the last time we visit this issue, but for now users can see what they're typing.
## PR Checklist
* [x] Closes#3657
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Played around with IME towards the edge of the Terminal window.
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
I originally thought that setting `TSFInputControl::_editContext.InputPaneDisplayPolicy` to be Automatic would allow the InputPanel to show and hide automatically when `TSFInputControl` gains and loses focus. It doesn't seem to behave that way, so we'll show the InputPanel manually.
I'll show the panel during `PointerPressedHandler` and during `GotFocusHandler`. A user will have the on-screen keyboard pop up when getting focus, but if they close the keyboard, they can simply re-tap on the terminal to bring it back up.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#3639
* [x] CLA signed.
* [x] Tests added/passed
## Validation Steps Performed
Played on my surfaces book with the on screen keyboard by closing/tapping on the terminal and using the search box.
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Emoji composition was only being shown one letter at a time. This is because of the way I expected `CoreTextTextUpdatingEventArgs.Range` to be provided to TSFInputControl during composition (for Chinese/Japanese IME). Emoji IME composition gives the `StartCaretPosition` as the same as `EndCaretPosition`, unlike how for Chinese/Japanese IME, `StartCaretPosition` is usually the start of the composition and `EndCaretPosition` is the latest character in the composition. The solution is to change the `_inputBuffer.substr()` call to simply grab all of the buffer starting from `_activeTextStart`. This way I can ensure that I grab all of the "active text", instead of trusting the given `args.Range` to tell me the active text.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#4828
* [x] CLA signed.
* [x] Tests added/passed
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Chinese, Japanese, Korean, Emoji composition performed. Emoji selection through pointer also performed.
## Summary of the Pull Request
Korean IME was not working correctly due to way we were clearing the input buffer inside of `TSFInputControl`. We wanted to clear our input buffer and tell TSF to clear its input buffer as well when we receive a `CompositionCompleted` event. This works fine in some IME languages such as Chinese and Japanese. However, Korean IME composes characters differently in such a way where we can't tell TSF to clear their buffer during a `CompositionCompleted` event because it would clear the character that triggered the `CompositionCompleted` event in the first place.
The solution in this PR is to keep our `_inputBuffer` intact until the user presses <kbd>Enter</kbd> or <kbd>Esc</kbd>, in which case we clear our buffer and the TSF buffer. I've chosen these two keys because it seems to make sense to clear the buffer after text is sent to the terminal with <kbd>Enter</kbd>, and <kbd>Esc</kbd> usually means to cancel a current composition anyway.
This means we need to keep track of our last known "Composition Start Point", which is represented by `_activeTextStart`. Whenever we complete a composition, we'll send the portion of the input buffer between `_activeTextStart` and the end of the input buffer to the terminal. Then, we'll update `_activeTextStart` to be the end of the input buffer so that the next time we send text to the terminal, we'll only send the portion of our buffer that's "active".
## PR Checklist
* [x] Closes#4226
* [x] CLA signed
* [x] Tests added/passed
## Validation Steps Performed
Manual testing with Chinese, Japanese, and Korean IME.
## Summary of the Pull Request
Currently, while in IME mode, selections with the Emoji/Kaomoji/Symbol Picker (which is brought up with <kbd>win+.</kbd>) are not displayed until the user starts a new composition. This is due to the fact that we hide the TextBlock when we receive a CompositionCompleted event, and we only show the TextBlock when we receive a CompositionStarted event. Input from the picker does not count as a composition, so we were never showing the text box, even if the symbols were thrown into the inputBuffer. In addition, we weren't receiving CompositionStarted events when we expected to.
We should be showing the TextBlock when we receive _any_ text, so we should make the TextBlock visible inside of `TextUpdatingHandler`. Furthermore, some really helpful discussion in #3745 around wrapping the `NotifyTextChanged` call with a `NotifyFocusLeave` and a `NotifyFocusEnter` allowed the control to much more consistently determine when a CompositionStarted and a CompositionEnded.
I've also went around and replaced casts with saturating casts, and have removed the line that sets the `textBlock.Width()` so that it would automatically set its width. This resolves the issue where while composing a sentence, the textBlock would be too small to contain all the text, so it would be cut off, but the composition is still valid and still able to continue.
## PR Checklist
* [x] Closes#4148
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Tested picking emojis, kaomojis, and symbols with numerous different languages.
Generated by https://github.com/jsoref/spelling `f`; to maintain your repo, please consider `fchurn`
I generally try to ignore upstream bits. I've accidentally included some items from the `deps/` directory. I expect someone will give me a list of items to drop, I'm happy to drop whole files/directories, or to split the PR into multiple items (E.g. comments/locals/public).
Closes#4294
* Remove unneeded c_str() calls when converting an hstring to a wstring_view.
* Remove unneeded c_str() calls when constructing a FontInfo class with a wstring face name.
* Remove unneeded winrt::to_hstring calls when passing a wstring to a method that expects an hstring.
* Remove unneeded c_str() calls when passing an hstring to a method that already accepts hstrings without conversion.
* Remove unneeded c_str() and data() calls when explicitly constructing an hstring from a wstring.
This commit introduces a new recursive pane shutdown that will give all
controls under a tab a chance to clean up their state before beign
detached from the UI. It also reorders the call to LastTabClosed() so
that the application does not exit before the final connections are
terminated.
It also teaches TSFInputControl how to shut down to avoid a dramatic
platform bug.
Fixes#4159.
Fixes#4336.
## PR Checklist
* [x] CLA signed
* [x] I've discussed this with core contributors already.
## Validation Steps Performed
Validated through manual terminal teardown within and without the debugger, given a crazy number of panes and tabs.
The first argument to `NotifyTextChanged` incorrectly was `[0,0]`
instead of the length of the text to be removed from the
`CoreTextEditContext`.
Best source of documentation for `NotifyTextChanged`:
https://docs.microsoft.com/en-us/windows/uwp/design/input/custom-text-input#overriding-text-updates
FYI @DHowett-MSFT (just in case): C++/WinRT uses `winrt::param::hstring`
for string parameters which intelligently borrows strings. As such you
can simply pass a `std::wstring` to most WinRT methods without the need
of having to allocate an intermediate `hstring`. 🙂
## Validation Steps Performed
I followed the reproduction instructions of #3706 and #3745 and ensured
the issue doesn't happen anymore.
Closes#3645Closes#3706Closes#3745
TerminalControl doesn't use any of the built in text input and edit
controls provided by XAML for text input, which means TermianlControl
needs to communicate with the Text Services Framework (TSF) in order to
provide Input Method Editor (IME) support. Just like the rest of
Terminal we get to take advantage of newer APIs (Windows.UI.Text.Core)
namespace to provide support vs. the old TSF 1.0.
Windows.UI.Text.Core handles communication between a text edit control
and the text services primarily through a CoreTextEditContext object.
This change introduces a new UserControl TSFInputControl which is a
custom EditControl similar to the CustomEditControl sample[1].
TSFInputControl is similar (overlay with IME text) to how old console
(conimeinfo) handled IME.
# Details
TSFInputControl is a Windows.UI.Xaml.Controls.UserControl
TSFInputControl contains a Canvas control for absolution positioning a
TextBlock control within its containing control (TerminalControl).
The TextBlock control is used for displaying candidate text from the
IME. When the user makes a choice in the IME the TextBlock is cleared
and the text is written to the Terminal buffer like normal text.
TSFInputControl creates an instance of the CoreTextEditContext and
attaches appropriate event handlers to CoreTextEditContext in order to
interact with the IME.
A good write-up on how to interact with CoreTextEditContext can be found
here[2].
## Text Updates
Text updates from the IME come in on the TextUpdating event handler,
text updates are stored in an internal buffer (_inputBuffer).
## Completed Text
Once a user selects a text in the IME, the CompositionCompleted handler
is invoked. The input buffer (_inputBuffer) is written to the Terminal
buffer, _inputBuffer is cleared and Canvas and TextBlock controls are
hidden until the user starts a composition session again.
## Positioning
Telling the IME where to properly position itself was the hardest part
of this change. The IME expects to know it's location in screen
coordinates as supposed to client coordinates. This is pretty easy if
you are a pure UWP, but since we are hosted inside a XAMLIsland the
client to screen coordinate translation is a little harder.
### Calculating Screen Coordinates
1. Obtaining the Window position in Screen coordinates.
2. Determining the Client coordinate of the cursor.
3. Converting the Client coordinate of the cursor to Screen coordinates.
4. Offsetting the X and Y coordinate of the cursor by the position of
the TerminalControl within the window (tabs if present, margins, etc..).
5. Applying any scale factor of the display.
Once we have the right position in screen coordinates, this is supplied
in the LayoutBounds of the CoreTextLayoutRequestedEventArgs which lets
the IME know where to position itself on the Screen.
## Font Information/Cursor/Writing to Terminal
3 events were added to the TSFInputControl to create a loosely-coupled
implementation between the TerminalControl and the TSFInputControl.
These events are used for obtaining Font information from the
TerminalControl, getting the Cursor position and writing to the terminal
buffer.
## Known Issues
- Width of TextBlock is hardcoded to 200 pixels and most likely should
adjust to the available width of the current input line on the console
(#3640)
- Entering text in the middle of an existing set of text has TextBlock
render under existing text. Current Console behavior here isn't good
experience either (writes over text)
- Text input at edges of window is clipped versus wrapping around to
next line. This isn't any worse than the original command line, but
Terminal should be better (#3657)
## Future Considerations
Ideally, we'd be able to interact with the console buffer directly and
replace characters as the user types.
## Validation
General steps to try functionality
- Open Console
- Switch to Simplified Chinese (Shortcut: Windows+Spacebar)
- Switch to Chinese mode on language bar
Scenarios validated:
- As user types unformatted candidates appear on command line and IME
renders in correct position under unformatted characters.
- User can dismiss IME and text doesn't appear on command line
- Switch back to English mode, functions like normal
- New tab has proper behavior
- Switching between tabs has proper behavior
- Switching away from Terminal Window with IME present causes IME to
disappear
[1]: https://github.com/Microsoft/Windows-universal-samples/tree/master/Samples/CustomEditControl
[2]: https://docs.microsoft.com/en-us/windows/uwp/design/input/custom-text-inputCloses#459Closes#2213Closes#3641
