This PR adds support for the `DA2` (Secondary Device Attributes) and
`DA3` (Tertiary Device Attributes) escape sequences, which are standard
VT queries reporting basic information about the terminal.
The _Secondary Device Attributes_ response is made up of a number of
parameters:
1. An identification code, for which I've used 0 to indicate that we
have the capabilities of a VT100 (using code 0 for this is an XTerm
convention, since technically DA2 would not have been supported by a
VT100).
2. A firmware revision level, which some terminal emulators use to
report their actual version number, but I thought it best we just
hardcode a value of 10 (the DEC convention for 1.0).
3. Additional hardware options, which tend to be device specific, but
I've followed the convention of the later DEC terminals using 1 to
indicate the presence of a PC keyboard.
The _Tertiary Device Attributes_ response was originally used to provide
a unique terminal identification code, and which some terminal emulators
use as a way to identify themselves. However, I think that's information
we'd probably prefer not to reveal, so I've followed the more common
practice of returning all zeros for the ID.
In terms of implementation, the only complication was the need to add an
additional code path in the `OutputStateMachine` to handle the `>` and
`=` intermediates (technically private parameter prefixes) that these
sequences require. I've done this as a single method - rather than one
for each prefix - since I think that makes the code easier to follow.
VALIDATION
----------
I've added output engine tests to make sure the sequences are dispatched
correctly, and adapter tests to confirm that they are returning the
responses we expect. I've also manually confirmed that they pass the
_Test of terminal reports_ in Vttest.
Closes#5836
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.
By storing up the accumulated delta in the mouse input handler, we can
enlighten both conhost and terminal about wheel events that are less
than one line in size. Previously, we had a workaround in conhost that
clamped small scroll deltas to a whole line, which made trackpad
scrolling unimaginably fast. Terminal didn't make this mistake, but it
also didn't handle delta accumulation . . . which resulted in the same
behavior.
MouseInput will now wait until it's received WHEEL_DELTA (well-known
constant, value 120) worth of scrolling delta before it dispatches a
single scroll event.
Future considerations may include sending multiple wheel button events
for every *multiple* of WHEEL_DELTA, but that would be a slightly larger
refactoring that I'm not yet ready to undertake.
There's a chance that we should be dividing WHEEL_DELTA by the system's
"number of lines to scroll at once" setting, because on trackpads
conhost now scrolls a little _slow_. I think the only way to determine
whether this is palatable is to just ship it.
Fixes#6184.
## Summary of the Pull Request
This PR adds full support for the `DECSCNM` reverse screen mode in the Windows Terminal to align with the implementation in conhost.
## References
* The conhost implementation of `DECSCNM` was in PR #3817.
* WT originally inherited that functionality via the colors being passed through, but that behaviour was lost in PR #6506.
## PR Checklist
* [x] Closes#6622
* [x] CLA signed.
* [ ] 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.
* [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: #6622
## Detailed Description of the Pull Request / Additional comments
The `AdaptDispatch::SetScreenMode` now checks if it's in conpty mode and simply returns false to force a pass-through of the mode change. And the `TerminalDispatch` now has its own `SetScreenMode` implementation that tracks any changes to the reversed state, and triggers a redraw in the renderer.
To make the renderer work, we just needed to update the `GetForegroundColor` and `GetBackgroundColor` methods of the terminal's `IRenderData` implementation to check the reversed state, and switch the colors being calculated, the same way the `LookupForegroundColor` and `LookupBackgroundColor` methods work in the conhost `Settings` class.
## Validation Steps Performed
I've manually tested the `DECSCNM` functionality for Windows Terminal in Vttest, and also with some of my own test scripts.
## Summary of the Pull Request
Updates the Terminal's scroll response to new output. The Terminal will not automatically scroll if...
- a selection is active, or
- the viewport is at the bottom of the scroll history
## References
#2529 - Spec
#3863 - Implementation
## PR Checklist
* [X] Closes#980
* [X] Closes#3863
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Updates the `_scrollOffset` value properly in TerminalCore when the cursor moves. We calculate a new `_scrollOffset` based on if we are circling the buffer and how far below the mutable bottom is.
We specifically check for if a selection is active and if the viewport is at the bottom, then use that as a condition for deciding if we should update `_scrollOffset` to the new calculated value or 0 (the bottom of the scroll history).
## Validation Steps Performed
Manual testing. Though I should add automated tests.
- [X] new output
- [X] new output when circling
- [X] new output when circling and viewport is at the top
Looking up the size of the viewport from the underlying dimensions of
the structures seemed like a good idea at the time (so it would only be
in one place), but it turns out to be more of a perf cost than we
expected. Not necessarily on any one hot path, but if we sort by
functions in WPR, it was the top consumer on the Terminal side. This
instead saves the size as a member of the `TextBuffer` and serves that
out. It only changes when it is constructed or resized traditionally, so
it's easy to update/keep track of. It impacted conhost/conpty to a
lesser degree but was still noticeable.
## Validation Steps Performed
- Run `time cat big.txt` under WPR. Checked before and after perf
metrics.
## PR Checklist
* [x] Closes perf itch
* [x] I work here
* [x] Manual test
* [x] Documentation irrelevant.
* [x] Schema irrelevant.
* [x] Am core contributor.
The main change in 16.7 is the separation of `AppContainerApplication`
into `WindowsStoreApp` and `WindowsAppContainer`. There's been a bit of
interest in splitting packaging away from containment, and this is the
first step in that direction.
We're a somewhat unique application, but as WinUI3 becomes more
prevalent we will become _less_ unique.
Some of these things, I've looked at and wondered how they ever worked.
## PR Checklist
* [x] Closes nothing
## Validation Steps Performed
Built locally and in CI. Tested the generated package with the package tester. Built on 16.6 and seen that it still seems to work.
A lot of time was spent between each individual line in the VT paint
engine in allocating some scratch space to assemble the clusters then
deallocating it only to have the next line do that again. Now we just
hold onto that memory space since it should be approximately the size of
a single line wide and will be used over and over and over as painting
continues.
## Validation Steps Performed
- Run `time cat big.txt` under WPR. Checked before and after perf
metrics.
## PR Checklist
* [x] Closes perf itch.
* [x] I work here.
* [x] Manual perf test.
* [x] Documentation irrelevant.
* [x] Schema irrelevant.
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
Passes the bitmap by ref into the tracing function instead of making a copy on the way in. It's only read anyway for tracing (if enabled) so the copy was a pointless oversight.
## Validation Steps Performed
- Observed WPR trace before and after with `time cat big.txt` in WSL.
This PR enables `ApplicationHighContrastAdjustment::None`. Doing this
disables a set of mitigations in XAML designed to band-aid apps that
were never explicitly designed for High Contrast (HC) modes. Terminal
now has full control of and responsibility for its appearance in HC
mode. This allows Terminal to look a lot better.
On paper, we should be able to set `HighContrastAdjustment="None"` on
the `<Application>` element. But that doesn't have any effect. I don't
know if this is a bug in `<Toolkit:XamlApplication>` or somewhere else.
So instead I set the property in codebehind, which is not as ideal, but
does at least work. I'd love to a way to move this into App.xaml.
The Find box had a couple stray styles to override the ToggleButton's
foreground color. With backplating removed, these styles became
actively harmful (white foreground on highlight color background), so I
just removed them. The built-in style for ToggleButton is perfect
as-is.
Closes#5360
WinUI's `Margin` and `Padding` work very similarly. `Margin` distances
ourselves from our parent. Whereas `Padding` distances our children from
ourselves.
Terminal's `padding` setting is actually implemented by defining
`Margin` on the SwapChainPanel. This means that the "padding" that is
created is actually belongs to SwapChainPanel's parent: Grid (not to be
confused with its parent, "RootGrid").
When a user clicks on the padded area, input goes to Grid. But there's a
twist: you can't actually hit Grid. To be able to hit Grid, you can't
just set IsHitTestVisible. You need to set it's Visibility to Visible,
and it's Background to Transparent (not null) [2].
## Validation Steps Performed
- [X] Start a selection from the padding area
- [X] Click on a SearchBox if one is available
- The SearchBox gets first dibs on the hit test so none gets through
to the SwapChainPanel
## References
[1] https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.uielement.ishittestvisible
[2] https://docs.microsoft.com/en-us/windows/uwp/xaml-platform/events-and-routed-events-overview#hit-testing-and-input-eventsCloses#5626
## Summary of the Pull Request
Pretty straightforward. Logs three scenarios:
* The user opened the command palette (and which mode it was opened in)
* The user ran a command from the palette
* The user dismissed the palette without running an action.
We discussed this in team sync yesterday.
## PR Checklist
* [x] I work here
* [n/a] Requires documentation to be updated
See: https://github.com/microsoft/microsoft-ui-xaml/releases/tag/v2.5.0-prerelease.200609001
> ### Notable Changes:
>
> Resize tab view items only once the pointer has left the TabViewItem strip (microsoft/microsoft-ui-xaml#2569)
> Align TabView visuals with Edge (microsoft/microsoft-ui-xaml#2201)
> Fix background of MenuFlyout in white high contrast (microsoft/microsoft-ui-xaml#2446)
> TabView: Make TabViewItem consume the TabViewItemHeaderForeground theme resource (microsoft/microsoft-ui-xaml#2348)
> TabView: Add tooltips to its scrolling buttons. (microsoft/microsoft-ui-xaml#2369)
* [x] Related to #5360 (@jtippet confirms that this alone does not close it.)
* [x] I work here
## Summary of the Pull Request
In the wake of #6635, a couple things got missed in merges:
* `toggleRetroEffect` didn't get into the schema, nor did `renameTab` or
`commandPalette`.
* `toggleRetroEffect` also didn't get a name
Furthermore, I thought it might be a good idea to start sticking
commands into `bindings` even without `keys`. So I tried doing that for
`opentabColorPicker` and `toggleRetroEffect`, and found immediately that
the labels for the key chord still appear even when the text is empty.
So I added some XAML magic to hide those when the text is empty.
## References
* #6762
* #6691
* #6557
* #6635
## PR Checklist
* [x] Closes#6762
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
* See also: https://docs.microsoft.com/en-us/windows/uwp/data-binding/data-binding-quickstart#formatting-or-converting-data-values-for-display
- make sure to switch to C++/WinRT at the top!
## Validation Steps Performed
Removed all my manual actions, ran the Terminal:
## Summary of the Pull Request
This proposes a change to how Terminal will scroll in response to newly
generated output. The Terminal will scroll upon receiving new output if
the viewport is at the bottom of the scroll history and no selection is
active.
This spec also explores the possibility of making this response
configurable with a `snapOnOutput` profile setting. It also discusses
the possibility of adding a scroll lock keybinding action.
## PR Checklist
* [X] Spec for #980
Update colors of our custom NewTab button to match MUX's TabView button
MUX has a NewTab button, but Terminal uses a homemade lookalike. The
version in Terminal doesn't use the same brush color resources as MUX's
button, so it looks very slightly different. This PR updates Terminal's
button to use the exact same colors that MUX uses. I literally copied
these brush names out of MUX source code.
## References
This is the color version of the layout fix#6766
This is a prerequisite for fixing #5360
## Detailed Description of the Pull Request / Additional comments
The real reason that this matters is that once you flip on
`ApplicationHighContrastAdjustment::None`, the existing colors will not
work at all. The existing brushes are themed to black foreground on a
black background when High Contrast (HC) Black theme is enabled. The
only thing that's saving you is
`ApplicationHighContrastAdjustment::Auto` is automatically backplating
the glyphs on the buttons, which (by design) hides the fact that the
colors are poor. The backplates are those ugly squares inside the
buttons on the HC themes.
Before I can push a PR that disables automatic backplating (set
`ApplicationHighContrastAdjustment` to `None`), we'll need to select
better brushes that work in HC mode. MUX has already selected brushes
that work great in all modes, so it just makes sense to use their
brushes.
The one very subtle difference here is that, for non-HC themes, the
glyph's foreground has a bit more contrast when the button is in
hovered/pressed states. Again this slight difference hardly matters
now, but using the correct brushes will become critical when we try to
remove the HC backplating.
Closes#6812
## Summary of the Pull Request
Let's try and figure out just how many people are actually using Solarized. I emailed @DHowett about this a week ago, but otherwise we don't really have any other tasks for this.
## PR Checklist
* [x] I work here
* [n/a] Requires documentation to be updated
The MUX TabView control has a uniquely-shaped [+] button. TerminalApp
doesn't use it: instead, it has a SplitView button that is styled to
look like MUX's official button. However, it doesn't get the button's
shape right. This PR updates TerminalApp's custom button to look more
like MUX's.
The difference is that MUX only rounds the top two corners, and it uses
a bigger radius. Without matching MUX's radius, the upper-left corner
of the button makes an awkward asymmetric divot with the abutting tab.
There's also a spot in the lower-left corner that just looks like
someone accidentally spilled a few pixels on the floor.
Current appearance before this PR:
New appearance with this PR:
Most important deltas highlighted with red circles:
Note that this PR does *not* attempt to fix the colors. The colors are
also just slightly different from what MUX uses. I'll save that for a
separate PR, since all those screenshots would clutter this up this PR.
## 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.
The VT reset operations `RIS` and `DECSTR` are implemented as a series
of steps, each of which could potentially fail. Currently these
operations abort as soon as an error is detected, which is particularly
problematic in conpty mode, where some steps deliberately "fail" to
indicate that they need to be "passed through" to the conpty client. As
a result, the reset won't be fully executed. This PR changes that
behaviour, so the error state is recorded for any failures, but the
subsequent steps are still run.
Originally the structure of these operations was of the form:
bool success = DoSomething();
if (success)
{
success = DoSomethingElse();
}
But I've now changed the code so it looks more like this:
bool success = DoSomething();
success = DoSomethingElse() && success;
This means that every one of the steps should execute, regardless of
whether previous steps were successful, but the final _success_ state
will only be true if none of the steps has failed.
While this is only really an issue in the conhost code, I've updated
both the `AdaptDispatch` and `TerminalDispatch` classes, since I thought
it would be best to have them in sync, and in general this seems like a
better way to handle multi-step operations anyway.
VALIDATION
I've manually tested the `RIS` escape sequence (`\ec`) in the Windows
Terminal, and confirmed that it now correctly resets the cursor
position, which it wasn't doing before.
Closes#6545
Pretty straightforward. `toggleRetroEffect` will work to toggle the
retro terminal effect on/off.
* Made possible by contributions from #6551, _and viewers like you_
This PR fixes the scheme resolution bug outlined in #5326
The approach is as follows:
* In [SchemeManager.cs], find the first scheme parser that actually
successfully parses the scheme, as opposed to the existing code, which
finds the first scheme parser which _says it can parse the scheme_, as
that logic spuriously returns `true` currently.
* In [XmlSchemeParser.cs] and [JsonParser.cs], ensure that the contents
of the file are read and the contents passed to XmlDocument.LoadXXX,
as this fails with an UriException on WSL otherwise.
* Remove `CanParse` as it is superfluous. The check for a valid scheme
parser should not just check an extension but also if the file exists
- this is best done by the `ParseScheme` function as it already
returns null on failure.
* Add `FileExtension` to the interface because we need it lifted now.
Closes#5326
Before sending calling the `HandleClipboardData` member function on
the `PasteFromClipboardEventArgs` object when we receive a request
from the `TermControl` to send it the clipboard's text content, we
now display a warning to let the user choose whether to continue or
not if the text is larger than 5 KiB or contains the _new line_
character, which can be a security issue if the user is pasting the
text in a shell.
These warnings can be disabled with the `largePasteWarning` and
`multiLinePasteWarning` global settings respectively.
Closes#2349
This is essentially a rewrite of the
`TerminalDispatch::SetGraphicsRendition` method, bringing it into closer
alignment with the `AdaptDispatch` implementation, simplifying the
`ITerminalApi` interface, and making the code easier to extend. It adds
support for a number of attributes which weren't previously implemented.
REFERENCES
* This is a mirror of the `AdaptDispatch` refactoring in PR #5758.
* The closer alignment with `AdaptDispatch` is a small step towards
solving issue #3849.
* The newly supported attributes should help a little with issues #5461
(italics) and #6205 (strike-through).
DETAILS
I've literally copied and pasted the `SetGraphicsRendition`
implementation from `AdaptDispatch` into `TerminalDispatch`, with only
few minor changes:
* The `SetTextAttribute` and `GetTextAttribute` calls are slightly
different in the `TerminalDispatch` version, since they don't return a
pointless `success` value, and in the case of the getter, the
`TextAttribute` is returned directly instead of by reference.
Ultimately I'd like to move the `AdaptDispatch` code towards that way
of doing things too, but I'd like to deal with that later as part of a
wider refactoring of the `ConGetSet` interface.
* The `SetIndexedForeground256` and `SetIndexedBackground256` calls
required the color indices to be remapped in the `AdaptDispatch`
implementation, because the conhost color table is in a different
order to the XTerm standard. `TerminalDispatch` doesn't have that
problem, so doesn't require the mapping.
* The index color constants used in the 16-color `SetIndexedForeground`
and `SetIndexedBackground` calls are also slightly different for the
same reason.
VALIDATION
I cherry-picked this code on top of the #6506 and #6698 PRs, since
that's only way to really get the different color formats passed-through
to the terminal. I then ran a bunch of manual tests with various color
coverage scripts that I have, and confirmed that all the different color
formats were being rendered as expected.
Closes#6725
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
With this commit, terminal will be able to copy text to the system
clipboard by using OSC 52 MANIPULATE SELECTION DAATA.
We chose not to implement the clipboard querying functionality offered
by OSC 52, as sending the clipboard text to an application without the
user's knowledge or consent is an immense security hole.
We do not currently support the clipboard specifier Pc to specify which
clipboard buffer should be filled
# Base64 encoded `foo`
$ echo -en "\e]52;;Zm9v\a"
# Multiple lines
# Base64 encoded `foo\r\nbar`
$ echo -en "\e]52;;Zm9vDQpiYXI=\a"
Closes#2946.
Restores the simple text run analysis and skipping of most of the
shaping/layout steps. Corrects one of the fast-path steps to ensure that
offsets and clusters are assigned.
## References
- Bug #6488
- Bug #6664
- Simple run PR #6206
- Simple run revert PR #6665
- Recycle glyph runs PR #6483
The "correction" functions, by which box drawing analysis is one of
them, is dependent on the steps coming before it properly assigning the
four main vectors of the text layout glyphs: indices, advances, offsets,
and clusters. When the fast path is identified by the code from #6206,
only two of those are fully updated: indices and advances. The offsets
doesn't tend to cause a problem because offsets are rarely used so
they're pretty much always 0 already (but this PR enforces that they're
zero for the simple/fast path.) The clusters, however, were not mapped
for the fast path. This showed itself in one of two ways:
1. Before the recycled runs PR #6483, the cluster map had a 0 in every
field for the stock initialized vector.
2. After the recycled runs PR #6483, the cluster map had the previous
run's mapping in it.
This meant that when we reached the steps where glyph runs were
potentially split during the correction phase for box drawing
characters, unexpected values were present to map the glyph indices to
clusters and were corrected, adjusted, or split in an unexpected
fashion.
For instance, the index out of range bug could appear when the default 0
values appended to the end of the clusters vector were decremented down
to a negative value during the run splitter as the true DWrite cluster
mapper doesn't generate that sort of pattern in the slow path case
without also breaking the run itself.
The resolution here is therefore to ensure that all of the fields
related to glyph layout are populated even in the fast path. This
doesn't affect the slow path because that one always populated all
fields by asking DWrite to do it. The fast path just skips a bunch of
DWrite steps because it can implicitly identify patterns and save a
bunch of time.
I've also identified a few vectors that weren't cleared on reset/reuse
of the layout. I'm clearing those now so the `.resize()` operations
performed on them to get to the correct lengths will fill them with
fresh and empty values instead of hanging on to ones that may have been
from the previous. This should be OK memory perf wise because the act of
`.clear()` on a vector shouldn't free anything, just mark it invalid.
And doing `.resize()` from an empty one should just default construct
them into already allocated space (which ought to be super quick).
## Validation
* [x] Far.exe doesn't crash and looks fine
* [x] "\e[30;47m\u{2500} What \u{2500}\e[m" from #6488 appears
appropriately antialiased
* [x] Validate the "\e[30;47m\u{2500} What \u{2500}\e[m" still works
when `FillGeometry` is nerfed as a quick test that the runs are split
correctly.
* [x] Put `u{fffd} into Powershell Core to make a replacement char in
the output. Then press enter a few times and see that shrunken initial
characters on random rows. Verify this is gone.
Closes#6668Closes#6669
Co-Authored-By: Chester Liu <skyline75489@outlook.com>
## Summary of the Pull Request
This adds a first iteration on the command palette. Notable missing features are:
* Commandline mode: This will be a follow-up PR, following the merge of #6537
* nested and iterable commands: These will additionally be a follow-up PR.
This is also additionally based off the addenda in #6532.
This does not bind a key for the palette by default. That will be done when the above follow-ups are completed.
## References
* #2046 - The original command palette thread
* #5400 - This is the megathread for all command palette issues, which is tracking a bunch of additional follow up work
* #5674 and #6532 - specs
* #6537 - related
## PR Checklist
* [x] Closes#2046
- incidentally also closes#6645
* [x] I work here
* [x] Tests added/passed
* [ ] Requires documentation to be updated - delaying this until it's more polished.
## Detailed Description of the Pull Request / Additional comments
* There's a lot of code for autogenerating command names. That's all in `ActionArgs.cpp`, because each case is so _not_ boilerplate, unlike the rest of the code in `ActionArgs.h`.
## Validation Steps Performed
* I've been playing with this for months.
* Tests
* Selfhost with the team
Occasionally, we get users with corrupt PATH environment variables: they
can't lauch PowerShell, because for some reason it's dropped off their
PATH. We also get users who have stray applications named
`powershell.exe` just lying around in random system directories.
We can combat both of these issues by simply hardcoding where we expect
PowerShell and CMD to live. %SystemRoot% was chosen over %WINDIR%
because apparently (according to Stack Overflow), SystemPath is
read-only and WINDIR isn't.
Refs #6039, #4390, #4228 (powershell was not found)
Refs #4682, Fixes#6082 (stray powershell.exe)
## Summary of the Pull Request
Adds a pair of `ShortcutAction`s for setting the tab color.
* `setTabColor`: This changes the color of the current tab to the provided color, or can be used to clear the color.
* `openTabColorPicker`: This keybinding immediately activates the tab color picker for the currently focused tab.
## References
## PR Checklist
* [x] scratches my own itch
* [x] I work here
* [x] Tests added/passed
* [x] https://github.com/MicrosoftDocs/terminal/pull/69
## Detailed Description of the Pull Request / Additional comments
## Validation Steps Performed
* hey look there are tests
* Tested with the following:
```json
// { "command": "setTabColor", "keys": [ "alt+c" ] },
{ "keys": "ctrl+alt+c", "command": { "action": "setTabColor", "color": "#123456" } },
{ "keys": "alt+shift+c", "command": { "action": "setTabColor", "color": null} },
{ "keys": "alt+c", "command": "openTabColorPicker" },
```
This pull request implements shift+double/triple click. Proper behavior
(as described in #4557) is to only expand one selection point, not both.
Adding the `bool targetStart` was a bit weird. I decided on this being
the cleanest approach though because I still want `PivotSelection` to be
its own helper function. Otherwise, the concept of "pivoting" gets kinda
messy.
## Validation Steps Performed
Manual testing as described on attached issue.
Tests were added for Shift+Click and pivoting the selection too.
Closes#4557
This reverts commit 94eab6e391.
We'll reintroduce this again after making sure it plays nicely with
recycling and box drawing glyphs.
Fixes#6488Fixes#6664
<!-- 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
Add keybinding for renaming a tab
<!-- 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] Fulfills format requirements set by #6567
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [X] Tests passed
* [X] 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: #6567 and here (#6557)
This no longer c loses #6256, as the spec changed.
<!-- 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
* Update _TerminalCursorPositionChanged to use ThrottledFunc.
* Rename previous ThrottledFunc to ThrottledArgFunc because now
ThrottledFunc is for functions that do not take an argument.
* Update ThrottledFunc and ThrottledArgFunc to accept a CoreDispatcher
on which the function should be called for convenience.
* Don't use coroutines/winrt::fire_and_forget in
ThrottledFunc/ThrottledArgFunc because they are too slow (see PR).
_AdjustCursorPosition went from 17% of samples to 3% in performance
testing.
Replace std::map with std::unordered_map when the order doesn't matter
and hash functions are provided. Simple optimizations, but I expect the
performance should be strictly better, especially for
CodepointWidthDetector.hpp.
<!-- 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
## Summary of the Pull Request
This is another iteration on the Command Palette spec, from #5674. These were some ideas that were tossed around by @DHowett, @cinnamon-msft and myself, formalized here. I proposed this as an addendum to the original spec, since I think the first made sense atomically, and this only makes sense as a set of changes to the original. I didn't want to go hacking up the original doc to add this set of changes.
**There are two proposals in this spec - they should be viewed as two atomic units. They can be accepted or rejected independently. I'm suggesting we approve both. They work _together_. I'm realizing now that this is worded confusingly, and it's on me to fix that.**
## PR Checklist
* [x] Another spec in the #2046 / #5400 saga
* [x] I work here
* [x] _is a doc_
> ## Abstract
>
> This document is intended to serve as an addition to the [Command Palette Spec].
> While that spec is complete in it's own right, subsequent discussion revealed
> additional ways to improve the functionality and usability of the command
> palette. This document builds largely on the topics already introduced in the
> original spec, so readers should first familiarize themselves with that
> document.
>
> One point of note from the original document was that the original specification
> was entirely too verbose when defining both keybindings and commands for
> actions. Consider, for instance, a user that wants to bind the action "duplicate
> the current pane". In that spec, they need to add both a keybinding and a
> command:
>
> ```json
> {
> "keybindings": [
> { "keys": [ "ctrl+alt+t" ], "command": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" } },
> ],
> "commands": [
> { "name": "Duplicate Pane", "action": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" }, "icon": null },
> ]
> }
> ```
>
> These two entries are practically the same, except for two key differentiators:
> * the keybinding has a `keys` property, indicating which key chord activates the
> action.
> * The command has a `name` property, indicating what name to display for the
> command in the Command Palette.
>
> What if the user didn't have to duplicate this action? What if the user could
> just add this action once, in their `keybindings` or `commands`, and have it
> work both as a keybinding AND a command?
>
<!-- 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
When the user double clicks on a tab, show the tab rename box
as if they right clicked on the tab and clicked on "Rename".
<!-- 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#6600
* [x] 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
I added a handler for the `DoubleTapped` event on the tab view item
when we are constructing it for the tab (in `Tab::_MakeTabViewItem`).
The code for that handler was copied the "rename tab menu item" click
handler.
I did not extract the code into a member function because it is very
short (only 2 lines of code) and only used twice so it is not worth
it IMO.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
This saves an awful lot of construction/destruction and memory
allocation, especially with text that changes a lot (see: cacafire).
Three things:
1. Recycling the text layouts. This holds onto the `CustomTextLayout` so
all the things that don't change related to drawing targets and
whatnot aren't freed and recreated every frame.
2. Reordering the runs in place. This saves a vector
allocation/copy/delete every time OrderRuns is called. They can be
rearranged in place.
3. Only clip once per row. This reduces the clip push/pop to only one
time per row. Since we're always redrawing an entire row at a time,
this saves a lot of alloc/free of the clip frame, dramatically
reduces queued commands, and makes less work on the flush since
clipping requires staging the drawing and then bringing it back to
the main surface.
I was told that the DeviceContext version supercedes the RenderTarget
one. This moves us to it so we can gain access to a higher level of
control over the various pieces in our pipeline as we continue to evolve
the renderer.
The underlying motivation here is to potentially use a
`ID2D1CommandList` to batch our commands and run them all later outside
the lock. That can only really be done with this more granular level of
control over the pipeline. So this moves to that in a single step that
is easily findable in history should we have problems
I discussed this with @NiklasBorson of the Direct2D/DirectWrite team as
well as with @DHowett before doing it.
## Validation
- [x] Checked docs to make sure that these work on Windows 7 with
Platform Update
- [x] Manual smoke test real quick
- [ ] Try running on Win7 + Platform Update after change
- [x] Probably do more than just a smoke test manually or otherwise
Closes#6525
`bitmap::_calculateArea` performance can be improved by leveraging the
optimized `find_first`/`find_next` methods instead of iterating through
the bitmap manually.
## Summary of the Pull Request
## PR Checklist
* [x] Closes#3927
* [x] I work here.
* [x] Tested manually.
* [x] Requires documentation to be updated: (generate doc bug here)
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
- I found four settings that weren't hot reloadable with the 3927 comment above them:
1. Experimental retro terminal effect
2. Experimental software rendering
3. Experimental full repaint rendering
4. Antialiasing settings for text
I made them all hot reloadable by telling the `TermControl` to propagate them on settings change to the `DxEngine`.
Then I set up the `DxEngine` inside the setters to only set them if they changed. And if they do change, to trigger a full repaint and/or a complete drop and recreate of the entire DX device chain (as would happen if it were lost for another reason like a user-mode graphics failure, disconnected display, etc.)
I made the boolean an atomic because the settings can be coming in off of another thread (the XAML eventing one) and the renderer is picking the status up on its thread at the top of the BeginPaint frame.
## Validation Steps Performed
- [x] Opened it up and toggled all the settings while staring at PowerShell
- [x] Opened it up and toggled all the settings while staring at something intensive like a `cacafire` fire
