Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[pull] main from microsoft:main #210

Merged
merged 20 commits into from
Aug 10, 2024
Merged

[pull] main from microsoft:main #210

merged 20 commits into from
Aug 10, 2024

Conversation

pull[bot]
Copy link

@pull pull bot commented Aug 9, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

j4james and others added 18 commits August 5, 2024 14:54
This PR adds support for querying the cursor style - technically the
state of the `DECSCUSR` setting - using a `DECRQSS` escape sequence.

## References and Relevant Issues

The initial `DECRQSS` support was added in PR #11152, but it wasn't
practical to report the cursor style until conpty passthrough was added
in PR #17510.

## Detailed Description of the Pull Request / Additional comments

If the user has chosen a cursor style that isn't one of the shapes
supported by the `DECSCUSR` control, we report those as 0 (i.e. the
default style). That way, if an application later tries to restore the
cursor using the returned value, it should still be reset to its
original state.

I also took the opportunity in this PR to do some refactoring of the
other `DECRQSS` reports, since several of them were using unnecessary
appending that could be simplified to a single `fmt::format` call, or
even just static strings in some cases.

## Validation Steps Performed

I've checked the reports are working as expected in Vttest, and also
added some unit tests.

## PR Checklist
- [x] Tests added/passed
Roughly 4 years ago we gave Windows Terminal the ability to
differentiate between black/white and the default colors.
One of the victims was PowerShell and most importantly PSReadLine,
which emit SRG 37 & 40 when what they really want is 38 & 48.
We fixed this on our side by adding a shim.

Since the addition of VT passthrough in #17510 we now intentionally
lost the ability to translate VT sequences from one thing to another.
This meant we also lost the ability to do this shim and as such
this PR removes it. Luckily Windows 11 now ships PSReadLine 2.0.0,
which contains a proper fix for this.

Unfortunately, this is not the case for Windows 10, which ships
PSReadLine 2.0.0-beta2. Users affected by this will have to install
a newer version of PSReadLine or use the default black/white theme.

See 1bf4c08

Closes #13037
One of these things can be optimized. It's not the one you thought.
BackendD2D will now draw one extra cell on all sides when rendering the
background, filled with the expected background color, starting at (-1,
-1) to ensure that cell backgrounds do not bleed over the edges of the
viewport where the is swapchain but no content.

Fixes #17672
This simply makes the project compile again.
This pulls one of the inlines in AtlasEngine out as a helper so we can
use it elsewhere.
* Every single place that called `read_file_as_utf8_string_if_exists`
  would immediately do a `.value_or(std::string{})`.
  As such, the function now returns a string directly.
* There was just one caller to `read_file_as_utf8_string`
  and it only cared about files that are non-empty.
  As such, the specialization got removed.

Both of these make sense to me, as in practice there's seldom
a difference between an empty file and a non-existent one.

## Validation Steps Performed
* Compiles ✅
* Starts ✅
* Deleting the `settings.json` contents triggers a reload ✅
As explained in the updated code comment, we can actually safely
allow more types to enjoy the fast-pass in `til::small_vector`.
* Added/changed comments as mentioned.
* Improved the ugly `resize_and_overwrite` hack into the STL.
* Add `Write` functions for xterm's window API.
* The only reason we needed a move operator for `VtIo::Writer`
  is because we used it in a ternary in `CONSOLE_INFORMATION`.
  Ternaries are like if branches with hidden move assignments.
  Instead, we simply construct each `Writer` in place.
  No ternary = No move = No problems in life.
  The best benefit of this is that this makes calling `GetVtWriter`
  a hundred times cheaper.

Otherwise, I still need to extend a few tests in `VtIoTests`,
but I'm planning to do that later.
## Summary of the Pull Request
Adds a scroll offset to avoid hiding the current search highlight with
the search box.
- Offset is based on the number of rows that the search box takes up.
  (I am not totally sure I am calculating this right)
- This won't help when the current highlight is in the first couple
  rows of the buffer.

Fixes: #4407
This adds an indirection for `_KeyHandler` so that `OnDirectKeyEvent`
can call `_KeyHandler`. This allows us to consistently handle
Alt-key-up events. Then I added custom handling for Alt+ddd (OEM),
Alt+0ddd (ANSI), and Alt+'+'+xxxx (Unicode) sequences, due to the
absence of Alt-key events with xaml islands and our TSF control.

Closes #17327

## Validation Steps Performed
* Tested it according to https://conemu.github.io/en/AltNumpad.html
* Unbind Alt+Space
* Run `showkey -a`
* Alt+Space generates `^[ `
* F7 generates `^[[18~`
This delays the CSI J until we know the new origin of the prompt.
That way it's at the right (reflowed) position.

## Validation Steps Performed
* conhost
  * Print a ton of text
  * Write a prompt of a hundred chars
  * Resize the window very narrow / wide
  * Works ✅
* Windows Terminal
  * Write a prompt of a hundred chars
  * Resize the window very narrow / wide
  * Works ✅
…17679)

This fixes several scaling issues with double width/height rows.

## Validation Steps Performed
* Run `RenderTests` ✅
In several places the old conhost codebase appears to assume that any
wide glyph is represented by two codepoints. This is probably an
artifact of the ASCII/DBCS split that conhost used to have.
When conhost got merged into a single UCS2-aware application,
this artifact was apparently never properly resolved.

To my knowledge there are at least two places where this assumption
exists: The clipboard code which translates non-wide non-ascii
characters to Alt-numpad sequences, and this code. Both are wrong.
This is because in a Unicode-context there's no correlation between
the number of codepoints and the width of the glyph, even with UCS2.

In a post-UCS2-world the correct check is for surrogate pairs,
as they must be avoided for the same reason DBCS were avoided.

One could consider this a breaking change of the API,
as this can now result in repeat counts >1 for wide glyphs.
If someone complained about this change in behavior, I'd probably
not change it back, as narrow complex Unicode characters exist too.
The answerback feature allows for the user to define a message that the
terminal will transmit to the host whenever an `ENQ` (enquiry) control
character is received.

## Detailed Description of the Pull Request / Additional comments

In Windows Terminal, the message can be configured at the profile level
of the settings file, as a string property named `AnswerbackMessage`.

In ConHost, the message can be configured in the registry, again as a
string value with the name `AnswerbackMessage`.

## Validation Steps Performed

I've confirmed that the control is working as intended in both Windows
Terminal and ConHost using Vttest.

Closes #11946
This sends a telemetry event if a session is interacted with.
Specifically, key events are essential to have an interactive session in
Windows Terminal, so we're tracking sessions that have had a key down
event.
Between fmt 7.1.3 and 11.0.2 a lot has happened. `wchar_t` support is
now more limited and implicit conversions don't work anymore.

Furthermore, even the non-`FMT_COMPILE` API is now compile-time checked
and so it fails to work in our UI code which passes `hstring` format
strings which aren't implicitly convertible to the expected type.
`fmt::runtime` was introduced for this but it also fails to work for
`hstring` parameters. To solve this, a new `RS_fmt` macro was added
to abstract the added `std::wstring_view` casting away.

Finally, some additional changes to reduce `stringstream` usage
have been made, whenever `format_to`, etc., is available.
This mostly affects `ActionArgs.cpp`.

Closes #16000

## Validation Steps Performed
* Compiles ✅
* Settings page opens ✅
The only reason we had the `SetTextAttributes` method in `ITerminalApi`
was to allow for conhost to remap the default color attributes when the
VT PowerShell quirk was active. Since that quirk has now been removed,
there's no need for this API anymore.

## References and Relevant Issues

The PowerShell quirk was removed in PR #17666.

## Validation Steps Performed

I've had to update all the attribute tests in adapterTest to manually
check the expected attributes, since those checks were previously being
handled in a `SetTextAttributes` mock which no longer exists.

I've also performed some manual tests of the VT attribute operations to
double check that they're still working as expected.
@pull pull bot added the ⤵️ pull label Aug 9, 2024
lhecker and others added 2 commits August 9, 2024 15:33
This simplifies the code (from the perspective of the CPU) by doing
some miniscule-feels-good optimizations like replacing `snprintf` with
regular string concatenation and by doing an actual optimization by
removing the remaining calls to the WinRT `ApplicationModel` namespace.

More importantly however it fixes a bug: The only reason `elevate-shim`
worked at all is because the shell extension passed "wrong" parameters
to `CreateProcess`. Instead of repeating the application path in the
command line argument again, as is convention in C and on Windows, and
getting the 2nd and following parameters as an argument to `wWinMain`,
it used `GetCommandLineW` to get the original, broken command line.
This fixes the issue by passing the application path as the first
argument, which allows `elevate-shim` to be called like any other app.

## Validation Steps Performed
* Deploy WT and restart explorer
* Clicking "Open in Terminal (Dev)" works ✅
* Clicking "Open in Terminal (Dev)" while holding Ctrl+Shift
  opens WT as admin ✅
In #17638, I am moving selection to an earlier phase of rendering (so
that further phases can take it into account). Since I am drafting off
the design of search highlights, one of the required changes is moving
to passing `span`s of `point_span`s around to make selection effectively
zero-copy.

We can't easily have zero-copy selection propagation without caching,
and we can't have caching without mandatory cache invalidation.

This pull request moves both conhost and Terminal to use
`til::generational` for all selection members that impact the ranges
that would be produced from `GetSelectionRects`.

This required a move from `std::optional<>` to a boolean to determine
whether a selection was active in Terminal.

We will no longer regenerate the selection rects from the selection
anchors plus the text buffer *every single frame*.

Apart from being annoying to read, there is one downside.

If you begin a selection on a narrow character, _and that narrow
character later turns into a wide character_, we will show it as
half-selected.

This should be a rare-enough case that we can accept it as a regression.
@pull pull bot merged commit 0199ca3 into l3dlp-sandbox:main Aug 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants