This commit replaces DeviceComm with the interface IDeviceComm and the
concrete implementation type ConDrvDeviceComm. This work is done in
preparation for different device backends.
In addition to separating out ConDrv-specific behavior, I've introduced
a "handle exchange" interface.
HANDLE EXCHANGE
---------------
There are points where we give ConDrv opaque handle identifiers to our
input buffer, output buffer and process data. The exact content of the
opaque identifier is meaningless to ConDrv: the driver's only
interaction with these identifiers is to hold onto them and send back
whichever are pertinent for each API call.
Because of that, we used the raw register-width pointer value of the
input buffer, output buffer or process data _as_ the opaque handle
value.
This works very well for ConDrv <-> conhost using the ConDrvDeviceComm.
It works less well for something like the "logging" DeviceComm that will
log packets to a file: those packets *cannot* contain pointer values (!)
To address this, and to afford flexibility to DeviceComm implementers,
I've introduced a two-member complement of handle management functions:
* `ULONG_PTR PutHandle(void*)` registers an object with the DeviceComm
and returns an opaque identifier.
* `void* GetHandle(ULONG_PTR)` takes an opaque identifier and returns
the original object.
ConDrvDeviceComm implements PutHandle and GetHandle by casting the
object pointer to the "opaque handle value", which maintains wire format
compatibility[1] with revisions of conhost prior to this change.
Simpler DeviceComm implementations that require handle tracking but
cannot bear raw pointers can implement these functions by returning an
autoincrementing ID (or similar) and registering the raw object pointer
in a mapping.
I've audited all existing handle exchanges with the driver and updated
them to use Put/GetHandle.
(I intended to add DestroyHandle, but we are not equipped for handle
removal at the moment. ConsoleHandleData/ConsoleProcessHandle are
destroyed during wait routine completion, on client disconnect, etc.
This does mean that an id<->pointer mapping will grow without bound, but
at a cost of ~8 bytes per entry and a short-lived console session I am
not too concerned about the cost.)
[1] Wire format compatibility is not required, and later we may want to
switch ConDrvDeviceComm to `EncodePointer` and `DecodePointer` just to
insulate us against a spurious ConDrv packet allowing for an arbitrary
4/8-byte read and subsequent liftoff into space.
There are two issue with copy to clipboard when block is selected:
* We don't add new lines for lines that were wrapped
* We remove trailing whitespaces which is not intuitive in block selection.
Fixed the copy logic to always add newlines and not to remove
whitespaces when block is selected.
Even if shift is pressed!
## Detailed Description of the Pull Request / Additional comments
* Added optional parameter to `TextBuffer::GetText`
that allows to apply formatting (includeCRLF / trimming)
to lines that were wrapped
* Changed `Terminal::RetrieveSelectedTextFromBuffer`
to apply the following parameters when block is selected:
* includeCRLF = true
* trimTrailingWhitespaces = false
* apply the formatting above to all rows, including the ones
that were wrapped
## Validation Steps Performed
* Manual tests for both block and standard selection
* Copy with both right-click and command
* Added UT
Closes#6740
In conhost there is a keyboard shortcut that applies colors to the
selected range of text, and another shortcut that searches for the
selected text, and applies colors to any matching content. The former
operation doesn't work correctly when the selection is wrapped, and both
have problems when the selected text spans DBCS characters. This PR
attempts to fix those issues.
The problem with the color section was that it applied the color to a
simple rect spanning the start and end points of the selection. I've now
updated it to use the `Selection::GetSelectionRects` method, which
correctly handles a wrapped range of lines, and makes sure that double
width characters are fully covered.
The problem with the "find-and-color" operation was in the way it
obtained the search text from the selected screen cells. Since it
retrieved one cell at a time, and a DBCS character can span two cells,
that resulted in some characters being repeated in the search text. I've
now corrected that code to take the width of the characters into
account.
## Validation Steps Performed
I've manually verified that the test cases described in #8572 and #8574
are now working correctly.
Closes#8572Closes#8574
A bunch of our feature tests don't work reliably in CI. They rely on
creating a new `OpenConsole.exe` window, then running the test _in that
console_. As a part of that test setup, the test runner used to wait a
second to attach to the newly created console. Then the test goes on
it's merry way, assuming the console is ready to go. However, in CI,
that might take more than a second. If it does, then the test would fail
pretty immediately, as soon as it tries to get at the buffer of the new
console.
This PR introduces a little retry loop to the test init. After attaching
to the new console, we'll try and get at the screen buffer. If that
fails, we'll wait a second and try again. We'll try 5 times total,
before bailing entirely. Hopefully, this should mitigate most of the
random CI failures we get in the feature tests.
Closes#8495
When resizing the buffer in the `SetConsoleScreenBufferSize` and
`SetConsoleScreenBufferInfoEx` APIs, we have tests in place to make sure
that the resize doesn't result in the viewport extending past the bottom
or right of the buffer (since that can eventually result in exceptions
being thrown). Unfortunately these tests were using the wrong X
coordinate, so they failed to detect an overflow along the horizontal
axis. This PR corrects that mistake.
PR #8309 was where the overflow detection was initially added.
The original code was using the `Viewport::EndExclusive` method to
determine the extent of the viewport, mistakenly thinking that it
returned the bottom right coordinates (it actually returns the left
coordinate). So I've now added a `BottomRightExclusive` method to the
`Viewport` class, that actually does return the coordinates we need, and
have updated the overflow tests to use that method instead.
## Validation Steps Performed
I've manually confirmed that the test case is issue #8453 is no longer
throwing an exception.
Closes#8453
When the viewport is moved to the "virtual bottom" of the buffer (via
the `MoveToBottom` method), it is important that the horizontal viewport
offset be left as it is, otherwise that can result in some undesirable
side effects.
Since the VT coordinate system is relative to the top of the viewport,
many VT operations call the `MoveToBottom` method to make sure the
viewport is correctly positioned before proceeding. There is no need for
the horizontal position to be adjusted, though, since the X coordinates
are not constrained by the viewport, but are instead relative to the
underlying buffer.
Setting the viewport X coordinate to 0 in `MoveToBottom` (as we were
previously doing) could result in the cursor being pushed off screen.
And if the operation itself was moving the cursor, that would then
trigger another viewport move to bring the cursor back into view. These
conflicting movements meant the viewport was always forced as far left
as possible, and could also result in cursor "droppings" as the cursor
lost track of where it had been.
I've now fixed this by updating the `GetVirtualViewport` method to match
the horizontal offset of the active viewport, instead of having the X
coordinate hardcoded to 0.
## Validation Steps Performed
I've manually confirmed that this fixes the cursor "droppings" test case
reported in issue #8213.
I've also added a screen buffer test that makes sure the `MoveToBottom`
method is working as expected, and not changing the horizontal viewport
offset when it moves down.
Closes#8213
Many include statements use forward slashes, while others use backwards
slashes. This is inconsistent formatting. For this reason, I changed the
backward slashes to forward slashes since that is the standard.
[Git2Git] Git Train: Merge of building/rs_onecore_dep_uxp/201117-2002 into official/rs_onecore_dep_uxp Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_onecore_dep_uxp 526043ab6050397506b3cdb77e6a43b5ca9a2e9a
Related work items: MSFT-29990377
This fixes a number of exceptions that can cause conhost to crash when
the buffer is resized in such a way that the viewport or cursor position
end up out of bounds.
Technically this is a fix for issue #256, although that has been closed
as "needs-repro".
The main fix was to add checks in the `SetConsoleScreenBufferSizeImpl`
and `SetConsoleScreenBufferInfoExImpl` methods, to make sure the
viewport doesn't extend past the bottom or right of the buffer after a
resize. If it has overflowed, we move the viewport back up or left until
it's back within the buffer boundaries. We also check if the cursor
position has ended up out of bounds, and if so, clamp it back inside the
buffer.
The `SCREEN_INFORMATION::SetViewport` was also a source of viewport
overflow problems, because it was mistakenly using inclusive coordinates
in its range checks, which resulted in them being off by one. That has
now been corrected to use exclusive coordinates.
Finally, the `IsCursorDoubleWidth` method was incorrectly marked as
`noexcept`, which was preventing its exceptions from being caught.
Ideally it shouldn't be throwing exceptions at all any more, but I've
removed the `noexcept` specifier, so if it does throw an exception,
it'll at least have more chance of recovering without a crash.
## Validation Steps Performed
I put together a few test cases (based on the reports in issues #276 and
#1976) which consistently caused conhost to crash, or to generate an
exception visible in the debug output. With this PR applied, those test
cases are no longer crashing or triggering exceptions.
Closes#1976
Fixed potential errors caused by overflow or underfow in
SectionInput.cpp
## PR Checklist
* [x] CLA signed
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
In selectionInput.cpp, there is both a potential overflow and potential
underflow. To address this issue, I casted the calculation up to int,
which is then checked because of integer promotion. Underflow and
underflow is therefore impossible because now if the calculation exceeds
SHORT_MAX, it will have exceeded bufferSize.BottomInclusive() or
bufferSize.Top() anyway, and be set to them.
## Validation Steps Performed
Passed Unit Testing
Manual Validation
This pull request is the initial implementation of hyperlink auto
detection
Overall design:
- Upon startup, TerminalCore gives the TextBuffer some patterns it
should know about
- Whenever something in the viewport changes (i.e. text
output/scrolling), TerminalControl tells TerminalCore (through a
throttled function for performance) to retrieve the visible pattern
locations from the TextBuffer
- When the renderer encounters a region that is associated with a
pattern, it paints that region differently
References #5001Closes#574
It turns out that we missed part of the OSC 8 spec which indicated that
_hyperlinks with the same ID but different URIs are logically distinct._
> Character cells that have the same target URI and the same nonempty id
> are always underlined together on mouseover.
> The same id is only used for connecting character cells whose URIs is
> also the same. Character cells pointing to different URIs should never
> be underlined together when hovering over.
This pull request fixes that oversight by appending the (hashed) URI to
the generated ID.
When Terminal receives one of these links over ConPTY, it will hash the
URL a second time and therefore append a second hashed ID. This is taken
as an acceptable cost.
Fixes#7698
This PR introduces a pair of classes for managing VT parameters that
automatically handle range checking and default fallback values, so the
individual operations don't have to do that validation themselves. In
addition to simplifying the code, this fixes a few cases where we were
mishandling missing or extraneous parameters, and adds support for
parameter sequences on commands that couldn't previously handle them.
This PR also sets a limit on the number of parameters allowed, to help
thwart DoS memory consumption attacks.
## References
* The new parameter class also introduces the concept of an
omitted/default parameter which is not necessarily zero, which is a
prerequisite for addressing issue #4417.
## Detailed Description of the Pull Request / Additional comments
There are two new classes provide by this PR: a `VTParameter` class,
similar in function to a `std::optional<size_t>`, which holds an
individual parameter (which may be an omitted/default value); and a
`VTParameters` class, similar in function to `gsl:span<VTParameter>`,
which holds a sequence of those parameters.
Where `VTParameter` differs from `std::optional` is with the inclusion
of two cast operators. There is a `size_t` cast that interprets omitted
and zero values as 1 (the expected behaviour for most numeric
parameters). And there is a generic cast, for use with the enum
parameter types, which interprets omitted values as 0 (the expected
behaviour for most selective parameters).
The advantage of `VTParameters` class is that it has an `at` method that
can never fail - out of range values simply return the a default
`VTParameter` instance (this is standard behaviour in VT terminals). It
also has a `size` method that will always return a minimum count of 1,
since an empty parameter list is typically the equivalent of a single
"default" parameter, so this guarantees you'll get at least one value
when iterating over the list with `size()`.
For cases where we just need to call the same dispatch method for every
parameter, there is a helper `for_each` method, which repeatedly calls a
given predicate function with each value in the sequence. It also
collates the returned success values to determine the overall result of
the sequence. As with the `size` method, this will always make at least
one call, so it correctly handles empty sequences.
With those two classes in place, we could get rid of all the parameter
validation and default handling code in the `OutputStateMachineEngine`.
We now just use the `VTParameters::at` method to grab a parameter and
typically pass it straight to the appropriate dispatch method, letting
the cast operators automatically handle the assignment of default
values. Occasionally we might need a `value_or` call to specify a
non-standard default value, but those cases are fairly rare.
In some case the `OutputStateMachineEngine` was also checking whether
parameters values were in range, but for the most part this shouldn't
have been necessary, since that is something the dispatch classes would
already have been doing themselves (in the few cases that they weren't,
I've now updated them to do so).
I've also updated the `InputStateMachineEngine` in a similar way to the
`OutputStateMachineEngine`, getting rid of a few of the parameter
extraction methods, and simplifying other parts of the implementation.
It's not as clean a replacement as the output engine, but there are
still benefits in using the new classes.
## Validation Steps Performed
For the most part I haven't had to alter existing tests other than
accounting for changes to the API. There were a couple of tests I needed
to drop because they were checking for failure cases which shouldn't
have been failing (unexpected parameters should never be an error), or
testing output engine validation that is no longer handled at that
level.
I've added a few new tests to cover operations that take sequences of
selective parameters (`ED`, `EL`, `TBC`, `SM`, and `RM`). And I've
extended the cursor movement tests to make sure those operations can
handle extraneous parameters that weren't expected. I've also added a
test to verify that the state machine will correctly ignore parameters
beyond the maximum 32 parameter count limit.
I've also manual confirmed that the various test cases given in issues
#2101 are now working as expected.
Closes#2101
For whatever reason, the super linter seems to think that any file it doesn't recognize is an EDITORCONFIG file. That means all our `cpp`, `hpp`, `h`, `resw`, `xaml`, etc files are going to get linted with different rules than the clang-format ones we already use.
This PR disables the EDITORCONFIG linter, and has a minimal change to a cpp file to ensure that it's no longer linted by the action.
See also:
* #7637 added this
* #7799 is blocked by this
* #7924 is blocked by this
* Correct the behaviour of parsing `rgb:R/G/B`. It should be interpreted
as `RR/GG/BB` instead of `0R/0G/0B`
* Add support for `rgb:RRR/GGG/BBB` and `rgb:RRRR/GGGG/BBBB`. The
behaviour of 12 bit variants is to repeat the first digit at the end,
e.g. `rgb:123/456/789` becomes `rgb:1231/4564/7897`.
* Add support for `#` formats. We are following the rules of
[XParseColor] by interpreting `#RGB` as `R000G000B000`.
* Add support for XOrg app color names, which are supported by xterm, VTE
and many other terminal emulators.
* Multi-parameter OSC 4 is now supported.
* The chaining of OSC 10-12 is not yet supported. But the parameter
validation is relaxed by parsing the parameters as multi-params but
only use the first one, which means `\e]10;rgb:R/G/B;` and
`\e]10:rgb:R/G/B;invalid` will execute `OSC 10` with the first color
correctly. This fixes some of the issues mentioned in #942 but not
all of them.
[XParseColor]: https://linux.die.net/man/3/xparsecolorCloses#3715
This fixes a bug when moving backwards by word that resulted in #7742.
This also includes...
- a minor refactor that leverages `GetWordStart` in `MoveToPreviousWord`
- additional unit tests for movement by word
- a feature test comprised of the referenced bug report
`MoveToPreviousWord()` would...
- move backwards for each whitespace character
- then, move backwards for each regular character
This would actually result in moving to the beginning of the current "word" (as defined by a11y).
We actually need to do this process twice:
- the first time gets you to the beginning of the current word
- attempt to move back by one character
- the second time gets you to the beginning of the previous word
Rather than implementing 4 while loops, we leverage `GetWordStart()` to
attempt to move to the beginning of the previous word. We call it twice
(as described above). The logic is unchanged, but we instead reuse a
function that has already undergone more testing.
To make sure this works as expected, additional unit tests were
introduced covering "MoveByWord" in the TextBuffer.
## Validation Steps Performed
Added test for repro steps.
Added unit tests for movement by word.
Closes#7742
Sometimes when we were sliding the viewport to fit inside the buffer, we
would end up with left > right.
That would cause us to crash down the line when rendering.
Fixes MSFT:28387423
Fixes#7744
`EndExclusive` represents the end of the buffer. This is designed to not
point to any data on the buffer. UiaTextRange would point to this
`EndExclusive` and then attempt to move based on it. However, since it
does not point to any data, it could experience undefined behavior or
(inevitably) crash from running out of bounds.
This PR specifically checks for expansion and movement at that point,
and prevents us from moving beyond it. There are plans in the future to
define the "end" as the last character in the buffer. Until then, this
solution will suffice and provide correct behavior that doesn't crash.
## Validation Steps Performed
Performed the referenced bugs' repro steps and added test coverage.
Closes MSFT-20458595
Closes#7663Closes#7664
This PR adds support for the _blink_ graphic rendition attribute. When a
character is output with this attribute set, it "blinks" at a regular
interval, by cycling its color between the normal rendition and a dimmer
shade of that color.
The majority of the blinking mechanism is encapsulated in a new
`BlinkingState` class, which is shared between the Terminal and Conhost
implementations. This class keeps track of the position in the blinking
cycle, which determines whether characters are rendered as normal or
faint.
In Windows Terminal, the state is stored in the `Terminal` class, and in
Conhost it's stored in the `CONSOLE_INFORMATION` class. In both cases,
the `IsBlinkingFaint` method is used to determine the current blinking
rendition, and that is passed on as a parameter to the
`TextAttribute::CalculateRgbColors` method when these classes are
looking up attribute colors.
Prior to calculating the colors, the current attribute is also passed to
the `RecordBlinkingUsage` method, which keeps track of whether there are
actually any blink attributes in use. This is used to determine whether
the screen needs to be refreshed when the blinking cycle toggles between
the normal and faint renditions.
The refresh itself is handled by the `ToggleBlinkingRendition` method,
which is triggered by a timer. In Conhost this is just piggybacking on
the existing cursor blink timer, but in Windows Terminal it needs to
have its own separate timer, since the cursor timer is reset whenever a
key is pressed, which is not something we want for attribute blinking.
Although the `ToggleBlinkingRendition` is called at the same rate as the
cursor blinking, we actually only want the cells to blink at half that
frequency. We thus have a counter that cycles through four phases, and
blinking is rendered as faint for two of those four. Then every two
cycles - when the state changes - a redraw is triggered, but only if
there are actually blinking attributes in use (as previously recorded).
As mentioned earlier, the blinking frequency is based on the cursor
blink rate, so that means it'll automatically be disabled if a user has
set their cursor blink rate to none. It can also be disabled by turning
off the _Show animations in Windows_ option. In Conhost these settings
take effect immediately, but in Windows Terminal they only apply when a
new tab is opened.
This PR also adds partial support for the `SGR 6` _rapid blink_
attribute. This is not used by DEC terminals, but was defined in the
ECMA/ANSI standards. It's not widely supported, but many terminals just
it implement it as an alias for the regular `SGR 5` blink attribute, so
that's what I've done here too.
## Validation Steps Performed
I've checked the _Graphic rendition test pattern_ in Vttest, and
compared our representation of the blink attribute to that of an actual
DEC VT220 terminal as seen on [YouTube]. With the right color scheme
it's a reasonably close match.
[YouTube]: https://www.youtube.com/watch?v=03Pz5AmxbE4&t=1m55sCloses#7388
Some IME implementations do not produce composition strings, and their
users have come to rely on the cursor that conhost traditionally left on
until a composition string showed up. We shouldn't hide the cursor until
we get a string (as opposed to hiding it when composition begins) so as
to not break those IMEs.
Related to #6207.
Fixes MSFT:29219348
This fixes an issue where two CPRs could end up corrupted in the input
buffer. An application that sent two CPRs back-to-back could
end up reading the first few characters of the first prepended CPR
before handing us another CPR. We would dutifully prepend it to the
buffer, causing them to overlap.
```
^[^[2;2R[1;1R
^^ ^^^^^ First CPR
^^^^^^ Second CPR
```
The end result of this corruption is that a requesting application
would receive an unbidden `R` on stdin; for vim, this would trigger
replace mode immediately on startup.
Response prepending was implemented in !997738 without much comment.
There's very little in the way of audit trail as to why we switched.
Michael believes that we wanted to make sure that applications got DSR
responses immediately. It had the unfortunate side effect of causing
subsequence CPRs across cursor moves to come out in the wrong order.
I discussed our options with him, and he suggested that we could
implement a priority queue in InputBuffer and make sure that "response"
input was dispatched to a client application before any application- or
user-generated input. This was deemed to be too much work.
We decided that DSR responses getting top billing was likely to be a
stronger guarantee than most terminals are capable of giving, and that
we should be fine if we just switch it back to append.
Thanks to @k-takata, @tekki and @brammool for the investigation on the
vim side.
Fixes#1637.
This PR is about the behavior of DECSCUSR. This PR changes the meaning
of DECSCUSR 0 to restore the cursor style back to user default. This
differs from what VT spec says but it’s used in popular terminal
emulators like iTerm2 and VTE-based ones. See #1604.
Another change is that for parameter greater than 6, DECSCUSR should be
ignored, instead of restoring the cursor to legacy. This PR fixes it.
See #7382.
Fixes#1604.
<!-- 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
Conhost can now support OSC8 sequences (as specified [here](https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda)). Terminal also supports those sequences and additionally hyperlinks can be opened by Ctrl+LeftClicking on them.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#204
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] Closes#204
* [ ] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
Added support to:
- parse OSC8 sequences and extract URIs from them (conhost and terminal)
- add hyperlink uri data to textbuffer/screeninformation, associated with a hyperlink id (conhost and terminal)
- attach hyperlink ids to text to allow for uri extraction from the textbuffer/screeninformation (conhost and terminal)
- process ctrl+leftclick to open a hyperlink in the clicked region if present
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Open up a PowerShell tab and type
```PowerShell
${ESC}=[char]27
Write-Host "${ESC}]8;;https://github.com/microsoft/terminal${ESC}\This is a link!${ESC}]8;;${ESC}\"
```
Ctrl+LeftClick on the link correctly brings you to the terminal page on github
Use the Helix testing orchestration framework to run our Terminal LocalTests and Console Host UIA tests.
## References
#### Creates the following new issues:
- #7281 - re-enable local tests that were disabled to turn on Helix
- #7282 - re-enable UIA tests that were disabled to turn on Helix
- #7286 - investigate and implement appropriate compromise solution to how Skipped is handled by MUX Helix scripts
#### Consumes from:
- #7164 - The update to TAEF includes wttlog.dll. The WTT logs are what MUX's Helix scripts use to track the run state, convert to XUnit format, and notify both Helix and AzDO of what's going on.
#### Produces for:
- #671 - Making Terminal UIA tests is now possible
- #6963 - MUX's Helix scripts are already ready to capture PGO data on the Helix machines as certain tests run. Presuming we can author some reasonable scenarios, turning on the Helix environment gets us a good way toward automated PGO.
#### Related:
- #4490 - We lost the AzDO integration of our test data when I moved from the TAEF/VSTest adapter directly back to TE. Thanks to the WTTLog + Helix conversion scripts to XUnit + new upload phase, we have it back!
## PR Checklist
* [x] Closes#3838
* [x] I work here.
* [x] Literally adds tests.
* [ ] Should I update a testing doc in this repo?
* [x] Am core contributor. Hear me roar.
* [ ] Correct spell-checking the right way before merge.
## Detailed Description of the Pull Request / Additional comments
We have had two classes of tests that don't work in our usual build-machine testing environment:
1. Tests that require interactive UI automation or input injection (a.k.a. require a logged in user)
2. Tests that require the entire Windows Terminal to stand up (because our Xaml Islands dependency requires 1903 or later and the Windows Server instance for the build is based on 1809.)
The Helix testing environment solves both of these and is brought to us by our friends over in https://github.com/microsoft/microsoft-ui-xaml.
This PR takes a large portion of scripts and pipeline configuration steps from the Microsoft-UI-XAML repository and adjusts them for Terminal needs.
You can see the source of most of the files in either https://github.com/microsoft/microsoft-ui-xaml/tree/master/build/Helix or https://github.com/microsoft/microsoft-ui-xaml/tree/master/build/AzurePipelinesTemplates
Some of the modifications in the files include (but are not limited to) reasons like:
- Our test binaries are named differently than MUX's test binaries
- We don't need certain types of testing that MUX does.
- We use C++ and C# tests while MUX was using only C# tests (so the naming pattern and some of the parsing of those names is different e.g. :: separators in C++ and . separators in C#)
- Our pipeline phases work a bit differently than MUX and/or we need significantly fewer pieces to the testing matrix (like we don't test a wide variety of OS versions).
The build now runs in a few stages:
1. The usual build and run of unit tests/feature tests, packaging verification, and whatnot. This phase now also picks up and packs anything required for running tests in Helix into an artifact. (It also unifies the artifact name between the things Helix needs and the existing build outputs into the single `drop` artifact to make life a little easier.)
2. The Helix preparation build runs that picks up those artifacts, generates all the scripts required for Helix to understand the test modules/functions from our existing TAEF tests, packs it all up, and queues it on the Helix pool.
3. Helix generates a VM for our testing environment and runs all the TAEF tests that require it. The orchestrator at helix.dot.net watches over this and tracks the success/fail and progress of each module and function. The scripts from our MUX friends handle installing dependencies, making the system quiet for better reliability, detecting flaky tests and rerunning them, and coordinating all the log uploads (including for the subruns of tests that are re-run.)
4. A final build phase is run to look through the results with the Helix API and clean up the marking of tests that are flaky, link all the screenshots and console output logs into the AzDO tests panel, and other such niceities.
We are set to run Helix tests on the Feature test policy of only x64 for now.
Additionally, because the set up of the Helix VMs takes so long, we are *NOT* running these in PR trigger right now as I believe we all very much value our 15ish minute PR turnaround (and the VM takes another 15 minutes to just get going for whatever reason.) For now, they will only run as a rolling build on master after PRs are merged. We should still know when there's an issue within about an hour of something merging and multiple PRs merging fast will be done on the rolling build as a batch run (not one per).
In addition to setting up the entire Helix testing pipeline for the tests that require it, I've preserved our classic way of running unit and feature tests (that don't require an elaborate environment) directly on the build machines. But with one bonus feature... They now use some of the scripts from MUX to transform their log data and report it to AzDO so it shows up beautifully in the build report. (We used to have this before I removed the MStest/VStest wrapper for performance reasons, but now we can have reporting AND performance!) See https://dev.azure.com/ms/terminal/_build/results?buildId=101654&view=ms.vss-test-web.build-test-results-tab for an example.
I explored running all of the tests on Helix but.... the Helix setup time is long and the resources are more expensive. I felt it was better to preserve the "quick signal" by continuing to run these directly on the build machine (and skipping the more expensive/slow Helix setup if they fail.) It also works well with the split between PR builds not running Helix and the rolling build running Helix. PR builds will get a good chunk of tests for a quick turn around and the rolling build will finish the more thorough job a bit more slowly.
## Validation Steps Performed
- [x] Ran the updated pipelines with Pull Request configuration ensuring that Helix tests don't run in the usual CI
- [x] Ran with simulation of the rolling build to ensure that the tests now running in Helix will pass. All failures marked for follow on in reference issues.
This PR adds support for the ANSI _doubly underlined_ graphic rendition
attribute, which is enabled by the `SGR 21` escape sequence.
There was already an `ExtendedAttributes::DoublyUnderlined` flag in the
`TextAttribute` class, but I needed to add `SetDoublyUnderlined` and
`IsDoublyUnderlined` methods to access that flag, and update the
`SetGraphicsRendition` methods of the two dispatchers to set the
attribute on receipt of the `SGR 21` sequence. I also had to update the
existing `SGR 24` handler to reset _DoublyUnderlined_ in addition to
_Underlined_, since they share the same reset sequence.
For the rendering, I've added a new grid line type, which essentially
just draws an additional line with the same thickness as the regular
underline, but slightly below it - I found a gap of around 0.05 "em"
between the lines looked best. If there isn't enough space in the cell
for that gap, the second line will be clamped to overlap the first, so
you then just get a thicker line. If there isn't even enough space below
for a thicker line, we move the offset _above_ the first line, but just
enough to make it thicker.
The only other complication was the update of the `Xterm256Engine` in
the VT renderer. As mentioned above, the two underline attributes share
the same reset sequence, so to forward that state over conpty we require
a slightly more complicated process than with most other attributes
(similar to _Bold_ and _Faint_). We first check whether either underline
attribute needs to be turned off to send the reset sequence, and then
check individually if each of them needs to be turned back on again.
## Validation Steps Performed
For testing, I've extended the existing attribute tests in
`AdapterTest`, `VTRendererTest`, and `ScreenBufferTests`, to make sure
we're covering both the _Underlined_ and _DoublyUnderlined_ attributes.
I've also manually tested the `SGR 21` sequence in conhost and Windows
Terminal, with a variety of fonts and font sizes, to make sure the
rendering was reasonably distinguishable from a single underline.
Closes#2916
Carlos Zamora (1)
* UIA: use full buffer comparison in rects and endpoint setter (GH-6447)
Dan Thompson (2)
* Tweaks: normalize TextAttribute method names (adjective form) (GH-6951)
* Fix 'bcz exclusive' typo (GH-6938)
Dustin L. Howett (4)
* Fix VT mouse capture issues in Terminal and conhost (GH-7166)
* version: bump to 1.3 on master
* Update Cascadia Code to 2007.15 (GH-6958)
* Move to the TerminalDependencies NuGet feed (GH-6954)
James Holderness (3)
* Render the SGR "underlined" attribute in the style of the font (CC-7148)
* Add support for the "crossed-out" graphic rendition attribute (CC-7143)
* Refactor grid line renderers with support for more line types (CC-7107)
Leonard Hecker (1)
* Added til::spsc, a lock-free, single-producer/-consumer FIFO queue (CC-6751)
Michael Niksa (6)
* Update TAEF to 10.57.200731005-develop (GH-7164)
* Skip DX invalidation if we've already scrolled an entire screen worth of height (GH-6922)
* Commit attr runs less frequently by accumulating length of color run (GH-6919)
* Set memory order on slow atomics (GH-6920)
* Cache the viewport to make invalidation faster (GH-6918)
* Correct comment in this SPSC test as a quick follow up to merge.
Related work items: MSFT-28208358
Updates TAEF to 10.57.200731005-develop
## PR Checklist
* [x] Helps #6992 by bringing `wttlog.dll` along with the rest of TAEF.
* [x] I work here.
* [x] Automated tests in CI
* [x] No doc/schema update necessary (checked for docs in this repo)
* [x] Am core contributor.
This PR updates the rendering of the _underlined_ graphic rendition
attribute, using the style specified in the active font, instead of just
reusing the grid line at the bottom of the character cell.
* Support for drawing the correct underline effect in the grid line
renderer was added in #7107.
There was already an `ExtendedAttributes` flag defined for the
underlined state, but I needed to update the `SetUnderlined` and
`IsUnderlined` methods in the `TextAttribute` class to use that flag
now in place of the legacy `LVB_UNDERSCORE` attribute. This enables
underlines set via a VT sequence to be tracked separately from
`LVB_UNDERSCORE` grid lines set via the console API.
I then needed to update the `Renderer::s_GetGridlines` method to
activate the `GridLines::Underline` style when the `Underlined`
attribute was set. The `GridLines::Bottom` style is still triggered by
the `LVB_UNDERSCORE` attribute to produce the bottom grid line effect.
Validation
----------
Because this is a change from the existing behaviour, certain unit tests
that were expecting the `LVB_UNDERSCORE` to be toggled by `SGR 4` and
`SGR 24` have now had to be updated to check the `Underlined` flag
instead.
There were also some UI Automation tests that were checking for `SGR 4`
mapping to `LVB_UNDERSCORE` attribute, which I've now substituted with a
test of the `SGR 53` overline attribute mapping to
`LVB_GRID_HORIZONTAL`. These tests only work with legacy attributes, so
they can't access the extended underline state, and I thought a
replacement test that covered similar ground would be better than
dropping the tests altogether.
As far as the visual rendering is concerned, I've manually confirmed
that the VT underline sequences now draw the underline in the correct
position and style, while grid lines output via the console API are
still displayed in their original form.
Closes#2915
- Move to GSL 3.1.0 (GH-6908)
- Replace the color table init code with two const arrays (GH-6913)
- Replace basic_string_view<T> with span<const T> (GH-6921)
- Replace gsl::at with a new til::at(span) for pre-checked bounds (GH-6925)
Related work items: MSFT:27866336
[Git2Git] Git Train: Merge of building/rs_onecore_dep_uxp/200720-1445 into official/rs_onecore_dep_uxp
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_onecore_dep_uxp 5e1509a00f1acdefe1e6392468ffc9ae44dc89cb
Related work items: MSFT:25731998
## Summary of the Pull Request
Text can have various attributes, such as "bold", "italic", "underlined", etc. The TextAttribute class embodies this. It has methods to set/query these attributes.
This change tweaks a few of the method names to make them match. I.e. for an imaginary text property "Foo", we should have methods along the lines of:
```
IsFoo
SetFoo(bool isFoo)
```
And variations should match: we should have "Foo" and "OverFoo", not "Fooey" and "OverFoo".
I chose to standardize on the adjective form, since that's what we are closest to already. The attributes I attacked here are:
SetItalic**s** --> SetItalic
SetUnderline --> SetUnderline**d**
SetOverline --> SetOverline**d**
("italic" is an adjective; "italics" is a plural noun, representing letters or words in an italic typeface)
And I also added methods for "DoublyUnderlined" for good measure.
I stopped short of renaming the GraphicsOptions enum values to match, too; but I'd be willing to do that in a follow-up change if people wanted it.
## Validation Steps Performed
It builds, and tests still pass.
In `Renderer::TriggerRedraw`, the act of fetching the viewport from the
`pData` over and over is wasted time. We already have a cached variable
of the viewport that is updated on every scroll check (on
`TriggerScroll` and on `PaintFrame`.) Scrolling wouldn't be working
correctly if the clients weren't already notifying us that the viewport
has changed for scroll purposes, so we can just keep using that cached
value for the invalidation restriction to speed things up over fetching
it again.
## Validation Steps Performed
- Run `time cat big.txt`. Checked average time before/after, WPR traces
before/after.
## PR Checklist
* [x] Closes perf itch
* [x] I work here
* [x] Manual test
* [x] Documentation irrelevant.
* [x] Schema irrelevant.
* [x] Am core contributor.
The recent changes to use gsl::span everywhere added a few bounds checks
along codepaths where we were already checking bounds. Some of them may
be non-obvious to the optimizer, so we can now use til::at to help them
along.
To accomplish this, I've added a new overload of til::at that takes a
span and directly accesses its backing buffer.
We were using std::basic_string_view as a stand-in for std::span so that
we could change over all at once when C++20 dropped with full span
support. That day's not here yet, but as of 54a7fce3e we're using GSL 3,
whose span is C++20-compliant.
This commit replaces every instance of basic_string_view that was not
referring to an actual string with a span of the appropriate type.
I moved the `const` qualifier into span's `T` because while
`basic_string_view.at()` returns `const T&`, `span.at()` returns `T&`
(without the const). I wanted to maintain the invariant that members of
the span were immutable.
* Mechanical Changes
* `sv.at(x)` -> `gsl::at(sp, x)`
* `sv.c{begin,end}` -> `sp.{begin,end}` (span's iterators are const)
I had to replace a `std::basic_string<>` with a `std::vector<>` in
ConImeInfo, and I chose to replace a manual array walk in
ScreenInfoUiaProviderBase with a ranged-for. Please review those
specifically.
This will almost certainly cause a code size regression in Windows
because I'm blowing out all the PGO counts. Whoops.
Related: #3956, #975.
GSL 3, the next major version of GSL after the one we're using, replaced
their local implementation of `span` with one that more closely mimics
C++20's span. Unfortunately, that is a breaking change for all of GSL's
consumers.
This commit updates our use of span to comply with the new changes in
GSL 3.
Chief among those breaking changes is:
* `span::at` no longer exists; I replaced many instances of `span::at`
with `gsl::at(x)`
* `span::size_type` has finally given up on `ptrdiff_t` and become
`size_t` like all other containers
While I was here, I also made the following mechanical replacements:
* In some of our "early standardized" code, we used std::optional's
`has_value` and `value` back-to-back. Each `value` incurs an
additional presence test.
* Change: `x.value().member` -> `x->member` (`optional::operator->`
skips the presence test)
* Change: `x.value()` -> `*x` (as above)
* GSL 3 uses `size_t` for `size_type`.
* Change: `gsl::narrow<size_t>(x.size())` -> `x.size()`
* Change: `gsl::narrow<ptrdiff_t>(nonSpan.size())` -> `nonSpan.size()`
during span construction
I also replaced two instances of `x[x.size() - 1]` with `x.back()` and
one instance of a manual array walk (for comparison) with a direct
comparison.
NOTE: Span comparison and `make_span` are not part of the C++20 span
library.
Fixes#6251
console: switch to /Zc:wchar_t (native wchar_t)
This matches what we use in OpenConsole and makes {fmt} play nice.
I've also removed the workaround we introduced into OutputCellIterator
to work around not using /Zc:wchar_t.
Fixes MSFT:27626309.
Fixes GH-2673.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_onecore_dep_uxp 1508f7c232ec58bebc37fedfdec3eb8f9bff5502
## Summary of the Pull Request
This PR adds support for the `SGR 2` escape sequence, which enables the
ANSI _faint_ graphic rendition attribute. When a character is output
with this attribute set, it uses a dimmer version of the active
foreground color.
## PR Checklist
* [x] Closes#6703
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number where discussion took place: #6703
## Detailed Description of the Pull Request / Additional comments
There was already an `ExtendedAttributes::Faint` flag in the
`TextAttribute` class, but I needed to add `SetFaint` and `IsFaint`
methods to access that flag, and update the `SetGraphicsRendition`
methods of the two dispatchers to set the attribute on receipt of the
`SGR 2` sequence. I also had to update the existing `SGR 22` handler to
reset _Faint_ in addition to _Bold_, since they share the same reset
sequence. For that reason, I thought it a good idea to change the name
of the `SGR 22` enum to `NotBoldOrFaint`.
For the purpose of rendering, I've updated the
`TextAttribute::CalculateRgbColors` method to return a dimmer version of
the foreground color when the _Faint_ attribute is set. This is simply
achieved by dividing each color component by two, which produces a
reasonable effect without being too complicated. Note that the _Faint_
effect is applied before _Reverse Video_, so if the output it reversed,
it's the background that will be faint.
The only other complication was the update of the `Xterm256Engine` in
the VT renderer. As mentioned above, _Bold_ and _Faint_ share the same
reset sequence, so to forward that state over conpty we have to go
through a slightly more complicated process than with other attributes.
We first check whether either attribute needs to be turned off to send
the reset sequence, and then check if the individual attributes need to
be turned on again.
## Validation
I've extended the existing SGR unit tests to cover the new attribute in
the `AdapterTest`, the `ScreenBufferTests`, and the `VtRendererTest`,
and added a test to confirm the color calculations when _Faint_ is set
in the `TextAttributeTests`.
I've also done a bunch of manual testing with all the different VT color
types and confirmed that our output is comparable to most other
terminals.
This is a refactoring of the renderer color calculations to simplify the
implementation, and to make it easier to support additional
color-altering rendition attributes in the future (e.g. _faint_ and
_conceal_).
## References
* This is a followup to PRs #3817 and #6809, which introduced additional
complexity in the color calculations, and which suggested the need for
refactoring.
## Detailed Description of the Pull Request / Additional comments
When we added support for `DECSCNM`, that required the foreground and
background color lookup methods to be able to return the opposite of
what was requested when the reversed mode was set. That made those
methods unnecessarily complicated, and I thought we could simplify them
considerably just by combining the calculations into a single method
that derived both colors at the same time.
And since both conhost and Windows Terminal needed to perform the same
calculations, it also made sense to move that functionality into the
`TextAttribute` class, where it could easily be shared.
In general this way of doing things is a bit more efficient. However, it
does result in some unnecessary work when only one of the colors is
required, as is the case for the gridline painter. So to make that less
of an issue, I've reordered the gridline code a bit so it at least
avoids looking up the colors when no gridlines are needed.
## Validation Steps Performed
Because of the API changes, quite a lot of the unit tests had to be
updated. For example instead of verifying colors with two separate calls
to `LookupForegroundColor` and `LookupBackgroundColor`, that's now
achieved with a single `LookupAttributeColors` call, comparing against a
pair of values. The specifics of the tests haven't changed though, and
they're all still working as expected.
I've also manually confirmed that the various color sequences and
rendition attributes are rendering correctly with the new refactoring.
There is going to be a very long tail of applications that will
explicitly request VT SGR 40/37 when what they really want is to
SetConsoleTextAttribute() with a black background/white foreground.
Instead of making those applications look bad (and therefore making us
look bad, because we're releasing this as an update to something that
"looks good" already), we're introducing this compatibility quirk.
Before the color reckoning in #6698 + #6506, *every* color was subject
to being spontaneously and erroneously turned into the default color.
Now, only the 16-color palette value that matches the active console
background/foreground color will be destroyed, and only when received
from specific applications.
Removal will be tracked by #6807.
Michael and I discussed what layer this quirk really belonged in. I
originally believed it would be sufficient to detect a background color
that matched the legacy default background, but @j4james provided an
example of where that wouldn't work out (powershell setting the
foreground color to white/gray). In addition, it was too heavyhanded: it
re-broke black backgrounds for every application.
Michael thought that it should live in the server, as a small VT parser
that righted the wrongs coming directly out of the application. On
further investigation, however, I realized that we'd need to push more
information up into the server (so that it could make the decision about
which VT was wrong and which was right) than should be strictly
necessary.
The host knows which colors are right and wrong, and it gets final say
in what ends up in the buffer.
Because of that, I chose to push the quirk state down through
WriteConsole to DoWriteConsole and toggle state on the
SCREEN_INFORMATION that indicates whether the colors coming out of the
application are to be distrusted. This quirk _only applies to pwsh.exe
and powershell.exe._
NOTE: This doesn't work for PowerShell the .NET Global tool, because it
is run as an assembly through dotnet.exe. I have no opinion on how to
fix this, or whether it is worth fixing.
VALIDATION
----------
I configured my terminals to have an incredibly garish color scheme to
show exactly what's going to happen as a result of this. The _default
terminal background_ is purple or red, and the foreground green. I've
printed out a heap of test colors to see how black interacts with them.
Pull request #6810 contains the images generated from this test.
The only color lines that change are the ones where black as a
background or white as a foreground is selected out of the 16-color
palette explicitly. Reverse video still works fine (because black is in
the foreground!), and it's even possible to represent "black on default"
and reverse it into "default on black", despite the black in question
having been `40`.
Fixes#6767.
## Summary of the Pull Request
This PR adds support for the `SGR 53` and `SGR 55` escapes sequences,
which enable and disable the ANSI _overline_ graphic rendition
attribute, the equivalent of the console character attribute
`COMMON_LVB_GRID_HORIZONTAL`. When a character is output with this
attribute set, a horizontal line is rendered at the top of the character
cell.
## PR Checklist
* [x] Closes#6000
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
To start with, I added `SetOverline` and `IsOverlined` methods to the
`TextAttribute` class, to set and get the legacy
`COMMON_LVB_GRID_HORIZONTAL` attribute. Technically there was already an
`IsTopHorizontalDisplayed` method, but I thought it more readable to add
a separate `IsOverlined` as an alias for that.
Then it was just a matter of adding calls to set and reset the attribute
in response to the `SGR 53` and `SGR 55` sequences in the
`SetGraphicsRendition` methods of the two dispatchers. The actual
rendering was already taken care of by the `PaintBufferGridLines` method
in the rendering engines.
The only other change required was to update the `_UpdateExtendedAttrs`
method in the `Xterm256Engine` of the VT renderer, to ensure the
attribute state would be forwarded to the Windows Terminal over conpty.
## Validation Steps Performed
I've extended the existing SGR unit tests to cover the new attribute in
the `AdapterTest`, the `OutputEngineTest`, and the `VtRendererTest`.
I've also manually tested the `SGR 53` and `SGR 55` sequences to confirm
that they do actually render (or remove) an overline on the characters
being output.
This PR reimplements the VT rendering engines to do a better job of
preserving the original color types when propagating attributes over
ConPTY. For the 16-color renderers it provides better support for
default colors and improves the efficiency of the color narrowing
conversions. It also fixes problems with the ordering of character
renditions that could result in attributes being dropped.
Originally the base renderer would calculate the RGB color values and
legacy/extended attributes up front, passing that data on to the active
engine's `UpdateDrawingBrushes` method. With this new implementation,
the renderer now just passes through the original `TextAttribute` along
with an `IRenderData` interface, and leaves it to the engines to extract
the information they need.
The GDI and DirectX engines now have to lookup the RGB colors themselves
(via simple `IRenderData` calls), but have no need for the other
attributes. The VT engines extract the information that they need from
the `TextAttribute`, instead of having to reverse engineer it from
`COLORREF`s.
The process for the 256-color Xterm engine starts with a check for
default colors. If both foreground and background are default, it
outputs a SGR 0 reset, and clears the `_lastTextAttribute` completely to
make sure any reset state is reapplied. With that out the way, the
foreground and background are updated (if changed) in one of 4 ways.
They can either be a default value (SGR 39 and 49), a 16-color index
(using ANSI or AIX sequences), a 256-color index, or a 24-bit RGB value
(both using SGR 38 and 48 sequences).
Then once the colors are accounted for, there is a separate step that
handles the character rendition attributes (bold, italics, underline,
etc.) This step must come _after_ the color sequences, in case a SGR
reset is required, which would otherwise have cleared any character
rendition attributes if it came last (which is what happened in the
original implementation).
The process for the 16-color engines is a little different. The target
client in this case (Windows telnet) is incapable of setting default
colors individually, so we need to output an SGR 0 reset if _either_
color has changed to default. With that out the way, we use the
`TextColor::GetLegacyIndex` method to obtain an approximate 16-color
index for each color, and apply the bold attribute by brightening the
foreground index (setting bit 8) if the color type permits that.
However, since Windows telnet only supports the 8 basic ANSI colors, the
best we can do for bright colors is to output an SGR 1 attribute to get
a bright foreground. There is nothing we can do about a bright
background, so after that we just have to drop the high bit from the
colors. If the resulting index values have changed from what they were
before, we then output ANSI 8-color SGR sequences to update them.
As with the 256-color engine, there is also a final step to handle the
character rendition attributes. But in this case, the only supported
attributes are underline and reversed video.
Since the VT engines no longer depend on the active color table and
default color values, there was quite a lot of code that could now be
removed. This included the `IDefaultColorProvider` interface and
implementations, the `Find(Nearest)TableIndex` functions, and also the
associated HLS conversion and difference calculations.
VALIDATION
Other than simple API parameter changes, the majority of updates
required in the unit tests were to correct assumptions about the way the
colors should be rendered, which were the source of the narrowing bugs
this PR was trying to fix. Like passing white on black to the
`UpdateDrawingBrushes` API, and expecting it to output the default `SGR
0` sequence, or passing an RGB color and expecting an indexed SGR
sequence.
In addition to that, I've added some VT renderer tests to make sure the
rendition attributes (bold, underline, etc) are correctly retained when
a default color update causes an `SGR 0` sequence to be generated (the
source of bug #3076). And I've extended the VT renderer color tests
(both 256-color and 16-color) to make sure we're covering all of the
different color types (default, RGB, and both forms of indexed colors).
I've also tried to manually verify that all of the test cases in the
linked bug reports (and their associated duplicates) are now fixed when
this PR is applied.
Closes#2661Closes#3076Closes#3717Closes#5384Closes#5864
This is only a partial fix for #293, but I suspect the remaining cases
are unfixable.
Essentially what this does is map the default legacy foreground and
background attributes (typically white on black) to the `IsDefault`
color type in the `TextColor` class. As a result, we can now initialize
the buffer for "legacy" shells (like PowerShell and cmd.exe) with
default colors, instead of white on black. This fixes the startup
rendering in conpty clients, which expect an initial default background
color. It also makes these colors update appropriately when the default
palette values change.
One complication in getting this to work, is that the console permits
users to change which color indices are designated as defaults, so we
can't assume they'll always be white on black. This means that the
legacy-to-`TextAttribute` conversion will need access to those default
values.
Unfortunately the defaults are stored in the conhost `Settings` class
(the `_wFillAttribute` field), which isn't easily accessible to all the
code that needs to construct a `TextAttribute` from a legacy value. The
`OutputCellIterator` is particularly problematic, because some iterator
types need to generate a new `TextAttribute` on every iteration.
So after trying a couple of different approaches, I decided that the
least worst option would be to add a pair of static properties for the
legacy defaults in the `TextAttribute` class itself, then refresh those
values from the `Settings` class whenever the defaults changed (this
only happens on startup, or when the conhost _Properties_ dialog is
edited).
And once the `TextAttribute` class had access to those defaults, it was
fairly easy to adapt the constructor to handle the conversion of default
values to the `IsDefault` color type. I could also then simplify the
`TextAttribute::GetLegacyAttributes` method which does the reverse
mapping, and which previously required the default values to be passed
in as a parameter
VALIDATION
I had to make one small change to the `TestRoundtripExhaustive` unit
test which assumed that all legacy attributes would convert to legacy
color types, which is no longer the case, but otherwise all the existing
tests passed as is. I added a new unit test verifying that the default
legacy attributes correctly mapped to default color types, and the
default color types were mapped back to the correct legacy attributes.
I've manually confirmed that this fixed the issue raised in #5952,
namely that the conhost screen is cleared with the correct default
colors, and also that it is correctly refreshed when changing the
palette from the properties dialog. And I've combined this PR with
#6506, and confirmed that the PowerShell and the cmd shell renderings in
Windows Terminal are at least improved, if not always perfect.
This is a prerequisite for PR #6506Closes#5952
<!-- 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
Many places in this codebase has an equality comparison to the boolean FALSE. This adds unneeded complexity as C and C++ has a NOT operand for use of these in if statements. This makes the code more readable in those areas.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [X] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
One boolean being compared to FALSE was only used once, with the boolean name being "b", so it is better off not existing at all.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Unit Testing passed, compiler refactoring
Bump Newtonsoft.Json from 10.0.3 to 12.0.3
## References
Part of #5297
## PR Checklist
* [ ] Closes (none)
* [x] CLA signed
* [ ] Tests added/passed N/A
* [ ] Requires documentation to be updated N/A
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Validation Steps Performed
CI builds successfully
This pull request moves WindowUiaProvider back into Win32 interactivity
and deletes all mention of it from Windows Terminal. Terminal does not
have a single toplevel window that requires Console-like UIA, as each
Xaml control inside it is in charge of its own destiny.
I've also merged `IUiaWindow` and `IConsoleWindow` back together, as
well as `WindowUiaProviderBase` and `WindowUiaProvider`.
Things look a lot more like they did before we tore them apart.
## PR Checklist
* [x] Closes#3564
* [x] CLA
* [x] Tests added/passed (manual)
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already
## Validation
Carlos validated conhost and terminal on this branch.
Adds support for `win32-input-mode` to conhost, conpty, and the Windows
Terminal.
* The shared `terminalInput` class supports sending these sequences when
a VT client application requests this mode.
* ConPTY supports synthesizing `INPUT_RECORD`s from the input sent to it
from a terminal
* ConPTY requests this mode immediately on startup (if started with a
new flag, `PSEUDOCONSOLE_WIN32_INPUT_MODE`)
* The Terminal now supports sending this input as well, when conpty asks
for it.
Also adds a new ConPTY flag `PSEUDOCONSOLE_WIN32_INPUT_MODE` which
requests this functionality from conpty, and the Terminal requests this
by default.
Also adds `experimental.input.forceVT` as a global setting to let a user
opt-out of this behavior, if they don't want it / this ends up breaking
horribly.
## Validation Steps Performed
* played with this mode in vtpipeterm
* played with this mode in Terminal
* checked a bunch of scenarios, as outlined in a [comment] on #4999
[comment]: https://github.com/microsoft/terminal/issues/4999#issuecomment-628718631
References #4999: The megathread
References #5887: The spec
Closes#879Closes#2865Closes#530Closes#3079Closes#1119Closes#1694Closes#3608Closes#4334Closes#4446
This PR provides a faster algorithm for converting 8-bit and 24-bit
colors into the 4-bit legacy values that are required by the Win32
console APIs. It also fixes areas of the code that were incorrectly
using a simple 16-color conversion that didn't handle 8-bit and 24-bit
values.
The faster conversion algorithm should be an improvement for issues #783
and #3950.
One of the main points of this PR was to fix the
`ReadConsoleOutputAttribute` API, which was using a simplified legacy
color conversion (the original `TextAttribute:GetLegacyAttributes`
method), which could only handle values from the 16-color table. RGB
values, and colors from the 256-color table, would be mapped to
completely nonsensical values. This API has now been updated to use the
more correct `Settings::GenerateLegacyAttributes` method.
But there were also a couple of other places in the code that were using
`GetLegacyAttributes` when they really had no reason to be working with
legacy attributes at all. This could result in colors being downgraded
to 4-bit values (often badly, as explained above), when the code was
already perfectly capable of displaying the full 24-bits.
This included the fill colors in the IME composer (in `ConsoleImeInfo`),
and the construction of the highlighting colors in the color
search/selection handler (`Selection::_HandleColorSelection`). I also
got rid of some legacy attribute code in the `Popup` class, which was
originally intended to update colors below the popup when the settings
changed, but actually caused more problems than it solved.
The other major goal of this PR was to improve the performance of the
`GenerateLegacyAttributes` method, since the existing implementation
could be quite slow when dealing with RGB values.
The simple cases are handled much the same as they were before. For an
`IsDefault` color, we get the default index from the
`Settings::_wFillAttribute` field. For an `IsIndex16` color, the index
can just be returned as is.
For an `IsRgb` color, the RGB components are compressed down to 8 bits
(3 red, 3 green, 2 blue), simply by dropping the least significant bits.
This 8-bit value is then used to lookup a representative 16-color value
from a hard-coded table. An `IsIndex256` color is also converted with a
lookup table, just using the existing 8-bit index.
The RGB mapping table was calculated by taking each compressed 8-bit
color, and picking a entry from the _Campbell_ palette that best
approximated that color. This was done by looking at a range of 24-bit
colors that mapped to the 8-bit value, finding the best _Campbell_ match
for each of them (using a [CIEDE2000] color difference calculation), and
then the most common match became the index that the 8-bit value would
map to.
The 256-color table was just a simpler version of this process. For each
entry in the table, we take the default RGB palette value, and find it's
closest match in the _Campbell_ palette.
Because these tables are hard-coded, the results won't adjust to changes
in the palette. However, they should still produce reasonable results
for palettes that follow the standard ANSI color range. And since
they're only a very loose approximation of the colors anyway, the exact
value really isn't that important.
That said, I have tried to make sure that if you take an RGB value for a
particular index in a reasonable color scheme, then the legacy color
mapped from that value should ideally match the same index. This will
never be possible for all color schemes, but I have tweaked a few of the
table entries to improve the results for some of the common schemes.
One other point worth making regarding the hard-coded tables: even if we
wanted to take the active palette into account, that wouldn't actually
be possible over a conpty connection, because we can't easily know what
color scheme the client application is using. At least this way the
results in conhost are guaranteed to be the same as in the Windows
Terminal.
[CIEDE2000]: https://en.wikipedia.org/wiki/Color_difference#CIEDE2000
## Validation Steps Performed
This code still passes the `TextAttributeTests` that check the basic
`GetLegacyAttribute` behaviour and verify the all legacy attributes
roundtrip correctly. However, some of the values in the `RgbColorTests`
had to be updated, since we're now intentionally returning different
values as a result of the changes to the RGB conversion algorithm.
I haven't added additional unit tests, but I have done a lot of manual
testing to see how well the new algorithm works with a range of colors
and a variety of different color schemes. It's not perfect in every
situation, but I think it works well enough for the purpose it serves.
I've also confirmed that the issues reported in #5940 and #6247 are now
fixed by these changes.
Closes#5940Closes#6247
In Windows, we build with /Zc:wchar_t- (which makes wchar_t an unsigned
short typedef.) This causes build breaks when we compare two wchar_t
values (or a wchar_t and an enum class that's of type wchar_t) and the
compiler decides that it might want to _promote them to TextAttribute_
before doing the comparison.
This PR improves our VT character set support, enabling the [`SCS`]
escape sequences to designate into all four G-sets with both 94- and
96-character sets, and supports invoking those G-sets into both the GL
and GR areas of the code table, with [locking shifts] and [single
shifts]. It also adds [`DOCS`] sequences to switch between UTF-8 and the
ISO-2022 coding system (which is what the VT character sets require),
and adds support for a lot more characters sets, up to around the level
of a VT510.
[`SCS`]: https://vt100.net/docs/vt510-rm/SCS.html
[locking shifts]: https://vt100.net/docs/vt510-rm/LS.html
[single shifts]: https://vt100.net/docs/vt510-rm/SS.html
[`DOCS`]: https://en.wikipedia.org/wiki/ISO/IEC_2022#Interaction_with_other_coding_systems
## Detailed Description of the Pull Request / Additional comments
To make it easier for us to declare a bunch of character sets, I've made
a little `constexpr` class that can build up a mapping table from a base
character set (ASCII or Latin1), along with a collection of mappings for
the characters the deviate from the base set. Many of the character sets
are simple variations of ASCII, so they're easy to define this way.
This class then casts directly to a `wstring_view` which is how the
translation tables are represented in most of the code. We have an array
of four of these tables representing the four G-sets, two instances for
the active left and right tables, and one instance for the single shift
table.
Initially we had just one `DesignateCharset` method, which could select
the active character set. We now have two designate methods (for 94- and
96- character sets), and each takes a G-set number specifying the target
of the designation, and a pair of characters identifying the character
set that will be designated (at the higher VT levels, character sets are
often identified by more than one character).
There are then two new `LockingShift` methods to invoke these G-sets
into either the GL or GR area of the code table, and a `SingleShift`
method which invokes a G-set temporarily (for just the next character
that is output).
I should mention here that I had to make some changes to the state
machine to make these single shift sequences work. The problem is that
the input state machine treats `SS3` as the start of a control sequence,
while the output state machine needs it to be dispatched immediately
(it's literally the _Single Shift 3_ escape sequence). To make that
work, I've added a `ParseControlSequenceAfterSs3` callback in the
`IStateMachineEngine` interface to decide which behavior is appropriate.
When it comes to mapping a character, it's simply an array reference
into the appropriate `wstring_view` table. If the single shift table is
set, that takes preference. Otherwise the GL table is used for
characters in the range 0x20 to 0x7F, and the GR table for characters
0xA0 to 0xFF (technically some character sets will only map up to 0x7E
and 0xFE, but that's easily controlled by the length of the
`wstring_view`).
The `DEL` character is a bit of a special case. By default it's meant to
be ignored like the `NUL` character (it's essentially a time-fill
character). However, it's possible that it could be remapped to a
printable character in a 96-character set, so we need to check for that
after the translation. This is handled in the `AdaptDispatch::Print`
method, so it doesn't interfere with the primary `PrintString` code
path.
The biggest problem with this whole process, though, is that the GR
mappings only really make sense if you have access to the raw output,
but by the time the output gets to us, it would already have been
translated to Unicode by the active code page. And in the case of UTF-8,
the characters we eventually receive may originally have been composed
from two or more code points.
The way I've dealt with this was to disable the GR translations by
default, and then added support for a pair of ISO-2022 `DOCS` sequences,
which can switch the code page between UTF-8 and ISO-8859-1. When the
code page is ISO-8859-1, we're essentially receiving the raw output
bytes, so it's safe to enable the GR translations. This is not strictly
correct ISO-2022 behavior, and there are edge cases where it's not going
to work, but it's the best solution I could come up with.
## Validation Steps Performed
As a result of the `SS3` changes in the state machine engine, I've had
to move the existing `SS3` tests from the `OutputEngineTest` to the
`InputEngineTest`, otherwise they would now fail (technically they
should never have been output tests).
I've added no additional unit tests, but I have done a lot of manual
testing, and made sure we passed all the character set tests in Vttest
(at least for the character sets we currently support). Note that this
required a slightly hacked version of the app, since by default it
doesn't expose a lot of the test to low-level terminals, and we
currently identify as a VT100.
Closes#3377Closes#3487
## Summary of the Pull Request
This is the plan that @miniksa suggested to me. Instead of trying to do lots of work in all the renderers to do backgrounds as one pass, and foregrounds as another, we can localize this change to basically just the DX renderer.
1. First, we give the DX engine a "heads up" on where the cursor is going to be drawn during the frame, in `PrepareRenderInfo`.
- This function is left unimplemented in the other render engines.
2. While printing runs of text, the DX renderer will try to paint the cursor in `CustomTextRenderer::DrawGlyphRun` INSTEAD of `DxEngine::PaintCursor`. This lets us weave the cursor background between the text background and the text.
## References
* #6151 was a spec in this general area. I should probably go back and update it, and we should probably approve that first.
* #6193 is also right up in this mess
## PR Checklist
* [x] Closes#1203
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
* This is essentially `"cursorTextColor": "textForeground"` from #6151.
* A follow up work item is needed to add support for the current behavior, (`"cursorTextColor": null`), and hooking up that setting to the renderer.
## Summary of the Pull Request
Restores proper line drawing during IME operations in `conhost`
## PR Checklist
* [x] Closes#803
* [x] I work here.
* [x] Tested manually.
* [x] Check the performance of this and see if it's worse-enough to merit a more confusing algorithm. It was worse for the majority case so I scoped it.
* [x] No doc, it should have worked this way.
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
- Changed `ConsoleImeInfo::s_ConvertToCells` to be less confusing. It's doing about the same thing, but it's way easier to read now and the compiler/linker/optimizer should just be the same.
- Edited `Renderer::_PaintBufferOutputHelper` to check each attribute for line drawing characters as the right half of a two-col character might have different line drawing characters than the left-half.
## Validation Steps Performed
- [x] Manual operation of IME in conhost with Japanese IME.
- [x] Manual operation of IME in conhost with Chinese IME.
- [x] Manual operation of IME in conhost with Chinese (Traditional) IME.
- [x] Manual operation of IME in conhost with and Korean IME. - @leonMSFT says Korean doesn't work this way. But Korean is broken worse in that it's not showing suggestions at all. Filing new bug. #6227
- [x] Validated against API-filling calls through `SetConsoleTextAttribute` per @j4james's sample code
## Summary of the Pull Request
This PR adds support for the core VT52 commands, and implements the `DECANM` private mode sequence, which switches the terminal between ANSI mode and VT52-compatible mode.
## References
PR #2017 defined the initial specification for VT52 support.
PR #4044 removed the original VT52 cursor ops that conflicted with VT100 sequences.
## PR Checklist
* [x] Closes#976
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #2017
## Detailed Description of the Pull Request / Additional comments
Most of the work involves updates to the parsing state machine, which behaves differently in VT52 mode. `CSI`, `OSC`, and `SS3` sequences are not applicable, and there is one special-case escape sequence (_Direct Cursor Address_), which requires an additional state to handle parameters that come _after_ the final character.
Once the parsing is handled though, it's mostly just a matter of dispatching the commands to existing methods in the `ITermDispatch` interface. Only one new method was required in the interface to handle the _Identify_ command.
The only real new functionality is in the `TerminalInput` class, which needs to generate different escape sequences for certain keys in VT52 mode. This does not yet support _all_ of the VT52 key sequences, because the VT100 support is itself not yet complete. But the basics are in place, and I think the rest is best left for a follow-up issue, and potentially a refactor of the `TerminalInput` class.
I should point out that the original spec called for a new _Graphic Mode_ character set, but I've since discovered that the VT terminals that _emulate_ VT52 just use the existing VT100 _Special Graphics_ set, so that is really what we should be doing too. We can always consider adding the VT52 graphic set as a option later, if there is demand for strict VT52 compatibility.
## Validation Steps Performed
I've added state machine and adapter tests to confirm that the `DECANM` mode changing sequences are correctly dispatched and forwarded to the `ConGetSet` handler. I've also added state machine tests that confirm the VT52 escape sequences are dispatched correctly when the ANSI mode is reset.
For fuzzing support, I've extended the VT command fuzzer to generate the different kinds of VT52 sequences, as well as mode change sequences to switch between the ANSI and VT52 modes.
In terms of manual testing, I've confirmed that the _Test of VT52 mode_ in Vttest now works as expected.
This PR introduces a new `ColorType` to allow us to distinguish between
`SGR` indexed colors from the 16 color table, the lower half of which
can be brightened, and the ISO/ITU indexed colors from the 256 color
table, which have a fixed brightness. Retaining the distinction between
these two types will enable us to forward the correct `SGR` sequences to
conpty when addressing issue #2661.
The other benefit of retaining the color index (which we didn't
previously do for ISO/ITU colors) is that it ensures that the colors are
updated correctly when the color scheme is changed.
## References
* This is another step towards fixing the conpty narrowing bugs in issue
#2661.
* This is technically a fix for issue #5384, but that won't be apparent
until #2661 is complete.
## PR Checklist
* [x] Closes#1223
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
The first part of this PR was the introduction of a new `ColorType` in
the `TextColor` class. Instead of just the one `IsIndex` type, there is
now an `IsIndex16` and an `IsIndex256`. `IsIndex16` covers the eight
original ANSI colors set with `SGR 3x` and `SGR 4x`, as well as the
brighter aixterm variants set with `SGR 9x` and `SGR 10x`. `IsIndex256`
covers the 256 ISO/ITU indexed colors set with `SGR 38;5` and `SGR
48;5`.
There are two reasons for this distinction. The first is that the ANSI
colors have the potential to be brightened by the `SGR 1` bold
attribute, while the ISO/ITO color do not. The second reason is that
when forwarding an attributes through conpty, we want to try and
preserve the original SGR sequence that generated each color (to the
extent that that is possible). By having the two separate types, we can
map the `IsIndex16` colors back to ANSI/aixterm values, and `IsIndex256`
to the ISO/ITU sequences.
In addition to the VT colors, we also have to deal with the legacy
colors set by the Windows console APIs, but we don't really need a
separate type for those. It seemed most appropriate to me to store them
as `IsIndex256` colors, since it doesn't make sense to have them
brightened by the `SGR 1` attribute (which is what would happen if they
were stored as `IsIndex16`). If a console app wanted a bright color it
would have selected one, so we shouldn't be messing with that choice.
The second part of the PR was the unification of the two color tables.
Originally we had a 16 color table for the legacy colors, and a separate
table for the 256 ISO/ITU colors. These have now been merged into one,
so color table lookups no longer need to decide which of the two tables
they should be referencing. I've also updated all the methods that took
a color table as a parameter to use a `basic_string_view` instead of
separate pointer and length variables, which I think makes them a lot
easier and safer to work with.
With this new architecture in place, I could now update the
`AdaptDispatch` SGR implementation to store the ISO/ITU indexed colors
as `IsIndex256` values, where before they were mapped to RGB values
(which prevented them reflecting any color scheme changes). I could also
update the `TerminalDispatch` implementation to differentiate between
the two index types, so that the `SGR 1` brightening would only be
applied to the ANSI colors.
I've also done a bit of code refactoring to try and minimise any direct
access to the color tables, getting rid of a lot of places that were
copying tables with `memmove` operations. I'm hoping this will make it
easier for us to update the code in the future if we want to reorder the
table entries (which is likely a requirement for unifying the
`AdaptDispatch` and `TerminalDispatch` implementations).
## Validation Steps Performed
For testing, I've just updated the existing unit tests to account for
the API changes. The `TextColorTests` required an extra parameter
specifying the index type when setting an index. And the `AdapterTest`
and `ScreenBufferTests` required the use of the new `SetIndexedXXX`
methods in order to be explicit about the index type, instead of relying
on the `TextAttribute` constructor and the old `SetForeground` and
`SetBackground` methods which didn't have a way to differentiate index
types.
I've manually tested the various console APIs
(`SetConsoleTextAttribute`, `ReadConsoleOutputAttribute`, and
`ReadConsoleOutput`), to make sure they are still setting and reading
the attributes as well as they used to. And I've tested the
`SetConsoleScreenBufferInfoEx` and `GetConsoleScreenBufferInfoEx` APIs
to make sure they can read and write the color table correctly. I've
also tested the color table in the properties dialog, made sure it was
saved and restored from the registry correctly, and similarly saved and
restored from a shortcut link.
Note that there are still a bunch of issues with the color table APIs,
but no new problems have been introduced by the changes in this PR, as
far as I could tell.
I've also done a bunch of manual tests of `OSC 4` to make sure it's
updating all the colors correctly (at least in conhost), and confirmed
that the test case in issue #1223 now works as expected.
## Summary of the Pull Request
When using an _Input Method Editor_ in conhost for East Asian languages, the text cursor is temporarily hidden while the characters are being composed. When the composition is complete, the cursor visibility is meant to be restored, but that doesn't always happen if the IME composition is cancelled. This PR makes sure the cursor visibility is always restored, regardless of how the IME is closed.
## PR Checklist
* [x] Closes#810
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.
## Detailed Description of the Pull Request / Additional comments
The original implementation hid the cursor whenever `ConsoleImeInfo::WriteCompMessage` was called (which could be multiple times in the course of a composition), and then only restored the visibility when `ConsoleImeInfo::WriteResultMessage` was called. If a composition is cancelled, though, `WriteResultMessage` would never be called, so the cursor visibility wouldn't be restored.
I've now made the `SaveCursorVisibility` and `RestoreCursorVisibility` methods public, so they can instead be called from the `ImeStartComposition` and `ImeEndComposition` functions. This makes sure `RestoreCursorVisibility` is always called, regardless of how the composition ended, and `SaveCursorVisibility` is only called once at the start of the composition (which isn't essential, but seems cleaner to me).
## Validation Steps Performed
I've manually tested opening and closing the IME, both while submitting characters and while cancelling a composition, and in all cases the cursor visibility was correctly restored.
This is an attempt to simplify the SGR (Select Graphic Rendition)
implementation in conhost, to cut down on the number of methods required
in the `ConGetSet` interface, and pave the way for future improvements
and bug fixes. It already fixes one bug that prevented SGR 0 from being
correctly applied when combined with meta attributes.
* This a first step towards fixing the conpty narrowing bugs in issue
#2661
* I'm hoping the simplification of `ConGetSet` will also help with
#3849.
* Some of the `TextAttribute` refactoring in this PR overlaps with
similar work in PR #1978.
## Detailed Description of the Pull Request / Additional comments
The main point of this PR was to simplify the
`AdaptDispatch::SetGraphicsRendition` implementation. So instead of
having it call a half a dozen methods in the `ConGetSet` API, depending
on what kinds of attributes needed to be set, there is now just one call
to get current attributes, and another call to set the new value. All
adjustments to the attributes are made in the `AdaptDispatch` class, in
a simple switch statement.
To help with this refactoring, I also made some change to the
`TextAttribute` class to make it easier to work with. This included
adding a set of methods for setting (and getting) the individual
attribute flags, instead of having the calling code being exposed to the
internal attribute structures and messing with bit manipulation. I've
tried to get rid of any methods that were directly setting legacy, meta,
and extended attributes.
Other than the fix to the `SGR 0` bug, the `AdaptDispatch` refactoring
mostly follows the behaviour of the original code. In particular, it
still maps the `SGR 38/48` indexed colors to RGB instead of retaining
the index, which is what we ultimately need it to do. Fixing that will
first require the color tables to be unified (issue #1223), which I'm
hoping to address in a followup PR.
But for now, mapping the indexed colors to RGB values required adding an
an additional `ConGetSet` API to lookup the color table entries. In the
future that won't be necessary, but the API will still be useful for
other color reporting operations that we may want to support. I've made
this API, and the existing setter, standardise on index values being in
the "Xterm" order, since that'll be essential for unifying the code with
the terminal adapter one day.
I should also point out one minor change to the `SGR 38/48` behavior,
which is that out-of-range RGB colors are now ignored rather than being
clamped, since that matches the way Xterm works.
## Validation Steps Performed
This refactoring has obviously required corresponding changes to the
unit tests, but most were just minor updates to use the new
`TextAttribute` methods without any real change in behavior. However,
the adapter tests did require significant changes to accommodate the new
`ConGetSet` API. The basic structure of the tests remain the same, but
the simpler API has meant fewer values needed to be checked in each test
case. I think they are all still covering the areas there were intended
to, though, and they are all still passing.
Other than getting the unit tests to work, I've also done a bunch of
manual testing of my own. I've made sure the color tests in Vttest all
still work as well as they used to. And I've confirmed that the test
case from issue #5341 is now working correctly.
Closes#5341
[Git2Git] Merged PR 4644345: conhost: disable the DX renderer in inbox builds
We're going to be taking on some changes to the Dx renderer that are at
the very least annoying and at the very most inconsequential to the
inbox console. This commit removes support for the DX renderer from the
inbox console.
SizeBench reports that ConRenderDx contributes 55.1kb to the conhost
image (as its third largest constituent library), so this should net us
a couple pleasant WPG improvements down the line.
Related work items: #26291552 Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp 6e36786d447b7975298ba31ccd77c5c649fbfbe6
Related work items: #26291552
[Git2Git] Git Train: Merge of building/rs_onecore_dep_uxp/200504-1008 into official/rs_onecore_dep_uxp Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp bbdf04608ba96c3f8ee06cf100428cde01f3df79
Related work items: #26071826
The Erase All VT sequence (`^[[2J`) is supposed to erase the entire
contents of the viewport. The way it usually does this is by shifting
the entirety of the viewport contents into scrollback, and starting the
new viewport below it.
Currently, conpty doesn't propagate that state change correctly. When
conpty gets a 2J, it simply erases the content of the connected
terminal's viewport, by writing over it with spaces. Conpty didn't
really have a good way of communicating "your viewport should move", it
only knew "the buffer is now full of spaces".
This would lead to bugs like #2832, where pressing <kbd>ctrl+L</kbd> in
`bash` would delete the current contents of the viewport, instead of
moving the viewport down.
This PR makes sure that when conpty sees a 2J, it passes that through
directly to the connected terminal application as well. Fortunately, 2J
was already implemented in the Windows Terminal, so this actually fixes
the behavior of <kbd>ctrl+L</kbd>/`clear` in WSL in the Terminal.
## References
* #4252 - right now this isn't the _most_ optimal scenario, we're
literally just printing a 2J, then we'll perform "erase line" `height`
times. The erase line operations are all redundant at this point - the
entire viewport is blank, but conpty doesn't really know that.
Fortunately, #4252 was already filed for me to come through and
optimize this path.
## PR Checklist
* [x] Closes#2832
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* ran tests
* compared <kbd>ctrl+L</kbd> with its behavior in conhost
* compared `clear` with its behavior in conhost
## Summary of the Pull Request
This PR implements a pair of shims for `cmd` and `powershell`, so that their `cls` and `Clear-Host` functions will clear the entire terminal buffer (like they do in conhost), instead of just the viewport. With the conpty viewport and buffer being the same size, there's effectively no way to know if an application is calling these API's in this way with the intention of clearing the buffer or the viewport. We absolutely have to guess.
Each of these shims checks to see if the way that the API is being called exactly matches the way `cmd` or `powershell` would call these APIs. If it does, we manually write a `^[[3J` to the connected terminal, to get he Terminal to clear it's own scrollback.
~~_⚠️ If another application were trying to clear the **viewport** with an exactly similar API call, this would also cause the terminal scrollback to get cleared ⚠️_~~
* [x] Should these shims be restricted to when the process that's calling them is actually `cmd.exe` or `powershell.exe`? Can I even do this? I think we've done such a good job of isolating the client process information from the rest of the host code that I can't figure out how to do this.
- YES, this can be done, and I did it.
* [ ] **TODO**: _While I'm here_, should I have `DoSrvPrivateEraseAll` (the implementation for `^[[2J`, in `getset.cpp`) also manually trigger a EraseAll in the terminal in conpty mode?
## PR Checklist
* [x] Closes#3126
* [x] Actually closes#1305 too, which is really the same thing, but probably deserves a callout
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* ran tests
* checked `cls` in the Terminal
* checked `Clear-Host` in the Terminal
* Checked running `powershell clear-host` from `cmd.exe`
If a class has a constructor which can be called with a single argument,
then this constructor becomes conversion constructor because such a
constructor allows conversion of the single argument to the class being
constructed. To ensure these constructors are passed the argument of its
type, I labeled them explicit.
In some header files, there were constructors that took a value that
could involve implicit conversions, so I added explicit to ensure that
does not happen.
Hide any commandline (cooked read) we have before we begin a resize, and
show it again after the resize.
## References
* I found #5618 while I was working on this.
## PR Checklist
* [x] Closes#1856
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Basically, during a resize, we try to restore the viewport position
correctly, and part of that checks where the current commandline ends.
However, when we do that, the commandline's _current_ state still
reflects the _old_ buffer size, so resizing to be smaller can cause us
to throw an exception, when we find that the commandline doesn't fit in
the new viewport cleanly.
By hiding it, then redrawing it, we avoid this problem entirely. We
don't need to perform the check on the old commandline contents (since
they'll be empty), and we'll redraw it just fine for the new buffer size
## Validation Steps Performed
* ran tests
* checked resizing, snapping in conhost with a cooked read
* checked resizing, snapping in the Terminal with a cooked read
## Summary of the Pull Request
- Adjusts scaling practices in `DxEngine` (and related scaling practices in `TerminalControl`) for pixel-perfect row baselines and spacing at High DPI such that differential row-by-row rendering can be applied at High DPI.
## References
- #5185
## PR Checklist
* [x] Closes#5320, closes#3515, closes#1064
* [x] I work here.
* [x] Manually tested.
* [x] No doc.
* [x] Am core contributor. Also discussed with some of them already via Teams.
## Detailed Description of the Pull Request / Additional comments
**WAS:**
- We were using implicit DPI scaling on the `ID2D1RenderTarget` and running all of our processing in DIPs (Device-Independent Pixels). That's all well and good for getting things bootstrapped quickly, but it leaves the actual scaling of the draw commands up to the discretion of the rendering target.
- When we don't get to explicitly choose exactly how many pixels tall/wide and our X/Y placement perfectly, the nature of floating point multiplication and division required to do the presentation can cause us to drift off slightly out of our control depending on what the final display resolution actually is.
- Differential drawing cannot work unless we can know the exact integer pixels that need to be copied/moved/preserved/replaced between frames to give to the `IDXGISwapChain1::Present1` method. If things spill into fractional pixels or the sizes of rows/columns vary as they are rounded up and down implicitly, then we cannot do the differential rendering.
**NOW:**
- When deciding on a font, the `DxEngine` will take the scale factor into account and adjust the proposed height of the requested font. Then the remainder of the existing code that adjusts the baseline and integer-ifies each character cell will run naturally from there. That code already works correctly to align the height at normal DPI and scale out the font heights and advances to take an exact integer of pixels.
- `TermControl` has to use the scale now, in some places, and stop scaling in other places. This has to do with how the target's nature used to be implicit and is now explicit. For instance, determining where the cursor click hits must be scaled now. And determining the pixel size of the display canvas must no longer be scaled.
- `DxEngine` will no longer attempt to scale the invalid regions per my attempts in #5185 because the cell size is scaled. So it should work the same as at 96 DPI.
- The block is removed from the `DxEngine` that was causing a full invalidate on every frame at High DPI.
- A TODO was removed from `TermControl` that was invalidating everything when the DPI changed because the underlying renderer will already do that.
## Validation Steps Performed
* [x] Check at 150% DPI. Print text, scroll text down and up, do selection.
* [x] Check at 100% DPI. Print text, scroll text down and up, do selection.
* [x] Span two different DPI monitors and drag between them.
* [x] Giant pile of tests in https://github.com/microsoft/terminal/pull/5345#issuecomment-614127648
Co-authored-by: Dustin Howett <duhowett@microsoft.com>
Co-authored-by: Mike Griese <migrie@microsoft.com>
Improve wide glyph support in UIA (GH-4946)
Add enhanced key support for ConPty (GH-5021)
Set DxRenderer non-text alias mode (GH-5149)
Reduce CursorChanged Events for Accessibility (GH-5196)
Add more object ID tracing for Accessibility (GH-5215)
Add SS3 cursor key encoding to ConPty (GH-5383)
UIA: Prevent crash from invalid UTR endpoint comparison (GH-5399)
Make CodepointWidthDetector::GetWidth faster (CC-3727)
add til::math, use it for float conversions to point, size (GH-5150)
Add support for renderer backoff, don't FAIL_FAST on 3x failures, add UI (GH-5353)
Fix a deadlock and a bounding rects issue in UIA (GH-5385)
Don't duplicate spaces from potentially-wrapped EOL-deferred lines (GH-5398)
Reimplement the VT tab stop functionality (CC-5173)
Clamp parameter values to a maximum of 32767. (CC-5200)
Prevent the cursor type being reset when changing the visibility (CC-5251)
Make RIS switch back to the main buffer (CC-5248)
Add support for the DSR-OS operating status report (CC-5300)
Update the virtual bottom location if the cursor moves below it (CC-5317)
ci: run spell check in CI, fix remaining issues (CC-4799) (CC-5352)
Set Cascadia Code as default font (GH-5121)
Show a double width cursor for double width characters (GH-5319)
Delegate all character input to the character event handler (CC-4192)
Update til::bitmap to use dynamic_bitset<> + libpopcnt (GH-5092)
Merged PR 4465022: [Git2Git] Merged PR 4464559: Console: Ingest OSS changes up to e0550798
Correct scrolling invalidation region for tmux in pty w/ bitmap (GH-5122)
Render row-by-row instead of invalidating entire screen (GH-5185)
Make conechokey use ReadConsoleInputW by default (GH-5148)
Manually pass mouse wheel messages to TermControls (GH-5131)
This fixes C-M-space for WSL but not for Win32, but I'm not sure there's a problem in Win32 quite yet. (GH-5208)
Fix copying wrapped lines by implementing better scrolling (GH-5181)
Emit lines wrapped due to spaces at the end correctly (GH-5294)
Remove unneeded whitespace (CC-5162)
[Git2Git] Git Train: Merge of building/rs_onecore_dep_uxp/200414-1630 into official/rs_onecore_dep_uxp Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp 90031afa8114b7d95e3993573fc8449a1524a7fd
Related work items: #25439646
# Summary of the Pull Request
This PR will allow the cursor to be double width when on top of a double width character. This required changing `IsCursorDoubleWidth` to check whether the glyph the cursor's on top of is double width. This code is exactly the same as the original PR that addressed this issue in #2932. That one got reverted at some point due to the crashes related to it, but due to a combination of Terminal having come further since that PR and other changes to address use-after-frees, some of the crashes may/may not be relevant now. The ones that seemed to be relevant/repro-able, I attempt to address in this PR.
The `IsCursorDoubleWidth` check would fail during the `TextBuffer::Reflow` call inside of `Terminal::UserResize` occasionally, particularly when `newCursor.EndDeferDrawing()` is called. This is because when we tell the newCursor to `EndDefer`, the renderer will attempt to redraw the cursor. As part of this redraw, it'll ask if `IsCursorDoubleWidth`, and if the renderer managed to ask this before `UserResize` swapped out the old buffer with the new one from `Reflow`, the renderer will be asking the old buffer if its out-of-bounds cursor is double width. This was pretty easily repro'd using `cmatrix -u0` and resizing the window like a madman.
As a solution, I've moved the Start/End DeferDrawing calls out of `Reflow` and into `UserResize`. This way, I can "clamp" the portion of the code where the newBuffer is getting created and reflowed and swapped into the Terminal buffer, and only allow the renderer to draw once the swap is done. This also means that ConHost's `ResizeWithReflow` needed to change slightly.
In addition, I've added a WriteLock to `SetCursorOn`. It was mentioned as a fix for a crash in #2965 (although I can't repro), and I also figured it would be good to try to emulate where ConHost locks with regards to Cursor operations, and this seemed to be one that we were missing.
## PR Checklist
* [x] Closes#2713
* [x] CLA signed
* [x] Tests added/passed
## Validation Steps Performed
Manual validation that the cursor is indeed chonky, added a test case to check that we are correctly saying that the cursor is double width (not too sure if I put it in the right place). Also open to other test case ideas and thoughts on what else I should be careful for since I am quite nervous about what other crashes might occur.
## Summary of the Pull Request
When WSL vim prints the initial empty buffer (the one that's just a bunch of '\~'s), it prints this by doing the following:
* Print '\~' followed by enough spaces to clear the line
* Use CUP (`^[[H`) to move the cursor to the start of the next line
* repeat until the buffer is full
When we'd get the line of "\~ "... in conhost, we'd mark that line as wrapped.
Logically, it doesn't really make any sense that when we follow that up by moving the cursor, the line is wrapped. However, this is just how conhost is right now.
This wasn't ever a problem in just conhost before, because we really didn't care if lines in the alt buffer were "wrapped" or not. Plus, when vim would get resized, it would just reprint it's own buffer anyways. Nor was this a problem in conpty before this year (2020). We've only just recently added logic to conpty to try and preserve wrapped lines.
Initially, I tried fixing this by breaking the line manually when the cursor was moved. This seemed to work great, except for the win32 vim.exe. Vim.exe doesn't emit a newline or a CUP to get to the next line. It just _goes for it_ and keeps printing. So there's _no way_ for us to know the line broke, because they're essentially just printing one long line, assuming we'll automatically move the cursor.
So instead, I'm making sure to emit the proper number of spaces at the end of a line when the line is wrapped. We won't do any funny business in that scenario and try to optimize for them, we'll _just print the spaces_.
## References
* #5181 - This change regressed this
* #4415 - Actually implemented wrapped lines in conpty
## PR Checklist
* [x] Closes#5291
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Wrote a unittest first and foremost
* Checked vtpipeterm to make sure vim still works
* checked Terminal to make sure vim still works
If an application writes to the screen while not in VT mode, and the
user has scrolled forward in the screen buffer, the _virtual bottom_
location is not updated to take that new content into account. As a
result, the viewport can later jump back to the previous _virtual
bottom_, making the content disappear off screen. This PR attempts to
fix that issue by updating the _virtual bottom_ location whenever the
cursor moves below that point.
## PR Checklist
* [x] CLA signed.
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
This simply adds a condition in the
`SCREEN_INFORMATION::SetCursorPosition` to check if the new _Y_
coordinate is below the current _virtual bottom_, and if so, updates the
_virtual bottom_ to that new value.
I considered trying to make it only update when something is actually
written to the screen, but this seemed like a cleaner solution, and is
less likely to miss out on a needed update.
## Validation Steps Performed
I've manually tested the case described in issue #5302, and confirmed
that it now works as expected. I've also added a unit test that checks
the virtual bottom is updated correctly under similar conditions.
Closes#5302
Now that the Terminal is doing a better job of actually marking which
lines were and were not wrapped, we're not always copying lines as
"wrapped" when they should be. We're more correctly marking lines as not
wrapped, when previously we'd leave them marked wrapped.
The real problem is here in the `ScrollFrame` method - we'd manually
newline the cursor to make the terminal's viewport shift down to a new
line. If we had to scroll the viewport for a _wrapped_ line, this would
cause the Terminal to mark that line as broken, because conpty would
emit an extra `\n` that didn't actually exist.
This more correctly implements `ScrollFrame`. Now, well move where we
"thought" the cursor was, so when we get to the next `PaintBufferLine`,
if the cursor needs to newline for the next line, it'll newline, but if
we're in the middle of a wrapped line, we'll just keep printing the
wrapped line.
A couple follow up bugs were found to be caused by the same bad logic.
See #5039 and #5161 for more details on the investigations there.
## References
* #4741 RwR, which probably made this worse
* #5122, which I branched off of
* #1245, #357 - a pair of other conpty wrapped lines bugs
* #5228 - A followup issue for this PR
## PR Checklist
* [x] Closes#5113
* [x] Closes#5180 (by fixing DECRST 25)
* [x] Closes#5039
* [x] Closes#5161 (by ensuring we only `removeSpaces` on the actual
bottom line)
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Checked the cases from #1245, #357 to validate that they still work
* Added more and more tests for these scenarios, and then I added MORE
tests
* The entire team played with this in selfhost builds
## Summary of the Pull Request
If we receive a _Reset to Initial State_ (`RIS`) sequence while in the alternate screen buffer, we should be switching back to the main buffer. This PR fixes that behavior.
## PR Checklist
* [x] Closes#3685
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
I've added a condition at the start of the `AdaptDispatch::HardReset` method to check whether we're using the alt buffer, and if so, call the `ConGetSet::PrivateUseMainScreenBuffer` API to switch back to the main buffer.
Calling `AdaptDispatch::UseMainScreenBuffer` would probably be neater for this, but it would also attempt to restore the cursor state, which seems pointless when we're in the process of resetting everything anyway.
## Validation Steps Performed
I've added a screen buffer test to confirm that the `RIS` sequence does actually switch back to the main buffer. I've also manually confirmed that the test case in issue #3685 does now behave as expected.
A side effect of the `SetConsoleCursorInfo` API is that it resets the
cursor type to _Legacy_. This makes it impossible to change the cursor
visibility via the console APIs without also resetting the user's
preferred cursor type. This PR attempts to fix that limitation, by only
resetting the cursor type if the size has also been changed.
## PR Checklist
* [x] Closes#4124
* [x] CLA signed
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I suspect the reason for the original behaviour was because the
`SetConsoleCursorInfo` API sets both the visibility and the size, and if
you're setting the size, it's assumed you'd want the _Legacy_ cursor
type, because that's the only style for which the size is applicable.
So my solution was to only reset the cursor type if the requested cursor
size was actually different from the current size. That should be
reasonably backwards compatible with most size-changing code, but also
allow for changing the visibility without resetting the cursor type.
## Validation Steps Performed
I've tested the example code from issue #4124, and confirmed that it now
works correctly without resetting the cursor type.
I've also tested the console's _mark mode_, which temporarily changes
the cursor size while selecting. I've confirmed that the size still
changes, and that the original cursor type is restored afterwards.
## Summary of the Pull Request
This is essentially a rewrite of the VT tab stop functionality, implemented entirely within the `AdaptDispatch` class. This significantly simplifies the `ConGetSet` interface, and should hopefully make it easier to share the functionality with the Windows Terminal VT implementation in the future.
By removing the dependence on the `SCREEN_INFORMATION` class, it fixes the problem of the the tab state not being preserved when switching between the main and alternate buffers. And the new architecture also fixes problems with the tabs not being correctly initialized when the screen is resized.
## References
This fixes one aspect of issue #3545.
It also supersedes the fix for #411 (PR #2816).
I'm hoping the simplification of `ConGetSet` will help with #3849.
## PR Checklist
* [x] Closes#4669
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
In the new tab architecture, there is now a `vector<bool>` (__tabStopColumns_), which tracks whether any particular column is a tab stop or not. There is also a __initDefaultTabStops_ flag indicating whether the default tab stop positions need to be initialised when the screen is resized.
The way this works, the vector is initially empty, and only initialized (to the current width of the screen) when it needs to be used. When the vector grows in size, the __initDefaultTabStops_ flag determines whether the new columns are set to false, or if every 8th column is set to true.
By default we want the latter behaviour - newly revealed columns should have default tab stops assigned to them - so __initDefaultTabStops_ is set to true. However, after a `TBC 3` operation (i.e. we've cleared all tab stops), there should be no tab stops in any newly revealed columns, so __initDefaultTabStops_ is set to false.
Note that the __tabStopColumns_ vector is never made smaller when the window is shrunk, and that way it can preserve the state of tab stops that are off screen, but which may come into range if the window is made bigger again.
However, we can can still reset the vector completely after an `RIS` or `TBC 3` operation, since the state can then be reconstructed automatically based on just the __initDefaultTabStops_ flag.
## Validation Steps Performed
The original screen buffer tests had to be rewritten to set and query the tab stop state using escape sequences rather than interacting with the `SCREEN_INFORMATION` class directly, but otherwise the structure of most tests remained largely the same.
However, the alt buffer test was significantly rewritten, since the original behaviour was incorrect, and the initialization test was dropped completely, since it was no longer applicable. The adapter tests were also dropped, since they were testing the `ConGetSet` interface which has now been removed.
I also had to make an addition to the method setup of the screen buffer tests (making sure the viewport was appropriately initialized), since there were some tests (unrelated to tab stops) that were previously dependent on the state being set in the tab initialization test which has now been removed.
I've manually tested the issue described in #4669 and confirmed that the tabs now produce the correct spacing after a resize.
<!-- 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
Every single time a PR is run, there are a bunch of warnings about whitespace in the .cs files, so I ran the code format on those files, without changing their contents, so it won't be flagged anymore.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [X] Tests added/passed
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Ran the code-format utility on the .cs files
Correct scrolling invalidation region for tmux in pty w/ bitmap
Add tracing for circling and scrolling operations. Fix improper
invalidation within AdjustCursorPosition routine in the subsection about
scrolling down at the bottom with a set of margins enabled.
## References
- Introduced with #5024
## Detailed Description of the Pull Request / Additional comments
- This occurs when there is a scroll region restriction applied and a
newline operation is performed to attempt to spin the contents of just
the scroll region. This is a frequent behavior of tmux.
- Right now, the Terminal doesn't support any sort of "scroll content"
operation, so what happens here generally speaking is that the PTY in
the ConHost will repaint everything when this happens.
- The PTY when doing `AdjustCursorPosition` with a scroll region
restriction would do the following things:
1. Slide literally everything in the direction it needed to go to take
advantage of rotating the circular buffer. (This would force a
repaint in PTY as the PTY always forces repaint when the buffer
circles.)
2. Copy the lines that weren't supposed to move back to where they were
supposed to go.
3. Backfill the "revealed" region that encompasses what was supposed to
be the newline.
- The invalidations for the three operations above were:
1. Invalidate the number of rows of the delta at the top of the buffer
(this part was wrong)
2. Invalidate the lines that got copied back into position (probably
unnecessary, but OK)
3. Invalidate the revealed/filled-with-spaces line (this is good).
- When we were using a simple single rectangle for invalidation, the
union of the top row of the buffer from 1 and the bottom row of the
buffer from 2 (and 3 was irrelevant as it was already unioned it)
resulted in repainting the entire buffer and all was good.
- When we switched to a bitmap, it dutifully only repainted the top line
and the bottom two lines as the middle ones weren't a consequence of
intersect.
- The logic was wrong. We shouldn't be invalidating rows-from-the-top
for the amount of the delta. The 1 part should be invalidating
everything BUT the lines that were invalidated in parts 2 and 3.
(Arguably part 2 shouldn't be happening at all, but I'm not optimizing
for that right now.)
- So this solves it by restoring an entire screen repaint for this sort
of slide data operation by giving the correct number of invalidated
lines to the bitmap.
## Validation Steps Performed
- Manual validation with the steps described in #5104
- Automatic test `ConptyRoundtripTests::ScrollWithMargins`.
Closes#5104
This commit introduces a github action to check our spelling and fixes
the following misspelled words so that we come up green.
It also renames TfEditSes to TfEditSession, because Ses is not a word.
currently, excerpt, fallthrough, identified, occurred, propagate,
provided, rendered, resetting, separate, succeeded, successfully,
terminal, transferred, adheres, breaks, combining, preceded,
architecture, populated, previous, setter, visible, window, within,
appxmanifest, hyphen, control, offset, powerpoint, suppress, parsing,
prioritized, aforementioned, check in, build, filling, indices, layout,
mapping, trying, scroll, terabyte, vetoes, viewport, whose
## Summary of the Pull Request
- Added better wide glyph support for UIA. We used to move one _cell_ at a time, so wide glyphs would be read twice.
- Converted a few things to use til::point since I'm already here.
- fixed telemetry for UIA
## PR Checklist
* [x] Closes#1354
## Detailed Description of the Pull Request / Additional comments
The text buffer has a concept of word boundaries, so it makes sense to have a concept of glyph boundaries too.
_start and _end in UiaTextRange are now til::point
## Validation Steps Performed
Verified using Narrator
## Summary of the Pull Request
When I did my last PR that was merged, the PR #4960, there were two more cases I forgot to include, so I included them here, for the sake of consistency and completion
## References
PR #4690
## PR Checklist
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [X] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
Replacing pointer casts to 0 with nullptr in two tests.
## Validation Steps Performed
Manual Testing
Automated Testing
## Summary of the Pull Request
Moves the ConPTY drawing mechanism (`VtRenderer`) to use the fine-grained `til::bitmap` individual-dirty-bit tracking mechanism instead of coarse-grained rectangle unions to improve drawing performance by dramatically reducing the total area redrawn.
## PR Checklist
* [x] Part of #778 and #1064
* [x] I work here
* [x] Tests added and updated.
* [x] I'm a core contributor
## Detailed Description of the Pull Request / Additional comments
- Converted `GetDirtyArea()` interface from `IRenderEngine` to use a vector of `til::rectangle` instead of the `SMALL_RECT` to banhammer inclusive rectangles.
- `VtEngine` now holds and operates on the `til::bitmap` for invalidation regions. All invalidation operation functions that used to be embedded inside `VtEngine` are deleted in favor of using the ones in `til::bitmap`.
- Updated `VtEngine` tracing to use new `til::bitmap` on trace and the new `to_string()` methods detailed below.
- Comparison operators for `til::bitmap` and complementary tests.
- Fixed an issue where the dirty rectangle shortcut in `til::bitmap` was set to 0,0,0,0 by default which means that `|=` on it with each `set()` operation was stretching the rectangle from 0,0. Now it's a `std::optional` so it has no value after just being cleared and will build from whatever the first invalidated rectangle is. Complementary tests added.
- Optional run caching for `til::bitmap` in the `runs()` method since both VT and DX renderers will likely want to generate the set of runs at the beginning of a frame and refer to them over and over through that frame. Saves the iteration and creation and caches inside `til::bitmap` where the chance of invalidation of the underlying data is known best. It is still possible to iterate manually with `begin()` and `end()` from the outside without caching, if desired. Complementary tests added.
- WEX templates added for `til::bitmap` and used in tests.
- `translate()` method for `til::bitmap` which will slide the dirty points in the direction specified by a `til::point` and optionally back-fill the uncovered area as dirty. Complementary tests added.
- Moves all string generation for `til` types `size`, `point`, `rectangle`, and `some` into a `to_string` method on each object such that it can be used in both ETW tracing scenarios AND in the TAEF templates uniformly. Adds a similar method for `bitmap`.
- Add tagging to `_bitmap_const_iterator` such that it appears as a valid **Input Iterator** to STL collections and can be used in a `std::vector` constructor as a range. Adds and cleans up operators on this iterator to match the theoretical requirements for an **Input Iterator**. Complementary tests added.
- Add loose operators to `til` which will allow some basic math operations (+, -, *, /) between `til::size` and `til::point` and vice versa. Complementary tests added. Complementary tests added.
- Adds operators to `til::rectangle` to allow scaling with basic math operations (+, -, *) versus `til::size` and translation with basic math operations (+, -) against `til::point`. Complementary tests added.
- In-place variants of some operations added to assorted `til` objects. Complementary tests added.
- Update VT tests to compare invalidation against the new map structure instead of raw rectangles where possible.
## Validation Steps Performed
- Wrote additional til Unit Tests for all additional operators and functions added to the project to support this operation
- Updated the existing VT renderer tests
- Ran perf check
The first issue is in the console host: when we erase a command history,
we also clear its _allocated_ flag. It's supposed to remain allocated
but become "reset". When we later check that a command history that
exists in the list is allocated, we fail loudly because allocated has
been cleared.
The second is that in Windows Server 2003, we rewrote the console client
APIs (in kernelbase!) regarding command history and changed one internal
function from taking char** to taking char*. Since the signature was
_actually_ void** and that changed to void*, the compiler didn't notice
when in only one single place we continued to pass a char** instead of a
char*. This caused us to send the wrong filename length for the ExeName
in SetConsoleNumberOfCommands.
Fixes MSFT:25265854
Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp b493fb5a06975c53b2fbb7b9fc0546244b551fa9
## Summary of the Pull Request
Block selections were always read and displayed as line selections in UIA. This fixes that.
## PR Checklist
* [x] Closes#4509
## Detailed Description of the Pull Request / Additional comments
1. Expose `IsBlockSelection()` via IUiaData
2. Update the constructor to be able to take in a block selection parameter
3. Make ScreenInfoUiaProviders pass step 1 output into step 2 constructor
4. Update all instances of `UiaTextRange::GetTextRects()` to include this new flag
## Validation Steps Performed
Manually tested.
Additional tests would be redundant as GetTextRects() is tested in the text buffer.
When we painted spaces up until the character right before the right
edge of the screen, we would erroneously use Erase in Line instead of
Erase Character due to an off-by-one.
Fixes#4727
## Summary of the Pull Request
Currently, when the user resizes the Terminal, we'll snap the visible viewport back to the bottom of the buffer. This PR changes the visible viewport of the Terminal to instead remain in the same relative location it was before the resize.
## References
Made possible by our sponsors at #4741, and listeners like you.
## PR Checklist
* [x] Closes#3494
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
We already hated the `std::optional<short>&` thing I yeet'd into #4741 right at the end to replace a `short*`. So I was already going to change that to a `std::optional<std::reference_wrapper<short>>`, which is more idomatic. But then I was looking through the list of bugs and #3494 caught my eye. I realized it would be trivial to not only track the top of the `mutableViewport` during a resize, but we could use the same code path to track the _visible_ viewport's start as well.
So basically I'm re-using that bit of code in `Reflow` to calculate the visible viewport's position too.
## Validation Steps Performed
Gotta love just resizing things all day, errday
cmatrix is somewhat of a pathological case for our infrastructure: it
prints out a bunch of green and white characters and then updates them a
million times a second.
It also maintains a column of space between every green character. When
it prints this column, it prints it in "default" or "white". This ends
up making runs of text that look like this:
(def: G=green B=bright white W=white *=matrix char =space)
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
As characters trickle in:
G*W G*W G*W G*W G*W G*W G*W B*W
G*W G*W G*W G*W G*W G*W G*W G W
G*W G*W G*W B*W G*W G*W G*W G W
G*W B*W G*W G W G*W G*W G*W G*W
G*W G W G*W G W G*W B*W G*W G*W
B*W G W G*W G W G*W G W B*W G*W
G W G W G*W G W G*W G W G W B*W
G W G W B*W G W G*W G W G W G W
Every one of those color transitions causes us to break up the run of
text and start rendering it again. This impacts GDI, Direct2D *and*
ConPTY. In the example above, there are 120 runs.
The problem is, printing a space doesn't **use** the foreground color!
This commit introduces an optimization. When we're about to break a text
cluster because its attributes changed, we make sure that it's not just
filled with spaces and doesn't differ in any visually-meaningful way
(like underline or strikethrough, considering global invert state).
This lets us optimize both the rendering _and_ the PTY output to look
like this:
G* * * * * * * B*G
G* * * * * * *
G* * * B*G * * *
G* B*G * * * * *
G* * * B*G * *
B*G * * B*G *
G * * B*G
G B*G *
Text will be printed at best line-by-line and at worst only when the
visible properties of the screen actually change. In the example
above, there are only 21 runs.
This speeds up cmatrix remarkably.
Refs #1064
This PR adds support for "Resize with Reflow" to the Terminal. In
conhost, `ResizeWithReflow` is the function that's responsible for
reflowing wrapped lines of text as the buffer gets resized. Now that
#4415 has merged, we can also implement this in the Terminal. Now, when
the Terminal is resized, it will reflow the lines of it's buffer in the
same way that conhost does. This means, the terminal will no longer chop
off the ends of lines as the buffer is too small to represent them.
As a happy side effect of this PR, it also fixed#3490. This was a bug
that plagued me during the investigation into this functionality. The
original #3490 PR, #4354, tried to fix this bug with some heavy conpty
changes. Turns out, that only made things worse, and far more
complicated. When I really got to thinking about it, I realized "conhost
can handle this right, why can't the Terminal?". Turns out, by adding
resize with reflow, I was also able to fix this at the same time.
Conhost does a little bit of math after reflowing to attempt to keep the
viewport in the same relative place after a reflow. By re-using that
logic in the Terminal, I was able to fix#3490.
I also included that big ole test from #3490, because everyone likes
adding 60 test cases in a PR.
## References
* #4200 - this scenario
* #405/#4415 - conpty emits wrapped lines, which was needed for this PR
* #4403 - delayed EOL wrapping via conpty, which was also needed for
this
* #4354 - we don't speak of this PR anymore
## PR Checklist
* [x] Closes#1465
* [x] Closes#3490
* [x] Closes#4771
* [x] Tests added/passed
## EDIT: Changes to this PR on 5 March 2020
I learned more since my original version of this PR. I wrote that in
January, and despite my notes that say it was totally working, it
_really_ wasn't.
Part of the hard problem, as mentioned in #3490, is that the Terminal
might request a resize to (W, H-1), and while conpty is preparing that
frame, or before the terminal has received that frame, the Terminal
resizes to (W, H-2). Now, there aren't enough lines in the terminal
buffer to catch all the lines that conpty is about to emit. When that
happens, lines get duplicated in the buffer. From a UX perspective, this
certainly looks a lot worse than a couple lost lines. It looks like
utter chaos.
So I've introduced a new mode to conpty to try and counteract this
behavior. This behavior I'm calling "quirky resize". The **TL;DR** of
quirky resize mode is that conpty won't emit the entire buffer on a
resize, and will trust that the terminal is prepared to reflow it's
buffer on it's own.
This will enable the quirky resize behavior for applications that are
prepared for it. The "quirky resize" is "don't `InvalidateAll` when the
terminal resizes". This is added as a quirk as to not regress other
terminal applications that aren't prepared for this behavior
(gnome-terminal, conhost in particular). For those kinds of terminals,
when the buffer is resized, it's just going to lose lines. That's what
currently happens for them.
When the quirk is enabled, conpty won't repaint the entire buffer. This
gets around the "duplicated lines" issue that requesting multiple
resizes in a row can cause. However, for these terminals that are
unprepared, the conpty cursor might end up in the wrong position after a
quirky resize.
The case in point is maximizing the terminal. For maximizing
(height->50) from a buffer that's 30 lines tall, with the cursor on
y=30, this is what happens:
* With the quirk disabled, conpty reprints the entire buffer. This is
60 lines that get printed. This ends up blowing away about 20 lines
of scrollback history, as the terminal app would have tried to keep
the text pinned to the bottom of the window. The term. app moved the
viewport up 20 lines, and then the 50 lines of conpty output (30
lines of text, and 20 blank lines at the bottom) overwrote the lines
from the scrollback. This is bad, but not immediately obvious, and
is **what currently happens**.
* With the quirk enabled, conpty doesn't emit any lines, but the
actual content of the window is still only in the top 30 lines.
However, the terminal app has still moved 20 lines down from the
scrollback back into the viewport. So the terminal's cursor is at
y=50 now, but conpty's is at 30. This means that the terminal and
conpty are out of sync, and there's not a good way of re-syncing
these. It's very possible (trivial in `powershell`) that the new
output will jump up to y=30 override the existing output in the
terminal buffer.
The Windows Terminal is already prepared for this quirky behavior, so it
doesn't keep the output at the bottom of the window. It shifts it's
viewport down to match what conpty things the buffer looks like.
What happens when we have passthrough mode and WT is like "I would like
quirky resize"? I guess things will just work fine, cause there won't be
a buffer behind the passthrough app that the terminal cares about. Sure,
in the passthrough case the Terminal could _not_ quirky resize, but the
quirky resize won't be wrong.
## Summary of the Pull Request
Move the contents and functionality of MouseInput from TerminalAdapter
to TerminalInput.
## References
#545 - VT Mouse Mode (Terminal)
#376 - VT Mouse Mode (ConPty)
## Detailed Description of the Pull Request / Additional comments
Pretty straightforward. The MouseInput class was a bit large though so I
split it up into a few files. This should make TerminalInput a bit
easier to manage.
- `mouseInputState`: enable some of the modes for mouse input. All saved
to `_mouseInputState`.
- `mouseInput`: basically just `HandleMouse()` and any helper functions
## Validation Steps Performed
Tests should still pass.
## Summary of the Pull Request
Allows VT engine methods that print formatted strings (cursor movements, color changes, etc.) to provide a guess at the max buffer size required eliminating the double-call for formatting in the common case.
## PR Checklist
* [x] Found while working on #778
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
- The most common case for VT rendering is formatting a few numbers into a sequence. For the most part, we can already tell the maximum length that the string could be based on the number of substitutions and the size of the parameters.
- The existing formatting method would always double-call. It would first call for how long the string was going to be post-formatting, allocate that memory, then call again and fill it up. This cost two full times of running through the string to find a length we probably already knew for the most part.
- Now if a size is provided, we allocate that first and attempt the "second pass" of formatting directly into the buffer. This saves the count step in the common case.
- If this fails, we fall back and do the two-pass method (which theoretically means the bad case is now 3 passes.)
- The next biggest waste of time in this method was allocating and freeing strings for every format pass. Due to the nature of the VT renderer, many things need to be formatted this way. I've now instead moved the format method to hold a static string that really only grows over the course of the session for all of these format operations. I expect a majority of the time, it will only be consuming approximately 5-15 length of a std::string of memory space. I cannot currently see a circumstance where it would use more than that, but I'm consciously trading memory usage when running as a PTY for overall runtime performance here.
## Validation Steps Performed
- Ran the thing manually and checked it out with wsl and cmatrix and Powershell and such attached to Terminal
- Wrote and ran automated tests on formatting method
This commit enables passthrough mode for VT Input Mode in ConPty. This
will be used to pass VT Input from Mouse Mode directly to the app on the
other side.
## References
#545 - VT Mouse Mode (Terminal)
#376 - VT Mouse Mode (ConPty)
## Detailed Description of the Pull Request / Additional comments
### ConHost
- Set the callback for the InputEngine.
- Retrieve `IsInVirtualTerminalInputMode` from the InputBuffer
### Adapter (Dispatch)
Retrieve `VTInputMode` setting from ConHost
### Parser
- Add a callback to passthrough unknown input sequences directly to the
input queue.
- If we're in VTInputMode, use the callback
## Validation Steps Performed
Tests should still pass.
## Summary of the Pull Request
`GetTextForClipboard` already exists in the TextBuffer. It makes sense to use that for UIA as well. This changes the behavior or `GetText()` such that it does not remove leading/trailing whitespace anymore. That is more of an expected behavior.
## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection anchors are an accurate representation of what is selected. We just need to move GetText to the TextBuffer. Then we can have those three issues just rely on code from the text buffer. This also means ConHost gets some of this stuff for free 😀
## Detailed Description of the Pull Request / Additional comments
- `TextBuffer::GetTextForClipboard()` --> `GetText()`
- `TextBuffer::GetText()` no longer requires GetForegroundColor/GetBackgroundColor. If either of these are not defined, we return a `TextAndColor` with only the `text` field populated.
- renamed a few parameters for copying text to the clipboard for clarity
- Updated `UiaTextRange::GetText()` to use `TextBuffer::GetText()`
## Validation Steps Performed
Manual tests for UIA using accessibility insights and Windows Terminal's copy action (w/ and w/out shift)
Added tests as well.
- When performing chunk selection, the expansion now occurs at the time
of the selection, not the rendering of the selection
- `GetSelectionRects()` was moved to the `TextBuffer` and is now shared
between ConHost and Windows Terminal
- Some of the selection variables were renamed for clarity
- Selection COORDs are now in the Text Buffer coordinate space
- Fixes an issue with Shift+Click after performing a Multi-Click
Selection
## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection
anchors are an accurate representation of what is selected. We just need
to move `GetText` to the `TextBuffer`. Then we can have those three
issues just rely on code from the text buffer. This also means ConHost
gets some of this stuff for free 😀
### TextBuffer
- `GetTextRects` is the abstracted form of `GetSelectionRects`
- `_ExpandTextRow` is still needed to handle wide glyphs properly
### Terminal
- Rename...
- `_boxSelection` --> `_blockSelection` for consistency with ConHost
- `_selectionAnchor` --> `_selectionStart` for consistency with UIA
- `_endSelectionPosition` --> `_selectionEnd` for consistency with
UIA
- Selection anchors are in Text Buffer coordinates now
- Really rely on `SetSelectionEnd` to accomplish appropriate chunk
selection and shift+click actions
## Validation Steps Performed
- Shift+Click
- Multi-Click --> Shift+Click
- Chunk Selection at...
- top of buffer
- bottom of buffer
- random region in scrollback
Closes#4465Closes#4547
## Summary of the Pull Request
Changes how conpty emits text to preserve line-wrap state, and additionally adds rudimentary support to the Windows Terminal for wrapped lines.
## References
* Does _not_ fix (!) #3088, but that might be lower down in conhost. This makes wt behave like conhost, so at least there's that
* Still needs a proper deferred EOL wrap implementation in #780, which is left as a todo
* #4200 is the mega bucket with all this work
* MSFT:16485846 was the first attempt at this task, which caused the regression MSFT:18123777 so we backed it out.
* #4403 - I made sure this worked with that PR before I even sent #4403
## PR Checklist
* [x] Closes#405
* [x] Closes#3367
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I started with the following implementation:
When conpty is about to write the last column, note that we wrapped this line here. If the next character the vt renderer is told to paint get is supposed to be at the start of the following line, then we know that the previous line had wrapped, so we _won't_ emit the usual `\r\n` here, and we'll just continue emitting text.
However, this isn't _exactly_ right - if someone fills the row _exactly_ with text, the information that's available to the vt renderer isn't enough to know for sure if this line broke or not. It is possible for the client to write a full line of text, with a `\n` at the end, to manually break the line. So, I had to also add the `lineWrapped` param to the `IRenderEngine` interface, which is about half the files in this changelist.
## Validation Steps Performed
* Ran tests
* Checked how the Windows Terminal behaves with these changes
* Made sure that conhost/inception and gnome-terminal both act as you'd expect with wrapped lines from conpty
## Summary of the Pull Request
Debugging our custom UIA providers has been a painful experience because outputting content to VS may result in UIA Clients getting impatient and giving up on extracting data.
Adding tracing allows us to debug these providers without getting in the way of reproducing a bug. This will help immensely with developing accessibility features on Windows Terminal and Console.
This pull request additionally contains payload from #4526:
* Make GetVisibleRanges() return one range (and add tracing for it).
`ScreenInfoUiaProvider::GetVisibleRanges()` used to return one range per line of visible text. The documentation for this function says that we should return one per contiguous span of text. Since all of the text in the TermControl will always be contiguous (at least by our standards), we should only ever be returning one range.
## PR Checklist
* [x] Closes#1914. Closes#4507.
* [x] CLA signed
## Detailed Description of the Pull Request / Additional comments
`UiaTracing` is a singleton class that is in charge of registration for trace logging. `TextRange` is used to trace `UiaTextRange`, whereas `TextProvider` is used to trace `ScreenInfoUiaProviderBase`.
`_getValue()` is overloaded to transform complex objects and enums into a string for logging.
`_getTextValue()` had to be added to be able to trace the text a UiaTextRange included. This makes following UiaTextRanges much simpler.
## Validation Steps Performed
Performed a few operations when under NVDA/Narrator and manually checked the results.
## Summary of the Pull Request
We used to return multiple text ranges to represent one selection. We only support one selection at a time, so we should only return one range.
Additionally, I moved all TriggerSelection() calls to the renderer from Terminal to TermControl for consistency. This ensures we only call it _once_ when we make a change to our selection state.
## References
#2447 - helps polish Signaling for Selection
#4465 - This is more apparent as the problem holding back Signaling for Selection
## PR Checklist
* [x] Closes#4452
Tested using Accessibility Insights.
## Summary of the Pull Request
Despite being specified as `noexcept`, `FillConsoleOutputCharacterA` emits an exception when a call to `ConvetToW` is made with an argument character which can't be converted. This PR fixes this throw, by wrapping `ConvertToW` in a try-catch_return.
## PR Checklist
* [x] Closes#4258
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed: thanks @miniksa
## Detailed Description of the Pull Request / Additional comments
Following the semantics of other `FillConsoleOutputCharacter*` the output param `cellsModified` is set to `0`. The try-catch_return is also what other functions of this family perform in case of errors.
## Validation Steps Performed
Original repro no longer crashes.
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
Moves the tests from using the `vstest.console.exe` route to just using `te.exe`.
PROs:
- `te.exe` is significantly faster for running tests because the TAEF/VSTest adapter isn't great.
- Running through `te.exe` is closer to what our developers are doing on their dev boxes
- `te.exe` is how they run in the Windows gates.
- `te.exe` doesn't seem to have the sporadic `0x6` error code thrown during the tests where somehow the console handles get lost
- `te.exe` doesn't seem to repro the other intermittent issues that we have been having that are inscrutable.
- Fewer processes in the tree (te is running anyway under `vstest.console.exe`, just indirected a lot
- The log outputs scroll live with all our logging messages instead of suppressing everything until there's a failure
- The log output is actually in the order things are happening versus vstest.
CONs:
- No more code coverage.
- No more test records in the ADO build/test panel.
- Tests really won't work inside Visual Studio at all.
- The log files are really big now
- Testing is not a test task anymore, just another script.
Refuting each CON:
- We didn't read the code coverage numbers
- We didn't look at the ADO test panel results or build-over-build velocities
- Tests didn't really work inside Visual Studio anyway unless you did the right incantations under the full moon.
- We could tone down the logging if we wanted at either the te.exe execution time (with a switch) or by declaring properties in the tests/classes/modules that are very verbose to not log unless it fails.
- I don't think anyone cares how they get run as long as they do.
Replace `utf8Parser` with `til::u8u16` in order to have the same
conversion algorithms used in terminal and conhost.
This PR addresses item 2 in this list:
1. ✉ Implement `til::u8u16` and `til::u16u8` (done in PR #4093)
2. ✔ **Unify UTF-8 handling using `til::u8u16` (this PR)**
2.1. ✔ **Update VtInputThread::_HandleRunInput()**
2.2. ✔ **Update ApiRoutines::WriteConsoleAImpl()**
2.3. ❌ (optional / ask the core team) Remove Utf8ToWideCharParser from the code base to avoid further use
3. ❌ Enable BOM discarding (follow up)
3.1. ❌ extend `til::u8u16` and `til::u16u8` with a 3rd parameter to enable discarding the BOM
3.2. ❌ Make use of the 3rd parameter to discard the BOM in all current function callers, or (optional / ask the core team) make it the default for `til::u8u16` and `til::u16u8`
4. ❌ Find UTF-16 to UTF-8 conversions and examine if they can be unified, too (follow up)
Closes#4086Closes#3378
## Summary of the Pull Request
This adds support for the [`DECAWM`](https://vt100.net/docs/vt510-rm/DECAWM) private mode escape sequence, which controls whether or not the output wraps to the next line when the cursor reaches the right edge of the screen. Tested manually, with [Vttest](https://invisible-island.net/vttest/), and with some new unit tests.
## PR Checklist
* [x] Closes#3826
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3826
## Detailed Description of the Pull Request / Additional comments
The idea was to repurpose the existing `ENABLE_WRAP_AT_EOL_OUTPUT` mode, but the problem with that was it didn't work in VT mode - specifically, disabling it didn't prevent the wrapping from happening. This was because in VT mode the `WC_DELAY_EOL_WRAP` behaviour takes affect, and that bypasses the usual codepath where `ENABLE_WRAP_AT_EOL_OUTPUT` is checked,
To fix this, I had to add additional checks in the `WriteCharsLegacy` function (7dbefe06e41f191a0e83cfefe4896b66094c4089) to make sure the `WC_DELAY_EOL_WRAP` mode is only activated when `ENABLE_WRAP_AT_EOL_OUTPUT` is also set.
Once that was fixed, though, another issue came to light: the `ENABLE_WRAP_AT_EOL_OUTPUT` mode doesn't actually work as documented. According to the docs, "if this mode is disabled, the last character in the row is overwritten with any subsequent characters". What actually happens is the cursor jumps back to the position at the start of the write, which could be anywhere on the line.
This seems completely broken to me, but I've checked in the Windows XP, and it has the same behaviour, so it looks like that's the way it has always been. So I've added a fix for this (9df98497ca38f7d0ea42623b723a8e2ecf9a4ab9), but it is only applied in VT mode.
Once that basic functionality was in place, though, we just needed a private API in the `ConGetSet` interface to toggle the mode, and then that API could be called from the `AdaptDispatch` class when the `DECAWM` escape sequence was received.
One last thing was to reenable the mode in reponse to a `DECSTR` soft reset. Technically the auto wrap mode was disabled by default on many of the DEC terminals, and some documentation suggests that `DECSTR` should reset it to that state, But most modern terminals (including XTerm) expect the wrapping to be enabled by default, and `DECSTR` reenables that state, so that's the behaviour I've copied.
## Validation Steps Performed
I've add a state machine test to confirm the `DECAWM` escape is dispatched correctly, and a screen buffer test to make sure the output is wrapped or clamped as appropriate for the two states.
I've also confirmed that the "wrap around" test is now working correctly in the _Test of screen features_ in Vttest.
## Summary of the Pull Request
This pull request is intended to achieve the following goals...
1) reduce duplicate code
2) remove static functions
3) improve readability
4) improve reliability
5) improve code-coverage for testing
6) establish functioning text buffer navigation in Narrator and NVDA
This also required a change to the wrapper class `XamlUiaTextRange` that has been causing issues with Narrator and NVDA.
See below for additional context.
## References
#3976 - I believe this might have been a result of improperly handling degenerate ranges. Fixed here.
#3895 - reduced the duplicate code. No need to separate into different files
#2160 - same as #3976 above
#1993 - I think just about everything is no longer static
## PR Checklist
* [x] Closes#3895, Closes#1993, Closes#3976, Closes#2160
* [x] CLA signed
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
### UiaTextRange
- converted endpoints into the COORD system in the TextBuffer coordinate space
- `start` is inclusive, `end` is exclusive. A degenerate range is when start == end.
- all functions are no longer static
- `MoveByUnit()` functions now rely on `MoveEndpointByUnit()` functions
- removed unnecessary typedefs like `Endpoint`, `ScreenInfoRow`, etc..
- relied more heavily on existing functionality from `TextBuffer` and `Viewport`
### XamlUiaTextRange
- `GetAttributeValue()` must return a special HRESULT that signifies that the requested attribute is not supported. This was the cause of a number of inconsistencies between Narrator and NVDA.
- `FindText()` should return `nullptr` if nothing was found. #4373 properly fixes this functionality now that Search is a shared module
### TextBuffer
- Word navigation functionality is entirely in `TextBuffer` for proper abstraction
- a total of 6 functions are now dedicated to word navigation to get a good understanding of the differences between a "word" in Accessibility and a "word" in selection
As an example, consider a buffer with this text in it:
" word other "
In selection, a "word" is defined as the range between two delimiters, so the words in the example include [" ", "word", " ", "other", " "].
In accessibility , a "word" includes the delimiters after a range of readable characters, so the words in the example include ["word ", "other "].
Additionally, accessibility word navigation must be able to detect if it is on the first or last word. This resulted in a slight variant of word navigation functions that return a boolean instead of a COORD.
Ideally, these functions can be consolidated, but that is too risky for a PR of this size as it can have an effect on selection.
### Viewport
- the concept of `EndExclusive` is added. This is used by UiaTextRange's `end` anchor as it is exclusive. To signify that the last character in the buffer is included in this buffer, `end` must be one past the end of the buffer. This is `EndExclusive`
- Since many functions check if the given `COORD` is in bounds, a flag must be set to allow `EndExclusive` as a valid `COORD` that is in bounds.
### Testing
- word navigation testing relies more heavily on TextBuffer tests
- additional testing was created for non-movement focused functions of UiaTextRange
- The results have been compared to Microsoft Word and some have been verified by UiAutomation/Narrator contacts as expected results.
## Validation Steps Performed
Tests pass
Narrator works
NVDA works
## Summary of the Pull Request
This is a pair of related fixes to conpty. For both of these bugs, the root cause was that the cursor was getting set to Off in conpty. Without the `CursorBlinkerTimer`, the cursor would remain off, and frames that only had cursor movements would not update the cursor position in the terminal.
## References
## PR Checklist
* [x] Closes#4102
* [x] Closes#2642
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Recall that there's a bunch of cursor state that's hard to parse without looking up:
* `Visibility` This controls whether the cursor is visible _at all_, regardless if it's been blinked on or off
* `Blinking` controls whether the blinker timer should do something, or leave the cursor alone.
* `IsOn`: When the cursor is blinking, this alternates between true and false.
The trick here is that we only `TriggerCursorMoved` when the cursor is `On`, and there are some scenarios where the cursor is manually set to off.
Fundamentally, these two bugs are similar cases, but they are triggered by different things:
* #2642 was caused by `DoSrvPrivateAllowCursorBlinking(false)` (`^[[?12l`) also manually turning the cursor off.
* #4102 was caused by the client calling `SetConsoleScreenBuffer` to change the active buffer. `win-curses` actually uses that API instead of the alt buffer.
## Summary of the Pull Request
This PR removes all of the VT-specific functionality from the `WriteCharsLegacy` function that dealt with control characters, since those controls are now handled in the state machine when in VT mode. It also removes most of the control character handling from the `Terminal::_WriteBuffer` method for the same reason.
## References
This is a followup to PR #4171
## PR Checklist
* [x] Closes#3971
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: https://github.com/microsoft/terminal/issues/780#issuecomment-570287435
## Detailed Description of the Pull Request / Additional comments
There are four changes to the `WriteCharsLegacy` implementation:
1. The `TAB` character had special case handling in VT mode which is now no longer required. This fixes a bug in the Python REPL editor (when run from a cmd shell in Windows Terminal), which would prevent you tabbing past the end of the line. It also fixes#3971.
2. Following on from point 1, the `WC_NONDESTRUCTIVE_TAB` flag could also now be removed. It only ever applied in VT mode, in which case the `TAB` character isn't handled in `WriteCharsLegacy`, so there isn't a need for a non-destructive version.
3. There used to be special case handling for a `BS` character at the beginning of the line when in VT mode, and that is also no longer required. This fixes an edge-case bug which would prevent a glyph being output for code point 8 when `ENABLE_PROCESSED_OUTPUT` was disabled.
4. There was quite a lot of special case handling for control characters in the "end-of-line wrap" implementation, which is no longer required. This fixes a bug which would prevent "low ASCII" characters from wrapping when output at the end of a line.
Then in the `Terminal::_WriteBuffer` implementation, I've simply removed all control character handling, except for `LF`. The Terminal is always in VT mode, so the control characters are always handled by the state machine. The exception for the `LF` character is simply because it doesn't have a proper implementation yet, so it still passes the character through to `_WriteBuffer`. That will get cleaned up eventually, but I thought that could wait for a later PR.
Finally, with the removal of the VT mode handling in `WriteCharsLegacy`, there was no longer a need for the `SCREEN_INFORMATION::InVTMode` method to be publicly accessible. That has now been made private.
## Validation Steps Performed
I've only tested manually, making sure the conhost and Windows Terminal still basically work, and confirming that the above-mentioned bugs are fixed by these changes.
## Summary of the Pull Request
#4354 is a pretty complicated PR. It's got a bunch of conpty changes, but what it also has was some critical improvements to the roundtrip test suite. I'm working on some other bugfixes in the same area currently, and need these tests enhancements in those branches _now_. The rest of #4354 is complex enough that I don't trust it will get merged soon (if ever). However, these fixes _should_ be in regardless.
## PR Checklist
* [x] Taken directly from #4354
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
This is four main changes:
* Enable conpty to be fully enabled in unittests. Just setting up a VT renderer isn't enough to trick the host into being in conpty mode - it also needs to have some other flags set.
* Some minor changes to `CommonState` to better configure the common test state for conpty
* Move some of the verify helpers from `ConptyRoundtripTests` into their own helper class, to be shared in multiple tests
* Add a `TerminalBufferTests` class, for testing the Terminal buffer directly (without conpty).
This change is really easier than
would suggest, I promise.
## Summary of the Pull Request
This adds support for the [`DECSCNM`](https://vt100.net/docs/vt510-rm/DECSCNM.html) private mode escape sequence, which toggles the display between normal and reverse screen modes. When reversed, the background and foreground colors are switched. Tested manually, with [Vttest](https://invisible-island.net/vttest/), and with some new unit tests.
## References
This also fixes issue #72 for the most part, although if you toggle the mode too fast, there is no discernible flash.
## PR Checklist
* [x] Closes#3773
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
I've implemented this as a new flag in the `Settings` class, along with updates to the `LookupForegroundColor` and `LookupBackgroundColor` methods, to switch the returned foreground and background colors when that flag is set.
It also required a new private API in the `ConGetSet` interface to toggle the setting. And that API is then called from the `AdaptDispatch` class when the screen mode escape sequence is received.
The last thing needed was to add a step to the `HardReset` method, to reset the mode back to normal, which is one of the `RIS` requirements.
Note that this does currently work in the Windows Terminal, but once #2661 is implemented that may no longer be the case. It might become necessary to let the mode change sequences pass through conpty, and handle the color reversing on the client side.
## Validation Steps Performed
I've added a state machine test to make sure the escape sequence is dispatched correctly, and a screen buffer test to confirm that the mode change does alter the interpretation of colors as expected.
I've also confirmed that the various "light background" tests in Vttest now display correctly, and that the `tput flash` command (in a bash shell) does actually cause the screen to flash.
## Summary of the Pull Request
This PR adds support for the `HPR` and `VPR` escape sequences from the VT510 terminal. `HPR` moves the cursor position forward by a given number of columns, and `VPR` moves the cursor position downward by a given number of rows. They're similar in function to the `CUF` and `CUD` escape sequences, except that they're not constrained by the scrolling margins.
## References
#3628 provided the new `_CursorMovePosition` method that made these operations possible
## PR Checklist
* [x] Closes#3428
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
Most of the implementation is in the new `_CursorMovePosition` method that was created in PR #3628, so all we're really doing here is hooking up the escape sequences to call that method with the appropriate parameters.
## Validation Steps Performed
I've extended the existing state machine tests for CSI cursor movement to confirm that the `HPR` and `VPR` sequences are dispatched correctly, and also added screen buffer tests to make sure the movement is clamped by the screen boundaries and not the scrolling margins (we don't yet support horizontal margins, but the test is at least in place for when we do eventually add that support).
I've also checked the `HPR` and `VPR` tests in Vttest (under _Test non-VT100 / ISO-6429 cursor-movement_) and confirmed that they are now working as expected.
## Summary of the Pull Request
This PR adds two tests:
* First, I started by writing a test where I could write output to the console host and inspect what output came out of conpty. This is the `ConptyOutputTests` in the host unit tests.
* Then I got crazy and thought _"what if I could take that output and dump it straight into the `Terminal`"_? Hence, the `ConptyRoundtripTests` were born, into the TerminalCore unit tests.
## References
Done in pursuit of #4200, but I felt this warranted it's own atomic PR
## PR Checklist
* [x] Doesn't close anything on it's own.
* [x] I work here
* [x] you better believe this adds tests
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
From the comment in `ConptyRoundtripTests`:
> This test class creates an in-proc conpty host as well as a Terminal, to
> validate that strings written to the conpty create the same resopnse on the
> terminal end. Tests can be written that validate both the contents of the
> host buffer as well as the terminal buffer. Everytime that
> `renderer.PaintFrame()` is called, the tests will validate the expected
> output, and then flush the output of the VtEngine straight to th
Also, some other bits had to be updated:
* The renderer needed to be able to survive without a thread, so I hadded a simple check that it actually had a thread before calling `pThread->NotifyPaint`
* Bits in `CommonState` used `NTSTATUS_FROM_HRESULT` which did _not_ work outside the host project. Since the `NTSTATUS` didn't seem that important, I replaced that with a `HRESULT`
* `CommonState` likes to initialize the console to some _weird_ defaults. I added an optional param to let us just use the defaults.
This commit moves the handling of the `BEL`, `BS`, `TAB`, and `CR`
controls characters into the state machine (when in VT mode), instead of
forwarding them on to the default string writer, which would otherwise
have to parse them out all over again.
This doesn't cover all the control characters, but `ESC`, `SUB`, and
`CAN` are already an integral part of the `StateMachine` itself; `NUL`
is filtered out by the `OutputStateMachineEngine`; and `LF`, `FF`, and
`VT` are due to be implemented as part of PR #3271.
Once all of these controls are handled at the state machine level, we
can strip out all the VT-specific code from the `WriteCharsLegacy`
function, which should simplify it considerably. This would also let us
simplify the `Terminal::_WriteBuffer` implementation, and the planned
replacement stream writer for issue #780.
On the conhost side, the implementation is handled as follows:
* The `BS` control is dispatched to the existing `CursorBackward`
method, with a distance of 1.
* The `TAB` control is dispatched to the existing `ForwardTab` method,
with a tab count of 1.
* The `CR` control required a new dispatch method, but the
implementation was a simple call to the new `_CursorMovePosition` method
from PR #3628.
* The `BEL` control also required a new dispatch method, as well as an
additional private API in the `ConGetSet` interface. But that's mostly
boilerplate code - ultimately it just calls the `SendNotifyBeep` method.
On the Windows Terminal side, not all dispatch methods are implemented.
* There is an existing `CursorBackward` implementation, so `BS` works
OK.
* There isn't a `ForwardTab` implementation, but `TAB` isn't currently
required by the conpty protocol.
* I had to implement the `CarriageReturn` dispatch method, but that was
a simple call to `Terminal::SetCursorPosition`.
* The `WarningBell` method I've left unimplemented, because that
functionality wasn't previously supported anyway, and there's an
existing issue for that (#4046).
## Validation Steps Performed
I've added a state machine test to confirm that the updated control
characters are now forwarded to the appropriate dispatch handlers. But
since the actual implementation is mostly relying on existing
functionality, I'm assuming that code is already adequately tested
elsewhere. That said, I have also run various manual tests of my own,
and confirmed that everything still worked as well as before.
References #3271
References #780
References #3628
References #4046
## Summary of the Pull Request
Originally there were 3 different methods for implementing VT cursor movement, and between them they still couldn't handle some of the operations correctly. This PR unifies those operations into a single method that can handle every type of cursor movement, and which fixes some of the issues with the existing implementations. In particular it fixes the `CNL` and `CPL` operations, so they're now correctly constrained by the `DECSTBM` margins.
## References
If this PR is accepted, the method added here should make it trivial to implement the `VPR` and `HPR` commands in issue #3428.
## PR Checklist
* [x] Closes#2926
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
The new [`AdaptDispatch::_CursorMovePosition`](d6c4f35cf6/src/terminal/adapter/adaptDispatch.cpp (L169)) method is based on the proposal I made in issue #3428 for the `VPR` and `HPR` comands. It takes three arguments: a row offset (which can be absolute or relative), a column offset (ditto), and a flag specifying whether the position should be constrained by the `DECSTBM` margins.
To make the code more readable, I've implemented the offsets using [a `struct` with some `constexpr` helper functions for the construction](d6c4f35cf6/src/terminal/adapter/adaptDispatch.hpp (L116-L125)). This lets you specify the parameters with expressions like `Offset::Absolute(col)` or `Offset::Forward(distance)` which I think makes the calling code a little easier to understand.
While implementing this new method, I noticed a couple of issues in the existing movement implementations which I thought would be good to fix at the same time.
1. When cursor movement is constrained horizontally, it should be constrained by the buffer width, and not the horizontal viewport boundaries. This is an issue I've previously corrected in other parts of the codebase, and I think the cursor movement was one of the last areas where it was still a problem.
2. A number of the commands had range and overflow checks for their parameters that were either unnecessary (testing for a condition that could never occur) or incorrect (if an operation overflows, the correct behavior is to clamp it, and not just fail). The new implementation handles legitimate overflows correctly, but doesn't check for impossible ranges.
Because of the change of behavior in point 1, I also had to update the implementations of [the `DECSC` and `CPR` commands](9cf7a9b577) to account for the column offset now being relative to the buffer and not the viewport, otherwise those operations would no longer work correctly.
## Validation Steps Performed
Because of the two changes in behavior mentioned above, there were a number of adapter tests that stopped working and needed to be updated. First off there were those that expected the column offset to be relative to the left viewport position and constrained by the viewport width. These now had to be updated to [use the full buffer width](49887a3589) as the allowed horizontal extent.
Then there were all the overflow and out-of-range tests that were testing conditions that could never occur in practice, or where the expected behavior that was tested was actually incorrect. I did spend some time trying to see if there was value in updating these tests somehow, but in the end I decided it was best to just [drop them](6e80d0de19) altogether.
For the `CNL` and `CPL` operations, there didn't appear to be any existing tests, so I added some [new screen buffer tests](d6c4f35cf6) to check that those operations now work correctly, both with and without margins.
## Summary of the Pull Request
This adds support for the `FF` (form feed) and `VT` (vertical tab) [control characters](https://vt100.net/docs/vt510-rm/chapter4.html#T4-1), as well as the [`NEL` (Next Line)](https://vt100.net/docs/vt510-rm/NEL.html) and [`IND` (Index)](https://vt100.net/docs/vt510-rm/IND.html) escape sequences.
## References
#976 discusses the conflict between VT100 Index sequence and the VT52 cursor back sequence.
## PR Checklist
* [x] Closes#3189
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3189
## Detailed Description of the Pull Request / Additional comments
I've added a `LineFeed` method to the `ITermDispatch` interface, with an enum parameter specifying the required line feed type (i.e. with carriage return, without carriage return, or dependent on the [`LNM` mode](https://vt100.net/docs/vt510-rm/LNM.html)). The output state machine can then call that method to handle the various line feed control characters (parsed in the `ActionExecute` method), as well the `NEL` and `IND` escape sequences (parsed in the `ActionEscDispatch` method).
The `AdaptDispatch` implementation of `LineFeed` then forwards the call to a new `PrivateLineFeed` method in the `ConGetSet` interface, which simply takes a bool parameter specifying whether a carriage return is required or not. In the case of mode-dependent line feeds, the `AdaptDispatch` implementation determines whether the return is necessary or not, based on the existing _AutoReturnOnNewLine_ setting (which I'm obtaining via another new `PrivateGetLineFeedMode` method).
Ultimately we'll want to support changing the mode via the [`LNM` escape sequence](https://vt100.net/docs/vt510-rm/LNM.html), but there's no urgent need for that now. And using the existing _AutoReturnOnNewLine_ setting as a substitute for the mode gives us backwards compatible behaviour, since that will be true for the Windows shells (which expect a linefeed to also generate a carriage return), and false in a WSL bash shell (which won't want the carriage return by default).
As for the actual `PrivateLineFeed` implementation, that is just a simplified version of how the line feed would previously have been executed in the `WriteCharsLegacy` function. This includes setting the cursor to "On" (with `Cursor::SetIsOn`), potentially clearing the wrap property of the line being left (with `CharRow::SetWrapForced` false), and then setting the new position using `AdjustCursorPosition` with the _fKeepCursorVisible_ parameter set to false.
I'm unsure whether the `SetIsOn` call is really necessary, and I think the way the forced wrap is handled needs a rethink in general, but for now this should at least be compatible with the existing behaviour.
Finally, in order to make this all work in the _Windows Terminal_ app, I also had to add a basic implementation of the `ITermDispatch::LineFeed` method in the `TerminalDispatch` class. There is currently no need to support mode-specific line feeds here, so this simply forwards a `\n` or `\r\n` to the `Execute` method, which is ultimately handled by the `Terminal::_WriteBuffer` implementation.
## Validation Steps Performed
I've added output engine tests which confirm that the various control characters and escape sequences trigger the dispatch method correctly. Then I've added adapter tests which confirm the various dispatch options trigger the `PrivateLineFeed` API correctly. And finally I added some screen buffer tests that check the actual results of the `NEL` and `IND` sequences, which covers both forms of the `PrivateLineFeed` API (i.e. with and without a carriage return).
I've also run the _Test of cursor movements_ in the [Vttest](https://invisible-island.net/vttest/) utility, and confirmed that screens 1, 2, and 5 are now working correctly. The first two depend on `NEL` and `IND` being supported, and screen 5 requires the `VT` control character.
## Summary of the Pull Request
In pursuit of reflowing the terminal buffer on resize, move the reflow algorithm to the TextBuffer. This does _not_ yet add support for reflowing in the Windows Terminal.
## References
## PR Checklist
* [ ] There's not really an issue for this yet, I'm just breaking this work up into as many PRs as possible to help the inevitable bisect.
* [x] I work here
* [x] Ideally, all the existing tests will pass
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
In `SCREEN_INFORMATION::ResizeScreenBuffer`, the screenbuffer needs to create a new buffer, and copy the contents of the old buffer into the new one. I'm moving that "copy contents from the old buffer to the new one" step to it's own helper, as a static function on `TextBuffer`. That way, when the time comes to implement this for the Terminal, the hard part of the code will already be there.
## Validation Steps Performed
Ideally, all the tests will still pass.
## Summary of the Pull Request
This PR reverses the behaviour of the `IS_GLYPH_CHAR` macro, so it now actually returns true if the given char is a glyph, and false if it isn't. Previously it returned the opposite of that, which meant it had to be called as `!IS_GLYPH_CHAR` to get the correct result.
## PR Checklist
* [x] Closes#4185
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #4185
## Detailed Description of the Pull Request / Additional comments
The original implementation returned true if the given character was a C0 control, or a DEL:
#define IS_GLYPH_CHAR(wch) (((wch) < L' ') || ((wch) == 0x007F))
It's now the exact opposite, so returns true for characters that are _not_ C0 controls, and are not the DEL character either:
#define IS_GLYPH_CHAR(wch) (((wch) >= L' ') && ((wch) != 0x007F))
The macro was only used in one place, where is was being called as `!IS_GLYPH_CHAR` when the intent was actually to test whether the char _was_ a glyph. That code could now be updated to remove the `!`, so it makes more sense.
## Validation Steps Performed
I've just tested manually and confirmed that basic output of text and control chars still worked as expected in a conhost shell.
## Summary of the Pull Request
New year, new unittests.
This PR introduces a new project, `TestHostApp`. This project is largely taken from the TAEF samples, and allows us to easily construct a helper executable and `resources.pri` for running TerminalApp unittests.
## References
## PR Checklist
* [x] Closes#3986
* [x] I work here
* [x] is Tests
* [n/a] Requires documentation to be updated
* [x] **Waiting for an updated version of TAEF to be available**
## Detailed Description of the Pull Request / Additional comments
Unittesting for the TerminalApp project has been a horrifying process to try getting everything pieced together just right. Dependencies need to get added to manifests, binplaced correctly, and XAML resources need to get compiled together as well. In addition, using a MUX `Application` (as opposed to the Windows.UI.Xaml `Application`) has led to additional problems.
This was always a horrifying house of cards for us. Turns out, the reason this was so horrible is that the test infrastructure for doing what we're doing _literally didn't exist_ when I started doing all that work last year.
So, with help from the TAEF team, I was able to get rid of our entire house of cards, and use a much simpler project to build and run the tests.
Unfortunately, the latest TAEF release has a minor bug in it's build rules, and only publishes the x86 version of a dll we need from them. But, the rest of this PR works for x86, and I'll bump this when that updated version is available. We should be able to review this even in the state it's in.
## Validation Steps Performed
ran the tests yo
<!-- 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
It's 2020 now. It's *about* time that we move on from 1990's macros.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] Closes#4152
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
Remove global namespaced min/max and replace it with STL min/max.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Run it.
## Summary of the Pull Request
Refactors parsing/adapting libraries and consumers to use safer and/or more consistent mechanisms for passing information.
## PR Checklist
* [x] I work here
* [x] Tests still pass
* [x] Am a core contributor.
## Detailed Description of the Pull Request / Additional comments
This is in support of hopefully turning audit mode on to more projects. If I turned it on, it would immediately complain about certain classes of issues like pointer and size, pointer math, etc. The changes in this refactoring will eliminate those off the top.
Additionally, this has caught a bunch of comments all over the VT classes that weren't updated to match the parameters lists.
Additionally, this has caught a handful of member variables on classes that were completely unused (and now gone).
Additionally, I'm killing almost all hungarian and shortening variable names. I'm only really leaving 'p' for pointers.
Additionally, this is vaguely in support of a future where we can have "infinite scrollback" in that I'm moving things to size_t across the board. I know it's a bit of a memory cost, but all the casting and moving between types is error prone and unfun to save a couple bytes.
## Validation Steps Performed
- [x] build it
- [x] run all the tests
- [x] everyone looked real hard at it
This pull request also includes build break fixes for things that do not build in the OSS repo
and disables the retro terminal effect in conhost.
Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp f10445678e59197c1ae2ee29d8f009c9607c4e5d
Related work items: #24387718
## Summary of the Pull Request
This adds support for the [`DECALN`](https://vt100.net/docs/vt510-rm/DECALN.html) escape sequence, which produces a kind of test pattern, originally used on VT terminals to adjust the screen alignment. It's needed to pass several of the tests in the [Vttest](https://invisible-island.net/vttest/) suite.
## PR Checklist
* [x] Closes#3671
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
To start with, the `ActionEscDispatch` method in the `OutputStateMachineEngine` needed to be extended to check for a new intermediate type (`#`). Then when that intermediate is followed by an `8`, it dispatches to a new `ScreenAlignmentPattern` method in the `ITermDispatch` interface.
The implementation of the `ScreenAlignmentPattern` itself is fairly simple. It uses the recently added `PrivateFillRegion` API to fill the screen with the character `E` using default attributes. Then in addition to that, a bunch of VT properties are reset:
* The meta/extended attributes are reset (although the active colors must be left unchanged).
* The origin mode is set to absolute positioning.
* The scrolling margins are cleared.
* The cursor position is moved to home.
## Validation Steps Performed
I've added a screen buffer test that makes sure the `DECALN` sequence fills the screen with the correct character and attributes, and that the above mentioned properties are all updated appropriately.
I've also tested in Vttest, and confirmed that the first two pages of the _Test of cursor movements_ are now showing the frame of E's that are expected there.
<!-- 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)? -->
This is the PR for feature Search: #605
This PR includes the newly introduced SearchBoxControl in TermControl dir, which is the search bar for the search experience. And the codes that enable Search in Windows Terminal.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
The PR that migrates the Conhost search module: https://github.com/microsoft/terminal/pull/3279
Spec (still actively updating): https://github.com/microsoft/terminal/pull/3299
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#605
* [ ] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
These functionalities are included in the search experience.
1. Search in Terminal text buffer.
2. Automatic wrap-around.
3. Search up or down switch by clicking different buttons.
4. Search case sensitively/insensitively by clicking a button. S. Move the search box to the top/bottom by clicking a button.
6. Close by clicking 'X'.
7. Open search by ctrl + F.
When the searchbox is open, the user could still interact with the terminal by clicking the terminal input area.
While I already have the search functionalities, currently there are still some known to-do works and I will keep updating my PR:
1. Optimize the search box UI, this includes:
1) Theme adaptation. The search box background and font color
should change according to the theme,
2) Add background. Currently the elements in search box are all
transparent. However, we need a background.
3) Move button should be highlighted once clicked.
2. Accessibility: search process should be able to performed without mouse. Once the search box is focused, the user should be able to navigate between all interactive elements on the searchbox using keyboard.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
To test:
1. checkout this branch.
2. Build the project.
3. Start Windows Terminal and press Ctrl+F
4. The search box should appear on the top right corner.
<!-- 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
Cool retro terminal effects
- glow
- scan lines
- cool
- will make terminal competitive with iterm2
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
## Summary of the Pull Request
When the `DECSTBM` margins are set, scrolling should only allowed within
those margins. So if the cursor is below the bottom margin, it should
just be clamped when it tries to move down from the bottom line of the
viewport, instead of scrolling the screen up. This PR implements that
restriction, i.e. it prevents scrolling taking place outside the
`DECSTBM` margins, which was previously allowed.
## PR Checklist
* [x] Closes#2657
* [x] CLA signed.
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
This simply adds a condition in the `AdjustCursorPosition` function to
check if the cursor is moving below the bottom of the viewport when the
margins are set. If so, it clamps the y coordinate to the bottom line of
the viewport.
The only time it's acceptable for scrolling to happen when margins are
set, is if the scrolling is taking place within those margins. But those
cases would already have been handled earlier in the function (in the
`fScrollDown` or `scrollDownAtTop` conditions), and thus the y
coordinate would have already been prevented from moving out of the
viewport.
## Validation Steps Performed
I've added some screen buffer tests to confirm this new behaviour, and
I've also checked the test case from the initial bug report (#2657) and
made sure it now matches the results in XTerm.
## Summary of the Pull Request
Operations that erase areas of the screen are typically meant to do so using the current color attributes, but with the rendition attributes reset (what we refer to as meta attributes). This also includes scroll operations that have to clear the area of the screen that has scrolled into view. The only exception is the _Erase Scrollback_ operation, which needs to reset the buffer with the default attributes. This PR updates all of these cases to apply the correct attributes when scrolling and erasing.
## PR Checklist
* [x] Closes#2553
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've not really discussed this with core contributors. I'm ready to accept this work might be rejected in favor of a different grand plan.
## Detailed Description of the Pull Request / Additional comments
My initial plan was to use a special case legacy attribute value to indicate the "standard erase attribute" which could safely be passed through the legacy APIs. But this wouldn't cover the cases that required default attributes to be used. And then with the changes in PR #2668 and #2987, it became clear that our requirements could be better achieved with a couple of new private APIs that wouldn't have to depend on legacy attribute hacks at all.
To that end, I've added the `PrivateFillRegion` and `PrivateScrollRegion` APIs to the `ConGetSet` interface. These are just thin wrappers around the existing `SCREEN_INFORMATION::Write` method and the `ScrollRegion` function respectively, but with a simple boolean parameter to choose between filling with default attributes or the standard erase attributes (i.e the current colors but with meta attributes reset).
With those new APIs in place, I could then update most scroll operations to use `PrivateScrollRegion`, and most erase operations to use `PrivateFillRegion`.
The functions affected by scrolling included:
* `DoSrvPrivateReverseLineFeed` (the RI command)
* `DoSrvPrivateModifyLinesImpl` (the IL and DL commands)
* `AdaptDispatch::_InsertDeleteHelper` (the ICH and DCH commands)
* `AdaptDispatch::_ScrollMovement` (the SU and SD commands)
The functions affected by erasing included:
* `AdaptDispatch::_EraseSingleLineHelper` (the EL command, and most ED variants)
* `AdaptDispatch::EraseCharacters` (the ECH command)
While updating these erase methods, I noticed that both of them also required boundary fixes similar to those in PR #2505 (i.e. the horizontal extent of the erase operation should apply to the full width of the buffer, and not just the current viewport width), so I've addressed that at the same time.
In addition to the changes above, there were also a few special cases, the first being the line feed handling, which required updating in a number of places to use the correct erase attributes:
* `SCREEN_INFORMATION::InitializeCursorRowAttributes` - this is used to initialise the rows that pan into view when the viewport is moved down the buffer.
* `TextBuffer::IncrementCircularBuffer` - this occurs when we scroll passed the very end of the buffer, and a recycled row now needs to be reinitialised.
* `AdjustCursorPosition` - when within margin boundaries, this relies on a couple of direct calls to `ScrollRegion` which needed to be passed the correct fill attributes.
The second special case was the full screen erase sequence (`ESC 2 J`), which is handled separately from the other ED sequences. This required updating the `SCREEN_INFORMATION::VtEraseAll` method to use the standard erase attributes, and also required changes to the horizontal extent of the filled area, since it should have been clearing the full buffer width (the same issue as the other erase operations mentioned above).
Finally, there was the `AdaptDispatch::_EraseScrollback` method, which uses both scroll and fill operations, which could now be handled by the new `PrivateScrollRegion` and `PrivateFillRegion` APIs. But in this case we needed to fill with the default attributes rather than the standard erase attributes. And again this implementation needed some changes to make sure the full width of the active area was retained after the erase, similar to the horizontal boundary issues with the other erase operations.
Once all these changes were made, there were a few areas of the code that could then be simplified quite a bit. The `FillConsoleOutputCharacterW`, `FillConsoleOutputAttribute`, and `ScrollConsoleScreenBufferW` were no longer needed in the `ConGetSet` interface, so all of that code could now be removed. The `_EraseSingleLineDistanceHelper` and `_EraseAreaHelper` methods in the `AdaptDispatch` class were also no longer required and could be removed.
Then there were the hacks to handle legacy default colors in the `FillConsoleOutputAttributeImpl` and `ScrollConsoleScreenBufferWImpl` implementations. Since those hacks were only needed for VT operations, and the VT code no longer calls those methods, there was no longer a need to retain that behaviour (in fact there are probably some edge cases where that behaviour might have been considered a bug when reached via the public console APIs).
## Validation Steps Performed
For most of the scrolling operations there were already existing tests in place, and those could easily be extended to check that the meta attributes were correctly reset when filling the revealed lines of the scrolling region.
In the screen buffer tests, I made updates of that sort to the `ScrollOperations` method (handling SU, SD, IL, DL, and RI), the `InsertChars` and `DeleteChars` methods (ICH and DCH), and the `VtNewlinePastViewport` method (LF). I also added a new `VtNewlinePastEndOfBuffer` test to check the case where the line feed causes the viewport to pan past the end of the buffer.
The erase operations, however, were being covered by adapter tests, and those aren't really suited for this kind of functionality (the same sort of issue came up in PR #2505). As a result I've had to reimplement those tests as screen buffer tests.
Most of the erase operations are covered by the `EraseTests` method, except the for the scrollback erase which has a dedicated `EraseScrollbackTests` method. I've also had to replace the `HardReset` adapter test, but that was already mostly covered by the `HardResetBuffer` screen buffer test, which I've now extended slightly (it could do with some more checks, but I think that can wait for a future PR when we're fixing other RIS issues).
# Summary of the Pull Request
When a horizontal tab ('\t') is output on the last column of the screen, the current implementation moves the cursor position to the start of the next line. However, the DEC STD 070 manual specifies that a horizontal tab shouldn't move past the last column of the active line (or the right margin, if we supported horizontal margins). This PR updates the forward tab implementation, to prevent it wrapping onto a new line when it reaches the end of a line.
# Detailed Description of the Pull Request / Additional comments
Originally the SCREEN_INFORMATION::GetForwardTab method had a condition which handled a tab at the end of the line as a special case, moving the cursor to the start of the next line. I've simply removed that condition, so an end-of-line tab is handled the same way as any other position (in this case it will just leaves the cursor where it is).
While testing, though, I found that there were circumstances where you could have tab stops greater than the width of the screen, and when that happens, a tab can still end up wrapping onto the next line. To fix that I had to add an additional check to make sure the tab position was always clamped to the width of the buffer.
With these fixes in place, a tab control should now never move off the active line, so I realised that the DoPrivateTabHelper function could be optimized to calculate all of the tab movements in advance, and then only make a single call to AdjustCursorPosition with the final coordinates. This change is not strictly necessary, though, so it can easily be reverted if there are any objections.
Regarding backwards compatibility, note that the GetForwardTab method is only used in two places:
when handling a tab character in the WriteCharsLegacy function, but this only applies in VT mode (see here).
when handling the CHT escape sequence in the DoPrivateTabHelper function, and obviously an escape sequence would also only be applicable in VT mode.
So this change should have no effect on legacy console applications, which wouldn't have VT mode activated.
# Validation Steps Performed
I've added another step to the TestGetForwardTab test which makes sure that a horizontal tab won't wrap at the end of a line.
I've also confirmed that this fixes the last remaining issue in the Test of autowrap in Vttest (pages 3 and 4 of the Test of cursor movements). Although I should note that this only works in conhost.
* Make search a shared component for conhost and terminal
* Remove inclusion of deprecated interface file
* Code review changes, remove text buffer modification in Terminal
* remove unreferenced objects to fix build errors
* Fix test failure, guarantee uiaData object is correctly initialized in Search
* minor comment typo fix and format fix
* minor PR comments change
* ColorSeclection directly throw and return
* remove coordAnchor initialization
* minor method signature change
* Change to use App CRT in preparation for universal.
* Try to make project build again by setting winconpty to static lib so it'll use the CRT inside TerminalConnection (or its other consumers) instead of linking its own.
* Remove test for conpty dll, it's a lib now. Add additional commentary on how CRT linking works for future reference. I'm sure this will come up again.
* use the _apiset variant until proven otherwise to match the existing one.
* Clarification in the comments for linking.
This commit cleans up and deduplicates all of the common build
preamble/postamble across exe, dll, lib and c++/winrt projects.
The following specific changes have been made:
* All projects now define their ConfigurationType
* All projects now set all their properties *before* including a common
build file (or any other build files)
* cppwinrt.pre and cppwinrt.post now delegate most of their
configuration to common.pre and common.post
* (becuase of the above,) all build options are conserved between
console and c++/winrt components, including specific warnings and
preprocessor definitions.
* More properties that are configurable per-project are now
conditioned so the common props don't override them.
* The exe, dll, exe.or.dll, and lib postincludes have been merged into
pre or post and switched based on condition as required
* Shared items (-shared, -common) are now explicitly vcxitems instead of
vcxproj files.
* The link line is now manipulated after Microsoft.Cpp sets it, so the
libraries we specify "win". All console things link first against
onecore_apiset.lib.
* Fix all compilation errors caused by build unification
* Move CascadiaPackage's resources into a separate item file
Fixes#922.
* Ensure rights check and increments pass before assigning an object to a handle (since deallocation of handles will automatically attempt to cleanup).
The current DECSC implementation only saves the cursor position and
origin mode. This PR improves that functionality with additional support
for saving the SGR attributes, as well as the active character set.
It also fixes the way the saved state interacts with the alt buffer
(private mode 1049), triggering a save when switching to the alt buffer,
and a restore when switching back, and tracking the alt buffer state
independently from the main state.
In order to properly save and restore the SGR attributes, we first
needed to add a pair of APIs in the `ConGetSet` interface which could
round-trip the attributes with full 32-bit colors (the existing methods
only work with legacy attributes).
I then added a struct in the `AdaptDispatch` implementation to make it
easier to manage all of the parameters that needed to be saved. This
includes the cursor position and origin mode that we were already
tracking, and now also the SGR text attributes and the active character
set (via the `TermOutput` class).
Two instances of this structure are required, since changes made to the
saved state in the alt buffer need to be tracked separately from changes
in the main buffer. I've added a boolean property that specifies whether
we're in the alt buffer or not, and use that to decide which of the two
instances we're working with.
I also needed to explicitly trigger a save when switching to the alt
buffer, and a restore when switching back, since we weren't already
doing that (the existing implementation gave the impression that the
state was saved, because each buffer has its own cursor position, but
that doesn't properly match the XTerm behaviour).
For the state tracking itself, we've now got two additional properties -
the SGR attributes, which we obtain via the new private API, and the
active character set, which we get from a local `AdaptDispatch` field.
I'm saving the whole `TermOutput` class for the character set, since I'm
hoping that will make it automatically supports future enhancements.
When restoring the cursor position, there is also now a fix to handle
the relative origin mode correctly. If the margins are changed between
the position being saved and restored, it's possible for the cursor to
end up outside of the new margins, which would be illegal. So there is
now an additional step that clamps the Y coordinate within the margin
boundaries if the origin mode is relative.
# Validation
I've added a couple of screen buffer tests which check that the various
parameters are saved and restored as expected, as well as checking that
the Y coordinate is clamped appropriately when the relative origin mode
is set.
I've also tested manually with vttest and confirmed that the
_SAVE/RESTORE CURSOR_ test (the last page of the _Test of screen
features_)) is now working a lot better than it used to.
Closes#148.
