As we start to work on implementing Action IDs, the spec written a few
years ago needs some updates. This PR makes those updates for the
current implementation plan.
References #6899
Right now, the localization submission pipeline runs every night and
sends our localizable resources over to Touchdown. Later, release builds
pick up the localizations directly from Touchdown, move them into place,
and consume them.
This allowed us to avoid having localized content in the repository, but
it came with too many downsides:
- Users could not contribute additional localizations very easily
- We use the same release pipeline and Touchdown configuration for every
branch, so strings needed to either slightly match or _entirely match_
across an entire set of active release branches
- Building from day to day can pull in different strings, making the
product not reproduceable
- Calling TDBuild during release builds requires network access from the
build machine (so does restoring NuGet packages, but that's neither
here nor there)
- Local developers and users could not test out other languages
This pull request moves all localization processing into the nightly
build phase and introduces support for checking loc in and submitting a
pull request. The pull request will not be created anew if one already
exists which has not been merged.
Anything we needed to do on release is now done overnight. This includes
moving loc files into the right places and merging the Cascadia
resources with the Context Menu resources (so that we can work around a
relatively lower amount of translations being chosen for the app versus
the context menu; see #12491 for more info.)
There are some smaller downsides to this approach and its
implementation:
- The first commit is going to be huge
- Right now, it only manages a single branch and uses a force push; if a
PR is not reviewed timely, it will be force-pushed and you cannot see
the day-to-day changes in the strings. Hopefully there won't be any.
I've taken great care to ensure that repeated runs of this new pipeline
will not result in unnecessary whitespace changes. This required
changing how we merge ContextMenu.resw into CascadiaPackage to always
use the .NET XmlWriter with specific flags.
NOTE that this does not allow users to _contribute_ translation fixes
for the 10 languages which we are importing. We will still need to pull
changes out of those files and submit them as bugs to the localization
team separately, and hope they come back around in another nightly
build. However, there is no reason users cannot contribute
_non-Touchdown_ languages.
We don't need to use `stringstream` to generate a ten-character string,
and we for _sure_ don't need to use the locale-aware ctype functions
after we just wrote a comment saying "XOrg colors are always Latin-1"
| Size Diff | Object | Library |
| --------- | -------------- | -------- |
| -11.8 KB | colorTable.obj | ConTypes |
I realize I might be one of the few developers that care about custom
shader support in terminal but I thought it's worth proposing it and see
what you think.
This is to support custom shaders with custom textures.
I was thinking of exposing the background image to the shader but that
felt complicated after looking into it.
I have tested exploratively. I think the texture loader is possible to
unit test so that is a possible improvement.
The error reporting (as with other custom pixel shader code) is not very
good. That is also an area that I could improve upon.
I do think the risk of adding this is rather low as the new code is only
executed when experimental.pixelShaderImagePath is set.
### Details
Only added to the Atlas engine.
Instead I load the texture using WIC into a shader resource view. When
binding shader resources I test for presence of custom texture and bind
it to register t1.
The image loading code was found in [the D3D Texture documentation].
It's a mouthful but seems rather robust.
Tested setting: "experimental.pixelShaderImagePath"
1. Tested not specifying it.
2. Tested setting it.
3. Tested changing it (the changes are picked up)
4. Tested invalid path
5. Tested a custom shader that made use of the custom texture.
[the D3D Texture documentation]: https://learn.microsoft.com/en-us/windows/win32/direct3d11/overviews-direct3d-11-resources-textures-how-to
Co-authored-by: Mike Griese <migrie@microsoft.com>
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
"ConptyConnection::CreateSettings()" was modified to include some extra
parameters related to the environment variable changes. This just
updates the call in Scratch.sln so that it builds and deploys properly.
When using the legacy console APIs, it's possible to write arbitrary
codepoints into the buffer. If any of those codepoints are in the C0 or
C1 range, and the buffer contents are forwarded over conpty, they can
end up mistakenly interpreted as controls by the connected terminal.
This PR fixes that issue by converting any C0 and C1 codepoints in the
buffer into printable glyphs before forwarding them over conpty. I've
used the C0 glyphs from the DOS 437 codepage and just a `?` for the C1
codepoints, since that's what you would typically have seen in the v1
console with a raster font.
Although this doesn't address the main problem in #16410, it should at
least fix the rendering issues they're seeing when running their app in
Windows Terminal.
I've confirmed that the test case in #4363 now looks the same in Windows
Terminal as it does in conhost, and I've tested the Windows version of
the terminal game [Gorched], and confirmed that it now works correctly
in Window Terminal.
[Gorched]: https://github.com/zladovan/gorchedCloses#4363Closes#6265
Welp, would you look at that? We never actually supported "canary"
feature settings. Canary's been defaulting to the "Dev" config since
inception.
There's a couple things that are supposed to only be on in Dev and not
Canary. They clearly haven't mattered, but better safe than sorry!
This PR adds support for the `OSC 21` sequence used on DEC terminals to
set the window title. It's just an alias of the `OSC 2` title sequence
used by XTerm.
This PR also corrects the handling of blank title sequences, which are
supposed to reset the title to its default value, but were previously
ignored.
## Detailed Description of the Pull Request / Additional comments
To handle the blank title parsing correctly, I had to make some changes
to the state machine. Previously it would not have dispatched an `OSC`
sequence unless it received a semicolon following the `OSC` number, but
when there's a blank string, that semicolon should not be required.
I also took this opportunity to simplify the `OSC` parsing in the state
machine, and eliminate the `_GetOscTitle` method which no longer served
any purpose.
## Validation Steps Performed
I've manually confirmed that the title sequences are now working as
expected, and added some state machine unit tests covering the blank
value handling for these sequences.
I also had to update one of the existing state machine tests to account
for the changes I made to allow the semicolon to be omitted.
Closes#16783Closes#16784
This pull request introduces the module Microsoft.Terminal.UI.dll, and
moves into it the following things:
- Any `IDirectKeyListener`
- All XAML converter helpers from around the project
- ... including `IconPathConverter` from TerminalSettingsModel
- ... but not `EmptyStringVisibilityConverter`, which has died
It also adds a XAML Markup Extension named `mtu:ResourceString`, which
will allow us to refer to string resources directly from XAML. It will
allow us to remove all of the places in the code where we manually set
resources on XAML controls.
---------
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
Make sure the delete button's `Tag` updates when the selected
axis/feature changes, so that the correct key value gets propagated when
the delete button is clicked.
Refs #16678#16104
## Validation Steps Performed
1. Add a new feature/axis
2. Change the key
3. Click the delete button
4. Delete button works
Aside from overall simplifying `CharToColumnMapper` this fixes 2 bugs:
* The backward search loop may have iterated 1 column too far,
because it didn't stop at `*current <= *target`, but rather at
`*(current - 1) <= *target`. This issue was only apparent when
surrogate pairs were being used in a row.
* When the target offset is that of a trailing surrogate pair
the forward search loop may have iterated 1 column too far.
It's somewhat unlikely for this to happen since this code is
only used through ICU, but you never know.
This is a continuation of PR #16775.
This is just a minor cleanup I did as a drive-by while working on
customized font fallback. The benefit of this change is that it's
a tiny bit less expensive, but also that it's a lot easier to read.
The split into "get index" and "get string by index" helps us to
more easily handle both, missing locales and locale fallback.
The code that ties everything together then ends up being just 7 lines.
## Summary of the Pull Request
Allow editing of font features and axes in the SUI to get the UI closer
towards JSON parity
The allowed font axes are obtained directly from the currently selected
font, and their display names are presented to the user in the user's
current locale (if it exists). Otherwise, we just display the axis tag
to the user.
## References and Relevant Issues
#10000
## Validation Steps Performed
- [x] Font Axes can be added/changed/removed from the Settings UI
## PR Checklist
- [ ] Closes #xxx
- [ ] 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 (if necessary)
This pull request removes the need for the SettingsModel tests to run in
a UAP harness and puts them into the standard CI rotation.
This required some changes to `Run-Tests.ps1` to ensure that the right
`te.exe` is selected for each test harness. It's a bit annoying, but for
things that depend on a `resources.pri`, that file must be in the same
directory as the EXE that is hosting the test. Not the DLL, mind you,
the EXE. In our case, that's `TE.ProcessHost.exe`
The bulk of the change is honestly namespace tidying.
Co-authored-by: Mike Griese <migrie@microsoft.com>
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
Shaded glyphs (U+2591..3, etc.) all have one problem in common:
The cell size may not be evenly divisible by the pixel/dot size in
the glyph. This either results in blurring, or in moiré-like patterns
at the edges of the cells with its neighbors, because they happen to
start with a pattern that overlaps with the end of the previous cell.
This PR solves the issue by moving the pixel/dot pattern generation
into the shader. That way the pixel/dot location can be made dependent
on the viewport-position of the actual underlying pixels, which avoids
repeating patterns between cells.
The PR contains some additional modifications, all of which either
extend or improve the existing debug facilities in AtlasEngine.
Suppressing whitespaces changes makes the diff way smaller.
`wstring_case_insensitive_compare` is not a great name for what it
does as it's incorrect to use for regular (human readable) strings.
This PR thus renames it to `env_key_sorter`.
`compare_string_ordinal` was renamed to `compare_ordinal_insensitive`
to make sure callers know that the comparison is insensitive
(this may otherwise be incorrect in certain contexts after all).
The return value was changed to match `memcmp` so that the API
is detached from its underlying implementation (= NLS).
`compare_linguistic_insensitive` and `contains_linguistic_insensitive`
were added to sort and filter human-readable strings respectively.
`prefix_split` was extended to allow for needles that are just a
single character. This significantly improves the generated assembly
and is also usually what someone would want to actually use.
I've left the string-as-needle variant in just in case.
This PR is prep-work for #2664
## Summary of the Pull Request
URL detection was broken again in #15858. When the regex matched, we
calculate the column(cell) by its offset, we use forward or backward
iteration of the column to find the correct column that displays the
glyphs of `_chars[offset]`.
abf5d9423a/src/buffer/out/Row.cpp (L95-L104)
However, when calculating the `currentOffset` we forget that MSB of
`_charOffsets[col]` could be `1`, or col is pointing to another glyph in
preceding column.
abf5d9423a/src/buffer/out/Row.hpp (L223-L226)
Up until now, we have treated inbox, fragment and user color schemes the
same: we load them all into one big map and when we save the settings
file we write them *all* out. It's been a big annoyance pretty much
forever.
In addition to cluttering the user's settings file, it prevents us from
making changes to the stock color schemes (like to change the cursor
color, or to adjust the colors in Tango Dark, or what have you) because
they're already copied in full in the user settings. It also means that
we need some special UI affordances for color schemes that you are
allowed to view but not to delete or rename.
We also have a funny hardcoded list of color scheme names and we use
that to determine whether they're "inbox" for UI purposes.
Because of all that, we are hesitant to add *more* color schemes to the
default set.
This pull request resolves all of those issues at once.
It:
- Adds an "origin" to color schemes indicating where they're from
(Inbox, Fragment, User, ...)
- Replaces the Edit UI with a much simpler version that pretty much only
has a "duplicate this color scheme to start editing it" button
- Deletes color schemes that we consider to be equivalent to inbox ones;
this allows us to finally disentangle the user's preferences from the
terminal's.
- Migrates all user settings that referred to schemes they may have
modified (even implicitly!) to their modified versions.
The equivalence check intentionally leaves out the cursor and selection
colors, so that we have the freedom to change them in the future.
The Origin is part of a new interface, `ISettingsModelObject`, which we
can use in the future for things like Themes and Actions.
I thought, "what if I could just have a script make all the releases,
tags and names and upload all the assets to the right place?"
So, here's that script.
Basically, title. If you null out the icon, we'll automatically try to
use the `commandline` as an icon (because we can now). We'll even be
smart about it - `cmd.exe /k echo wassup` will still just use the ico of
`cmd.exe`.
This doesn't work for `ubuntu.exe` (et. al), because that commandline is
technically a reparse point, that doesn't actually have an icon
associated with it.
Closes#705
`"none"` becomes our sentinel value for "no icon".
This will also use the same `NormalizeCommandLine` we use for
commandline matching for finding the full path to the exe.
Surprisingly easier than I thought this would be. ActionMap already
supports layering (from defaults.json), so this basically re-uses a lot
of that for fun and profit.
The trickiest bits:
* In `SettingsLoader::_parseFragment`, I'm constructing a fake, empty
JSON object, and taking _only_ the actions out from the fragment, and
stuffing them into this temp json. Then, I parse that as a globals
object, and set _that_ as the parent to the user settings file. That
results in _only_ the actions from the fragment being parsed before the
user's actions.
* In that same method, I'm also explicitly preventing the ActionMap (et
al.) from parsing `keys` from these actions. We don't want fragments to
be able to say "ctrl+f is clear buffer" or something like that. This
required a bit of annoying plumbing.
Closes#16063
Tests added.
Docs need to be updated.
This includes a fix for the hang on shutdown due to the folder change
reader.
WIL now validates format strings in `LOG...` macros (yay!) and so we
needed to fix some of our `LOG` macros.
Closes#16456
(cherry picked from commit ce30e7c89c)
Service-Card-Id: 91923199
Service-Version: 1.19
This changeset makes 3 improvements:
* Dotted lines now use a 2:1 ratio between gaps and dots (from 1:1).
This makes the dots a lot easier to spot at small font sizes.
* Dashed lines use a 1:2 ratio and a cells-size independent stride.
By being cell-size independent it works more consistently with a
wider variety of fonts with weird cell aspect ratios.
* Curly lines are now cell-size independent as well and have a
height that equals the double-underline size.
This ensures that the curve isn't cut off anymore and just like
with dashed lines, that it works under weird aspect ratios.
Closes#16712
## Validation Steps Performed
This was tested using RenderingTests using Cascadia Mono, Consolas,
Courier New, Lucida Console and MS Gothic.
(cherry picked from commit 9c8058c326)
Service-Card-Id: 91922825
Service-Version: 1.19
This adds support for drawing our own box drawing, block element,
and basic Powerline (U+E0Bx) glyphs in AtlasEngine.
This PR consists of 4 parts:
* AtlasEngine was refactored to simplify `_drawGlyph` because I've
wanted to do that for ~1 year now and never got a chance.
Well, now I'm doing it and you all will review it muahahaha.
The good news is that it removes a goto usage that even for my
standards was rather dangerous. Now it's gone and the risk with it.
* AtlasEngine was further refactored to properly parse out text that
we want to handle different from regular text. Previously, we only
did that for soft fonts, but now we want to do that for a lot more,
so a refactor was in order. The new code is still extremely
disgusting, because I now stuff `wchar_t`s into an array that's
intended for glyph indices, but that's the best way to make it fast
and not blow up the complexity of the code even further.
* Finally this adds a huge LUT for all the aforementioned glyphs.
The LUT has 4 "drawing instruction" entries per glyph which describe
the shape (rectangle, circle, lines, etc.) and the start/end coord.
With a lot of bit packing each entry is only 4 bytes large.
* Finally-finally a `builtinGlyphs` setting was added to the font
object and it defaults to `true`.
Closes#5897
## Validation Steps Performed
* RenderingTests with soft fonts ✅
* All the aforementioned glyphs ✅
* ...with color ✅
* `customGlyphs` setting can be toggled on and off ✅
`ConsoleBench` is capable of launching conhost instances and measuring
their performance characteristics. It writes these out as an HTML file
with violin graphs (using plotly.js) for easy comparison.
Currently, only a small number of tests have been added, but the code
is structured in such a way that more tests can be added easily.
This changeset makes 3 improvements:
* Dotted lines now use a 2:1 ratio between gaps and dots (from 1:1).
This makes the dots a lot easier to spot at small font sizes.
* Dashed lines use a 1:2 ratio and a cells-size independent stride.
By being cell-size independent it works more consistently with a
wider variety of fonts with weird cell aspect ratios.
* Curly lines are now cell-size independent as well and have a
height that equals the double-underline size.
This ensures that the curve isn't cut off anymore and just like
with dashed lines, that it works under weird aspect ratios.
Closes#16712
## Validation Steps Performed
This was tested using RenderingTests using Cascadia Mono, Consolas,
Courier New, Lucida Console and MS Gothic.
With AtlasEngine being fairly stable at this point and being enabled
by default in the 1.19 branch, this changeset removes DxEngine.
## Validation Steps Performed
* WT builds and runs ✅
* WpfTestNetCore ✅
* Saving the config removes the `useAtlasEngine` key ✅
## Summary of the Pull Request
This PR adds support for the missing operations that are required in the
8-bit interface architecture extension.
`DECAUPSS` - Assign User-Preference Supplemental Set
`DECRQUPSS` - Request User-Preference Supplemental Set
`ACS` - Announce Code Structure for ANSI levels 1, 2, and 3
## Detailed Description of the Pull Request / Additional comments
For the UPSS assignment there's a new `wstring_view` in `TerminalOutput`
that tracks the current mapping table for the character set, and a new
`VTID` field tracking the ID so it can be queried. The ANSI conformance
just required a couple of calls to existing methods to designate the
appropriate character sets and GL/GR maps.
And now that we've implemented everything that's required, I've updated
our device attributes report (`DA1`) to indicate support for the 8-bit
interface architecture (feature 14).
This PR also addresses some issues with the way the 8-bit GR capability
is managed. Previously a soft reset (`DECSTR`) would return the code
page to its startup state, and the restore cursor operation (`DECRC`)
could have a similar effect. This was a problem for 8-bit apps that
weren't expecting that behavior.
I've now made it so those operations no longer have any effect on the
code page, and the same applies to the C1 parsing option (`DECAC1`). The
only way to restore the code page and C1 parsing to their startup state
now is with a hard reset (`RIS`).
## Validation Steps Performed
I've added unit tests covering the `DECAUPSS` and `DECRQUPSS` sequences,
and I've also manually tested their behavior in Vttest.
## PR Checklist
- [x] Closes#16546
- [x] Tests added/passed
There's an apparent miscompilation of `dynamic_bitset` on x86 with
MSVC 19.38, where the following code:
```cpp
dynamic_bitset<uint64_t> bits(80 * 24, 0);
bits.set(0, 3 * 80, true);
```
is expected to set the first 3.75 blocks to 1 which should produce
the following blocks:
```
0xffffffffffffffff
0xffffffffffffffff
0xffffffffffffffff
0x0000ffffffffffff
```
but it actually produces:
```
0xffffffffffffffff
0x00000000ffffffff
0xffffffffffffffff
0x0000ffffffffffff
```
The weird thing here is that this only happens if `til::bitmap`
uses a `dynamic_bitset<uint64_t>`. As soon as it uses `<uint32_t>`
any other instantiation of `<uint64_t>` is magically fixed.
Conclusion: Use `size_t` until we know what's going on.
Last known good CL version: 14.37.32822
Current broken CL version: 14.38.33130
## Validation Steps Performed
The following test completes successfully again:
```cpp
til::bitmap map{ { 80, 24 } };
map.translate({ 0, 3 }, true);
VERIFY_ARE_EQUAL(3u, map.runs().size());
```
(cherry picked from commit d3ec47a7fc)
Service-Card-Id: 91885583
Service-Version: 1.19
There's an apparent miscompilation of `dynamic_bitset` on x86 with
MSVC 19.38, where the following code:
```cpp
dynamic_bitset<uint64_t> bits(80 * 24, 0);
bits.set(0, 3 * 80, true);
```
is expected to set the first 3.75 blocks to 1 which should produce
the following blocks:
```
0xffffffffffffffff
0xffffffffffffffff
0xffffffffffffffff
0x0000ffffffffffff
```
but it actually produces:
```
0xffffffffffffffff
0x00000000ffffffff
0xffffffffffffffff
0x0000ffffffffffff
```
The weird thing here is that this only happens if `til::bitmap`
uses a `dynamic_bitset<uint64_t>`. As soon as it uses `<uint32_t>`
any other instantiation of `<uint64_t>` is magically fixed.
Conclusion: Use `size_t` until we know what's going on.
Last known good CL version: 14.37.32822
Current broken CL version: 14.38.33130
## Validation Steps Performed
The following test completes successfully again:
```cpp
til::bitmap map{ { 80, 24 } };
map.translate({ 0, 3 }, true);
VERIFY_ARE_EQUAL(3u, map.runs().size());
```
Write "<command that doesn't exist> foo" in cmd.exe, move yours cursor
past the <command> and press tab. The "foo" will still be there but
will be inaccessible. This commit fixes the issue. As far as I can
tell, this never worked in any conhost version ever.
Closes#16704
(cherry picked from commit 5e9f223a6c)
Service-Card-Id: 91851714
Service-Version: 1.19
Write "<command that doesn't exist> foo" in cmd.exe, move yours cursor
past the <command> and press tab. The "foo" will still be there but
will be inaccessible. This commit fixes the issue. As far as I can
tell, this never worked in any conhost version ever.
Closes#16704
Changes recently landed in `ge_release_we_adept_dev` that result in
conhost loading `Windows.Storage.dll` during startup, rather than
`shell32`. This causes a memory usage regression.
The only thing we need from `shell32` during startup is
`ExtractIconExW`. Fortunately, that function is relatively simple and
it relies only on things from `user32` (and one thing from `comctl32`).
Enclosed herein is an implementation of `ExtractIconExW` that is tuned
for conhost's specific use case and tidied up.
Related work items: MSFT-48947348
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/ge_release_we_adept_dev c54c102a362c3dbc7a64cc148b45b993b4154ead
## Summary of the Pull Request
Implements an `ActionsViewModel` for the `Actions` page in the SUI
## References
## 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
* [ ] 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: #xxx
## Detailed Description of the Pull Request / Additional comments
A few annoyances:
- Because of the shifts in the UI when switching between edit
mode/regular mode on the `KeyBindingViewModel`s, the page used to
manually handle moving focus accordingly so that focus does not get
lost. However, the page no longer owns the `KeyBindingViewModel`,
instead the `ActionsViewModel` owns them but the `ActionsViewModel`
cannot manually move focus around since it does not own the UI Element.
So, the `ActionsViewModel` emits an event for the page to catch that
tells the page to move focus (`FocusContainer`).
- Similarly, the page used to manually update the `ContainerBackground`
of the `KeyBindingViewModel` when the kbdVM enters `EditMode`. The
`ActionsViewModel` does not have access to the page's resources though
(to determine the correct background brush to use). So, the
`ActionsViewModel` emits another event for the page to catch
(`UpdateBackground`).
## Validation Steps Performed
Actions page still works as before
---------
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
Hi, I realized I had a bug in my pull request for search selections
where when the highlight is wrapped it causes the selection to select
back to start of the current line instead of wrapping to the next line.
PR where I introduced this bug: #16227
This is a specification for a way to customize console allocations.
The new manifest type `consoleAllocationPolicy` and the new
`AllocConsoleWithOptions` API were already added to the console
client library internally.
Closes#7335
DECKPAM originally tracked in #16506.
Support was added in #16511.
But turns out people didn't expect the Terminal to actually be like,
compliant: #16654
This closes#16654 while we think over in #16672 how we want to solve
this
This fixes two issues where the `Space` key wasn't being handled
correctly:
* Keyboards with an `AltGr`+`Space` mapping were not generating the
expected character.
* Pressing a dead key followed by `Space` is supposed to generate the
accent character associated with that key, but it wasn't doing so.
## References and Relevant Issues
These were both regressions from the keyboard refactor in PR #16511.
## Detailed Description of the Pull Request / Additional comments
The problem was that we were treating `VK_SPACE` as a "functional" key,
which means it gets hardcoded VT mappings which take precedence over
whatever is in the keyboard layout. This was deemed necessary to deal
with the fact that many keyboards incorrectly map `Ctrl`+`Space` as a
`SP` character, when it's expected to be `NUL`.
I've now dropped `VK_SPACE` from the functional mapping table and allow
it be handled by the default mapping algorithm for "graphic" keys.
However, I've also introduced a special case check for `Ctrl`+`Space`
(and other modifier variants), so we can bypass any incorrect keyboard
layouts for those combinations.
## Validation Steps Performed
I couldn't test with a French-BEPO keyboard layout directly, because the
MS Keyboard Layout Creator wouldn't accept a `Space` key mapping that
wasn't whitespace. However, if I remapped the `AltGr`+`Space` combo to
`LF`, I could confirm that we are now generating that correctly.
I've also tested the dead key `Space` combination on various keyboard
layouts and confirmed that that is now working correctly, and checked
that the `Ctrl`+`Space` combinations are still working too.
Closes#16641Closes#16642
This contains all the parts of #16598 that aren't specific to session
restore, but are required for the code in #16598:
* Adds new GUID<>String functions that remove the `{}` brackets.
* Adds `SessionId` to the `ITerminalConnection` interface.
* Flush the `ApplicationState` before we terminate the process.
* Not monitoring `state.json` for changes is important as it prevents
disturbing the session state while session persistence is ongoing.
That's because when `ApplicationState` flushes to disk, the FS
monitor will be triggered and reload the `ApplicationState` again.
TIL: You could Ctrl+V files into Windows Terminal and here I am,
always opening the context menu and selecting "Copy as path"... smh
This restores the support by adding a very rudimentary HDROP handler.
The flip side of the regression is that I learned about this and so
conhost also gets this now, because why not!
Closes#16627
* Single files can be pasted in WT and conhost ✅
(cherry picked from commit ef96e225da)
Service-Card-Id: 91727725
Service-Version: 1.19
TIL: You could Ctrl+V files into Windows Terminal and here I am,
always opening the context menu and selecting "Copy as path"... smh
This restores the support by adding a very rudimentary HDROP handler.
The flip side of the regression is that I learned about this and so
conhost also gets this now, because why not!
Closes#16627
## Validation Steps Performed
* Single files can be pasted in WT and conhost ✅
#16592 passes the return value of `GetEnvironmentStringsW` directly
to the `hstring` constructor even though the former returns a
double-null terminated string and the latter expects a regular one.
This PR fixes the issue by using a basic strlen() loop to compute
the length ourselves. It's still theoretically beneficial over
the previous code, but now it's rather bitter since the code isn't
particularly short anymore and so the biggest benefit is gone.
Closes#16623
## Validation Steps Performed
* Validated the `env` string in a debugger ✅
It's 1 character shorter than the old `til::env` string.
That's fine however, since any `HSTRING` is always null-terminated
anyways and so we get an extra null-terminator for free.
* `wt powershell` works ✅
(cherry picked from commit c669afe2a0)
Service-Card-Id: 91719862
Service-Version: 1.19
Due to things outside our control, sometimes the Package phase fails
when VPack publication is enabled. Because of this, symbols won't be
published. We still want these builds to be considered "golden" and we
are still shipping them, so we *must* publish symbols.
(cherry picked from commit bcca7aac1b)
Service-Card-Id: 91719595
Service-Version: 1.19
#16592 passes the return value of `GetEnvironmentStringsW` directly
to the `hstring` constructor even though the former returns a
double-null terminated string and the latter expects a regular one.
This PR fixes the issue by using a basic strlen() loop to compute
the length ourselves. It's still theoretically beneficial over
the previous code, but now it's rather bitter since the code isn't
particularly short anymore and so the biggest benefit is gone.
Closes#16623
## Validation Steps Performed
* Validated the `env` string in a debugger ✅
It's 1 character shorter than the old `til::env` string.
That's fine however, since any `HSTRING` is always null-terminated
anyways and so we get an extra null-terminator for free.
* `wt powershell` works ✅
This includes a fix for the hang on shutdown due to the folder change
reader.
WIL now validates format strings in `LOG...` macros (yay!) and so we
needed to fix some of our `LOG` macros.
Closes#16456
Due to things outside our control, sometimes the Package phase fails
when VPack publication is enabled. Because of this, symbols won't be
published. We still want these builds to be considered "golden" and we
are still shipping them, so we *must* publish symbols.
conhost has 2 bugs related to clipboard handling:
* Missing retry on `OpenClipboard`: When copying to the clipboard
explorer.exe is very eager to open the clipboard and peek into it.
I'm not sure why it happens, but I can see `CFSDropTarget` in the
call stack. It uses COM RPC and so this takes ~20ms every time.
That breaks conhost's clipboard randomly during `ConsoleBench`.
During non-benchmarks I expect this to break during RDP.
* Missing null-terminator check during paste: `CF_UNICODETEXT` is
documented to be a null-terminated string, which conhost v2
failed to handle as it relied entirely on `GlobalSize`.
Additionally, this changeset simplifies the `HGLOBAL` code slightly
by adding `_copyToClipboard` to abstract it away.
* `ConsoleBench` (#16453) doesn't fail randomly anymore ✅
(cherry picked from commit 86c30bd)
The primary reason for this refactoring was to simplify the management
of VT input sequences that vary depending on modes, adding support for
the missing application keypad sequences, and preparing the way for
future extensions like `S8C1T`.
However, it also includes fixes for a number of keyboard related bugs,
including a variety of missing or incorrect mappings for the `Ctrl` and
`Ctrl`+`Alt` key combinations,
## References and Relevant Issues
This PR also includes a fix for #10308, which was previously closed as a
duplicate of #10551. I don't think those bugs were related, though, and
although they're both supposed to be fixed in Windows 11, this PR fixes
the issue in Windows 10.
## Detailed Description of the Pull Request / Additional comments
The way the input now works, there's a single keyboard map that takes a
virtual key code combined with `Ctrl`, `Alt`, and `Shift` modifier bits
as the lookup key, and the expected VT input sequence as the value. This
map is initially constructed at startup, and then regenerated whenever a
keyboard mode is changed.
This map takes care of the cursor keys, editing keys, function keys, and
keys like `BkSp` and `Return` which can be affected by mode changes. The
remaining "graphic" key combinations are determined manually at the time
of input.
The order of precedence looks like this:
1. If the virtual key is `0` or `VK_PACKET`, it's considered to be a
synthesized keyboard event, and the `UnicodeChar` value is used
exactly as given.
2. If it's a numeric keypad key, and `Alt` is pressed (but not `Ctrl`),
then it's assumedly part of an Alt-Numpad composition, so the key
press is ignored (the generated character will be transmitted when
the `Alt` is released).
3. If the virtual key combined with modifier bits is found in the key
map described above, then the matched escape sequence will be used
used as the output.
4. If a `UnicodeChar` value has been provided, that will be used as the
output, but possibly with additional Ctrl and Alt modifiers applied:
a. If it's an `AltGr` key, and we've got either two `Ctrl` keys
pressed or a left `Ctrl` key that is distinctly separate from a
right `Alt` key, then we will try and convert the character into
a C0 control code.
b. If an `Alt` key is pressed (or in the case of an `AltGr` value,
both `Alt` keys are pressed), then we will convert it into an
Alt-key sequence by prefixing the character with an `ESC`.
5. If we don't have a `UnicodeChar`, we'll use the `ToUnicodeEx` API to
check whether the current keyboard state reflects a dead key, and if
so, return nothing.
6. Otherwise we'll make another `ToUnicodeEx` call but with any `Ctrl`
and `Alt` modifiers removed from the state to determine the base key
value. Once we have that, we can apply the modifiers ourself.
a. If the `Ctrl` key is pressed, we'll try and convert the base value
into a C0 control code. But if we can't do that, we'll try again
with the virtual key code (if it's alphanumeric) as a fallback.
b. If the `Alt` key is pressed, we'll convert the base value (or
control code value) into an Alt-key sequence by prefixing it with
an `ESC`.
For step 4-a, we determine whether the left `Ctrl` key is distinctly
separate from the right `Alt` key by recording the time that those keys
are pressed, and checking for a time gap greater than 50ms. This is
necessary to distinguish between the user pressing `Ctrl`+`AltGr`, or
just pressing `AltGr` alone, which triggers a fake `Ctrl` key press at
the same time.
## Validation Steps Performed
I created a test script to automate key presses in the terminal window
for every relevant key, along with every Ctrl/Alt/Shift modifier, and
every relevant mode combination. I then compared the generated input
sequences with XTerm and a DEC VT240 terminal. The idea wasn't to match
either of them exactly, but to make sure the places where we differed
were intentional and reasonable.
This mostly dealt with the US keyboard layout. Comparing international
layouts wasn't really feasible because DEC, Linux, and Windows keyboard
assignments tend to be quite different. However, I've manually tested a
number of different layouts, and tried to make sure that they were all
working in a reasonable manner.
In terms of unit testing, I haven't done much more than patching the
ones that already existed to get them to pass. They're honestly not
great tests, because they aren't generating events in the form that
you'd expect for a genuine key press, and that can significantly affect
the results, but I can't think of an easy way to improve them.
## PR Checklist
- [x] Closes#16506
- [x] Closes#16508
- [x] Closes#16509
- [x] Closes#16510
- [x] Closes#3483
- [x] Closes#11194
- [x] Closes#11700
- [x] Closes#12555
- [x] Closes#13319
- [x] Closes#15367
- [x] Closes#16173
- [x] Tests added/passed
conhost has 2 bugs related to clipboard handling:
* Missing retry on `OpenClipboard`: When copying to the clipboard
explorer.exe is very eager to open the clipboard and peek into it.
I'm not sure why it happens, but I can see `CFSDropTarget` in the
call stack. It uses COM RPC and so this takes ~20ms every time.
That breaks conhost's clipboard randomly during `ConsoleBench`.
During non-benchmarks I expect this to break during RDP.
* Missing null-terminator check during paste: `CF_UNICODETEXT` is
documented to be a null-terminated string, which conhost v2
failed to handle as it relied entirely on `GlobalSize`.
Additionally, this changeset simplifies the `HGLOBAL` code slightly
by adding `_copyToClipboard` to abstract it away.
## Validation Steps Performed
* `ConsoleBench` (#16453) doesn't fail randomly anymore ✅
#15541 changed `AdaptDispatch::_FillRect` which caused it to not affect
the `ROW::_wrapForced` flag anymore. This change in behavior was not
noticeable as `TextBuffer::GetLastNonSpaceCharacter` had a bug where
rows of only whitespace text would always be treated as empty.
This would then affect `AdaptDispatch::_EraseAll` to accidentally
correctly guess the last row with text despite the `_FillRect` change.
#15701 then fixed `GetLastNonSpaceCharacter` indirectly by fixing
`ROW::MeasureRight` which now made the previous change apparent.
`_EraseAll` would now guess the last row of text incorrectly,
because it would find the rows that `_FillRect` cleared but still
had `_wrapForced` set to `true`.
This PR fixes the issue by replacing the `_FillRect` usage to clear
rows with direct calls to `ROW::Reset()`. In the future this could be
extended by also `MEM_DECOMMIT`ing the now unused underlying memory.
Closes#16603
## Validation Steps Performed
* Enter WSL and resize the window to <40 columns
* Execute
```sh
cd /bin
ls -la
printf "\e[3J"
ls -la
printf "\e[3J"
printf "\e[2J"
```
* Only one viewport-height-many lines of whitespace exist between the
current prompt line and the previous scrollback contents ✅
(cherry picked from commit 5f71cf3e94)
Service-Card-Id: 91707937
Service-Version: 1.19
`EnableScrollbar()` and especially `SetScrollInfo()` are prohibitively
expensive functions nowadays. This improves throughput of good old
`type` in cmd.exe by ~10x, by briefly releasing the console lock.
## Validation Steps Performed
* `type`ing a file in `cmd` is as fast while the window is scrolling
as it is while it isn't scrolling ✅
* Scrollbar pops in and out when scroll-forward is disabled ✅
(cherry picked from commit 71a6f26e6e)
Service-Card-Id: 91152166
Service-Version: 1.19
#15541 changed `AdaptDispatch::_FillRect` which caused it to not affect
the `ROW::_wrapForced` flag anymore. This change in behavior was not
noticeable as `TextBuffer::GetLastNonSpaceCharacter` had a bug where
rows of only whitespace text would always be treated as empty.
This would then affect `AdaptDispatch::_EraseAll` to accidentally
correctly guess the last row with text despite the `_FillRect` change.
#15701 then fixed `GetLastNonSpaceCharacter` indirectly by fixing
`ROW::MeasureRight` which now made the previous change apparent.
`_EraseAll` would now guess the last row of text incorrectly,
because it would find the rows that `_FillRect` cleared but still
had `_wrapForced` set to `true`.
This PR fixes the issue by replacing the `_FillRect` usage to clear
rows with direct calls to `ROW::Reset()`. In the future this could be
extended by also `MEM_DECOMMIT`ing the now unused underlying memory.
Closes#16603
## Validation Steps Performed
* Enter WSL and resize the window to <40 columns
* Execute
```sh
cd /bin
ls -la
printf "\e[3J"
ls -la
printf "\e[3J"
printf "\e[2J"
```
* Only one viewport-height-many lines of whitespace exist between the
current prompt line and the previous scrollback contents ✅
This changeset ensures that the message queue of frozen windows is
always being serviced. This should ensure that it won't fill up and
lead to deadlocks, freezes, or similar. I've tried _a lot_ of different
approaches before settling on this one. Introducing a custom `WM_APP`
message has the benefit of being the least intrusive to the existing
code base.
The approach that I would have favored the most would be to never
destroy the `AppHost` instance in the first place, as I imagined that
this would be more robust in general and resolve other (rare) bugs.
However, I found that this requires rewriting some substantial parts
of the code base around `AppHost` and it could be something that may
be of interest in the future.
Closes#16332
Depends on #16587 and #16575
(cherry picked from commit 5d2fa4782f)
Service-Card-Id: 91642479
Service-Version: 1.19
If you're already in the "output" state, then an app requesting an
"output" mark probably shouldn't end the current mark and start a new
one. It should just keep on keepin' on.
The decision to end the previous one was arbitrary in the first place,
so let's arbitrarily change it back.
Especially noticable if you hit <kbd>Enter</kbd> during a command,
because the auto-mark prompt work will do a CommandEnd, so long-running
commands will get broken into multiple marks 🥲
(cherry picked from commit 0144cdd7bc)
Service-Card-Id: 91707294
Service-Version: 1.19
`TextBuffer::GenHTML` and `TextBuffer::GenRTF` now read directly from
the TextBuffer.
- Since we're reading from the buffer, we can now read _all_ the
attributes saved in the buffer. Formatted copy now copies most (if not
all) font/color attributes in the requested format (RTF/HTML).
- Use `TextBuffer::CopyRequest` to pass all copy-related options into
text generation functions as one unit.
- Helper function `TextBuffer::CopyRequest::FromConfig()` generates a
copy request based on Selection mode and user configuration.
- Both formatted text generation functions now use `std::string` and
`fmt::format_to` to generate the required strings. Previously, we were
using `std::ostringstream` which is not recommended due to its potential
overhead.
- Reading attributes from `ROW`'s attribute RLE simplified the logic as
we don't have to track attribute change between the text.
- On the caller side, we do not have to rebuild the plain text string
from the vector of strings anymore. `TextBuffer::GetPlainText()` returns
the entire text as one `std::string`.
- Removed `TextBuffer::TextAndColors`.
- Removed `TextBuffer::GetText()`. `TextBuffer::GetPlainText()` took its
place.
This PR also fixes two bugs in the formatted copy:
- We were applying line breaks after each selected row, even though the
row could have been a Wrapped row. This caused the wrapped rows to break
when they shouldn't.
- We mishandled Unicode text (\uN) within the RTF copy. Every next
character that uses a surrogate pair or high codepoint was missing in
the copied text when pasted to MSWord. The command `\uc4` should have
been `\uc1`, which is used to tell how many fallback characters are used
for each Unicode codepoint (\u). We always use one `?` character as the
fallback.
Closes#16191
**References and Relevant Issues**
- #16270
**Validation Steps Performed**
- Casual copy-pasting from Terminal or OpenConsole to word editors works
as before.
- Verified HTML copy by copying the generated HTML string and running it
through an HTML viewer.
[Sample](https://codepen.io/tusharvickey/pen/wvNXbVN)
- Verified RTF copy by copy-pasting the generated RTF string into
MSWord.
- SingleLine mode works (<kbd>Shift</kbd>+ copy)
- BlockSelection mode works (<kbd>Alt</kbd> selection)
This changeset ensures that the message queue of frozen windows is
always being serviced. This should ensure that it won't fill up and
lead to deadlocks, freezes, or similar. I've tried _a lot_ of different
approaches before settling on this one. Introducing a custom `WM_APP`
message has the benefit of being the least intrusive to the existing
code base.
The approach that I would have favored the most would be to never
destroy the `AppHost` instance in the first place, as I imagined that
this would be more robust in general and resolve other (rare) bugs.
However, I found that this requires rewriting some substantial parts
of the code base around `AppHost` and it could be something that may
be of interest in the future.
Closes#16332
Depends on #16587 and #16575
17cc109 and e9de646 both made the same mistake: When cleaning up our
telemetry code they also removed the calls to `TraceLoggingRegister`
which also broke regular tracing. Windows Defender in particular uses
the "CookedRead" event to monitor for malicious shell commands.
This doesn't fix it the "right way", because destructors of statics
aren't executed when DLLs are unloaded. But I felt like that this is
fine because we have way more statics than that in conhost land,
all of which have the same kind of issue.
(cherry picked from commit a65d5f321f)
Service-Card-Id: 91337330
Service-Version: 1.19
After exiting the main loop in this function the invariant
`nFont <= NumberOfFonts` still holds true. Additionally,
preceding this removed code is this (paraphrased):
```cpp
if (nFont < NumberOfFonts) {
RtlMoveMemory(...);
}
```
It ensures that the given slot `nFont` is always unoccupied by moving
it and all following items upwards if needed. As such, the call to
`DeleteObject` is always incorrect, as the slot is always "empty",
but may contain a copy of the previous occupant due to the `memmove`.
This regressed in 154ac2b.
Closes#16297
## Validation Steps Performed
* All fonts have a unique look in the preview panel ✅
(cherry picked from commit 35240f263e)
Service-Card-Id: 91120871
Service-Version: 1.19
Closes MSFT:46744208
BODGY: If the emperor is being dtor'd, it's because we've gone past the
end of main, and released the ref in main. Then we might run into an
edge case where main releases it's ref to the emperor, but one of the
window threads might be in the process of exiting, and still holding a
strong ref to the emperor. In that case, we can actually end up with
the _window thread_ being the last reference, and calling App::Close
on that thread will crash us with a E_WRONG_THREAD.
This fixes the issue by calling `TerminateProcess` explicitly.
How validated: The ES team manually ran the test pass this was
crashing in a hundred times to make sure this actually fixed it.
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
(cherry picked from commit 0d47c862c2)
Service-Card-Id: 91642489
Service-Version: 1.19
This simplifies the function in two ways:
* Passing `nCmdShow` from `wWinMain` alleviates the need to interpret
the return value of `GetStartupInfoW`.
* `til::env::from_current_environment()` calls `GetEnvironmentStringsW`
to get the environment variables, while `to_string()` turns it back.
Calling the latter directly alleviates the need for this round-trip.
(cherry picked from commit a39ac598cd)
Service-Card-Id: 91643115
Service-Version: 1.19
Closes MSFT:46744208
BODGY: If the emperor is being dtor'd, it's because we've gone past the
end of main, and released the ref in main. Then we might run into an
edge case where main releases it's ref to the emperor, but one of the
window threads might be in the process of exiting, and still holding a
strong ref to the emperor. In that case, we can actually end up with
the _window thread_ being the last reference, and calling App::Close
on that thread will crash us with a E_WRONG_THREAD.
This fixes the issue by calling `TerminateProcess` explicitly.
How validated: The ES team manually ran the test pass this was
crashing in a hundred times to make sure this actually fixed it.
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
This simplifies the function in two ways:
* Passing `nCmdShow` from `wWinMain` alleviates the need to interpret
the return value of `GetStartupInfoW`.
* `til::env::from_current_environment()` calls `GetEnvironmentStringsW`
to get the environment variables, while `to_string()` turns it back.
Calling the latter directly alleviates the need for this round-trip.
In WindowsTerminal, there was a leak of a BSTR with every call to
ITextRangeProvider::GetText, and a failure to call VariantClear in
ITextRange::GetAttributeValue when the value stored in the variant is
VT_BSTR. These were fixed by switching to wil::unique_bstr and
wil::unique_variant.
(cherry picked from commit da99d892f4)
Service-Card-Id: 91631736
Service-Version: 1.19
## Summary of the Pull Request
This PR adds support for the `DECST8C` escape sequence, which resets the
tab stops to every 8 columns.
## Detailed Description of the Pull Request / Additional comments
This is actually a private parameter variant of the ANSI `CTC` sequence
(Cursor Tabulation Control), which accepts a selective parameter which
specifies the type of tab operation to be performed. But the DEC variant
only defines a single parameter value (5), which resets all tab stops.
It also considers an omitted parameter to be the equivalent of 5, so we
support that too.
## Validation Steps Performed
I've extended the existing tab stop tests in `ScreenBufferTests` with
some basic coverage of this sequence.
I've also manually verified that the `DECTABSR` script in #14984 now
passes the `DECST8C` portion of the test.
## PR Checklist
- [x] Closes#16533
- [x] Tests added/passed
(cherry picked from commit f5898886be)
Service-Card-Id: 91631721
Service-Version: 1.19
This reverts commit abab8705fe.
It went badly, as you might imagine.
(cherry picked from commit fe65d9ac8f)
Service-Card-Id: 91620326
Service-Version: 1.19
## Summary of the Pull Request
This PR adds support for more Device Status Report (`DSR`) queries,
specifically:
* Printer Status (`DSR ?15`)
* User Defined Keys (`DSR ?25`)
* Keyboard Status (`DSR ?26`)
* Locator Status (`DSR ?55`)
* Locator Identity (`DSR ?56`)
* Data Integrity (`DSR ?75`)
* Multiple Session Status (`DSR ?85`)
## Detailed Description of the Pull Request / Additional comments
For most of these, we just need to return a `DSR` sequence indicating
that the functionality isn't supported.
* `DSR ?13` indicates that a printer isn't connected.
* `DSR ?23` indicates the UDK extension isn't supported.
* `DSR ?53` indicates that a locator device isn't connected
* `DSR ?57;0` indicates the locator type is unknown or not connected.
* `DSR ?83` indicates that multiple sessions aren't supported.
For the keyboard, we report `DSR ?27;0;0;5`, indicating a PC keyboard
(the `5` parameter), a "ready" status (the second `0` parameter), and an
unknown language (the first `0` parameter). In the long term, there may
be some value in identifying the actual keyboard language, but for now
this should be good enough.
The data integrity report was originally used to detect communication
errors between the terminal and host, but that's not really applicable
for modern terminals, so we always just report `DSR ?70`, indicating
that there are no errors.
## Validation Steps Performed
I've added some more adapter tests and output engine tests covering the
new reports.
## PR Checklist
- [x] Closes#16518
- [x] Tests added/passed
(cherry picked from commit 6c192d15be)
Service-Card-Id: 91631713
Service-Version: 1.19
At the time of writing, closing the last tab of a window inexplicably
doesn't lead to the destruction of the remaining TermControl instance.
On top of that, on Win10 we don't destroy window threads due to bugs in
DesktopWindowXamlSource. In other words, we leak TermControl instances.
Additionally, the XAML timer class is "self-referential".
Releasing all references to an instance will not stop the timer.
Only calling Stop() explicitly will achieve that.
The result is that the message loop of a frozen window thread has so
far received 1-2 messages per second due to the blink timer not being
stopped. This may have filled the message queue and lead to bugs as
described in #16332 where keyboard input stopped working.
(cherry picked from commit 521a300c17)
Service-Card-Id: 91642474
Service-Version: 1.19
In the Settings UI's Color Scheme page (where you edit the color scheme itself), update the color chip buttons to include the RGB value in the tooltip and screen reader announcements.
Closes#15985Closes#15983
## Validation Steps Performed
Tooltip and screen reader announcement is updated on launch and when a new value is selected.
(cherry picked from commit 057183b651)
Service-Card-Id: 91642735
Service-Version: 1.19
Fix overlapping disclaimer text in Profiles' Defaults section
In #16261, when we removed ScrollViewer from the subpages in the
settings UI, the main Grid child element order was not preserved and as
a result, the disclaimer text overlapped with the main content on the
page.
To fix that we now apply (the lost) `Grid.Row` property on the parent
StackPanel of the main content.
### Validation Steps Performed
- Disclaimer text does not overlap.
### PR Checklist
- [x] Tests added/passed
In WindowsTerminal, there was a leak of a BSTR with every call to
ITextRangeProvider::GetText, and a failure to call VariantClear in
ITextRange::GetAttributeValue when the value stored in the variant is
VT_BSTR. These were fixed by switching to wil::unique_bstr and
wil::unique_variant.
## Summary of the Pull Request
This PR adds support for the `DECST8C` escape sequence, which resets the
tab stops to every 8 columns.
## Detailed Description of the Pull Request / Additional comments
This is actually a private parameter variant of the ANSI `CTC` sequence
(Cursor Tabulation Control), which accepts a selective parameter which
specifies the type of tab operation to be performed. But the DEC variant
only defines a single parameter value (5), which resets all tab stops.
It also considers an omitted parameter to be the equivalent of 5, so we
support that too.
## Validation Steps Performed
I've extended the existing tab stop tests in `ScreenBufferTests` with
some basic coverage of this sequence.
I've also manually verified that the `DECTABSR` script in #14984 now
passes the `DECST8C` portion of the test.
## PR Checklist
- [x] Closes#16533
- [x] Tests added/passed
## Summary of the Pull Request
This PR adds support for more Device Status Report (`DSR`) queries,
specifically:
* Printer Status (`DSR ?15`)
* User Defined Keys (`DSR ?25`)
* Keyboard Status (`DSR ?26`)
* Locator Status (`DSR ?55`)
* Locator Identity (`DSR ?56`)
* Data Integrity (`DSR ?75`)
* Multiple Session Status (`DSR ?85`)
## Detailed Description of the Pull Request / Additional comments
For most of these, we just need to return a `DSR` sequence indicating
that the functionality isn't supported.
* `DSR ?13` indicates that a printer isn't connected.
* `DSR ?23` indicates the UDK extension isn't supported.
* `DSR ?53` indicates that a locator device isn't connected
* `DSR ?57;0` indicates the locator type is unknown or not connected.
* `DSR ?83` indicates that multiple sessions aren't supported.
For the keyboard, we report `DSR ?27;0;0;5`, indicating a PC keyboard
(the `5` parameter), a "ready" status (the second `0` parameter), and an
unknown language (the first `0` parameter). In the long term, there may
be some value in identifying the actual keyboard language, but for now
this should be good enough.
The data integrity report was originally used to detect communication
errors between the terminal and host, but that's not really applicable
for modern terminals, so we always just report `DSR ?70`, indicating
that there are no errors.
## Validation Steps Performed
I've added some more adapter tests and output engine tests covering the
new reports.
## PR Checklist
- [x] Closes#16518
- [x] Tests added/passed
Avoid generating extra formatted copies when action's `copyFormatting`
is not present and globally set `copyFormatting` is used.
Previously, when the action's `copyFormatting` wasn't set we deferred
the decision of which formats needed to be copied to the
`TerminalPage::CopyToClipboard` handler. This meant we needed to copy
the text in all the available formats and pass it to the handler to copy
the required formats after querying the global `copyFormatting`.
To avoid making extra copies, we'll store the global `copyFormatting` in
TerminalSettings and pass it down to `TermControl`. If
`ControlCore::CopySelectionToClipboard()` doesn't receive action
specific `copyFormatting`, it will fall back to the global one _before
generating the texts_.
## Validation Steps Performed
- no `copyFormatting` set for the copy action: Copies formats according
to the global `copyFormatting`.
- `copyFormatting` is set for the copy action: Copies formats according
to the action's `copyFormatting`.
At the time of writing, closing the last tab of a window inexplicably
doesn't lead to the destruction of the remaining TermControl instance.
On top of that, on Win10 we don't destroy window threads due to bugs in
DesktopWindowXamlSource. In other words, we leak TermControl instances.
Additionally, the XAML timer class is "self-referential".
Releasing all references to an instance will not stop the timer.
Only calling Stop() explicitly will achieve that.
The result is that the message loop of a frozen window thread has so
far received 1-2 messages per second due to the blink timer not being
stopped. This may have filled the message queue and lead to bugs as
described in #16332 where keyboard input stopped working.
Up to now we've using `U+2E2E` (reverse question mark) to represent the
`SUB` control glyph. This PR changes the glyph to `U+2426` (substitute
form two), which is also rendered as a reverse question mark, but is
more semantically correct.
The original `SUB` control rendering was implemented in PR #15075.
I've manually confirmed that `printf "\x1A"` is now shown as a reverse
question mark in OpenConsole when using the Cascadia Code font. That
would not previously have worked, because `U+2E2E` is not supported by
Cascadia Code.
Closes#16558
(cherry picked from commit 92f9ff948b)
Service-Card-Id: 91559316
Service-Version: 1.19
This pull request started out very differently. I was going to move all
the EDP code from the internal `conint` project into the public, because
EDP is [fully documented]!
Well, it doesn't have any headers in the SDK.
Or import libraries.
And it's got a deprecation notice:
> [!NOTE]
> Starting in July 2022, Microsoft is deprecating Windows Information
> Protection (WIP) and the APIs that support WIP. Microsoft will
continue
> to support WIP on supported versions of Windows. New versions of
Windows
> won't include new capabilities for WIP, and it won't be supported in
> future versions of Windows.
So I'm blasting it out the airlock instead.
[fully documented]:
https://learn.microsoft.com/en-us/windows/win32/devnotes/windows-information-protection-api
(cherry picked from commit c4c06dadad)
Service-Card-Id: 91327265
Service-Version: 1.19
Up to now we've using `U+2E2E` (reverse question mark) to represent the
`SUB` control glyph. This PR changes the glyph to `U+2426` (substitute
form two), which is also rendered as a reverse question mark, but is
more semantically correct.
The original `SUB` control rendering was implemented in PR #15075.
I've manually confirmed that `printf "\x1A"` is now shown as a reverse
question mark in OpenConsole when using the Cascadia Code font. That
would not previously have worked, because `U+2E2E` is not supported by
Cascadia Code.
Closes#16558
This change adds a different label to the property sheet which will be
displayed when conhostv1 is missing. It explains why the legacy console
checkbox is not enabled.
Related work items: MSFT-46195288
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 41871b6f4c0bba64852bfbaa9255f7677246d6fe
In the Settings UI's Color Scheme page (where you edit the color scheme itself), update the color chip buttons to include the RGB value in the tooltip and screen reader announcements.
Closes#15985Closes#15983
## Validation Steps Performed
Tooltip and screen reader announcement is updated on launch and when a new value is selected.
This PR enables alternate scroll mode by default, and also fixes the
precedence so if there is any other mouse tracking mode enabled, that
will take priority.
## Validation Steps Performed
I've manually tested by viewing a file with `less`, and confirmed that
it can now scroll using the mouse wheel by default. Also tested mouse
mouse in vim and confirmed that still works.
## PR Checklist
Closes#13187
This pull request started out very differently. I was going to move all
the EDP code from the internal `conint` project into the public, because
EDP is [fully documented]!
Well, it doesn't have any headers in the SDK.
Or import libraries.
And it's got a deprecation notice:
> [!NOTE]
> Starting in July 2022, Microsoft is deprecating Windows Information
> Protection (WIP) and the APIs that support WIP. Microsoft will
continue
> to support WIP on supported versions of Windows. New versions of
Windows
> won't include new capabilities for WIP, and it won't be supported in
> future versions of Windows.
So I'm blasting it out the airlock instead.
[fully documented]:
https://learn.microsoft.com/en-us/windows/win32/devnotes/windows-information-protection-api
The tolerance value for a similar repo was changed from 0.8 to 0.75.
This is because I changed the backend service for this to use pinecone
instead of Azure AI search (see here
f72fa59e23
) and the metric changed as a result of that. They are slightly lower
than they were before, so this should offset that.
While #16444 left wavy lines in an amazing state already, there were
a few more things that could be done to make GDI look more consistent
with other well known Windows applications.
But before that, a couple unrelated, but helpful changes were made:
* `GdiEngine::UpdateFont` was heavily modified to do all calculations
in floats. All modern CPUs have fast FPUs and even the fairly slow
`lroundf` function is so fast (relatively) nowadays that in a cold
path like this, we can liberally call it to convert back to `int`s.
This makes intermediate calculation more accurate and consistent.
* `GdiEngine::PaintBufferGridLines` was exception-unsafe due to its
use of a `std::vector` with catch clause and this PR fixes that.
Additionally, the vector was swapped out with a `til::small_vector`
to reduce heap allocations. (Arena allocators!)
* RenderingTests was updated to cover styled underlines
With that in place, these improvements were done:
* Word's double-underline algorithm was ported over from `AtlasEngine`.
It uses a half underline-width (aka `thinLineWidth`) which will now
also be used for wavy lines to make them look a bit more filigrane.
* The Bézier curve for wavy/curly underlines was modified to use
control points at (0.5,0.5) and (0.5,-0.5) respectively. This results
in a maxima at y=0.1414 which is much closer to a sine curve with a
maxima at 1/(2pi) = 0.1592. Previously, the maxima was a lot higher
(roughly 4x) depending on the aspect ratio of the glyphs.
* Wavy underlines don't depend on the aspect ratio of glyphs anymore.
This previously led to several problems depending on the exact font.
The old renderer would draw exactly 3 periods of the wave into
each cell which would also ensure continuity between cells.
Unfortunately, this meant that waves could look inconsistent.
The new approach always uses the aforementioned sine-like waves.
* The wavy underline offset was clamped so that it's never outside of
bounds of a line. This avoids clipping.
* Compile RenderingTests and run it
* Using Consolas, MS Gothic and Cascadia Code while Ctrl+Scrolling
up and down works as expected without clipping ✅
(cherry picked from commit 99193c9a3f)
Service-Card-Id: 91356394
Service-Version: 1.19
Fixes Curlyline being drawn as single underline in some cases
**Detailed Description**
- Curlyline is drawn at all font sizes.
- We might render a curlyline that is clipped in cases where we don't
have enough space to draw a full curlyline. This is to give users a
consistent view of Curlylines. Previously in those cases, it was drawn
as a single underline.
- Removed minimum threshold `minCurlyLinePeakHeight` for Curlyline
drawing.
- GDIRender changes:
- Underline offset now points to the (vertical) mid position of the
underline. Removes redundant `underlineMidY` calculation inside the draw
call.
Closes#16288
(cherry picked from commit f5b45c25c9)
Service-Card-Id: 91349182
Service-Version: 1.19
Add support for underline style and color in the renderer
> [!IMPORTANT]
> The PR adds underline style and color feature to AtlasEngine (WT) and
GDIRenderer (Conhost) only.
After the underline style and color feature addition to Conpty, this PR
takes it further and add support for rendering them to the screen!
Out of five underline styles, we already supported rendering for 3 of
those types (Singly, Doubly, Dotted) in some form in our (Atlas)
renderer. The PR adds the remaining types, namely, Dashed and Curly
underlines support to the renderer.
- All renderer engines now receive both gridline and underline color,
and the latter is used for drawing the underlines. **When no underline
color is set, we use the foreground color.**
- Curly underline is rendered using `sin()` within the pixel shader.
- To draw underlines for DECDWL and DECDHL, we send the line rendition
scale within `QuadInstance`'s texcoord attribute.
- In GDI renderer, dashed and dotted underline is drawn using `HPEN`
with a desired style. Curly line is a cubic Bezier that draws one wave
per cell.
## PR Checklist
- ✅ Set the underline color to underlines only, without affecting the
gridline color.
- ❌ Port to DX renderer. (Not planned as DX renderer soon to be replaced
by **AtlasEngine**)
- ✅ Port underline coloring and style to GDI renderer (Conhost).
- ✅ Wide/Tall `CurlyUnderline` variant for `DECDWL`/`DECDHL`.
Closes#7228
(cherry picked from commit e268c1c952)
Service-Card-Id: 91349180
Service-Version: 1.19
Even with the previous fixes we still randomly encounter win32-
input-mode sequences that are broken up in exactly such a way that
e.g. lone escape keys are encounters. Those for instance clear the
current prompt. The remaining parts of the sequence are then visible.
This changeset fixes the issue by skipping the entire force-to-ground
code whenever we saw at least 1 win32-input-mode sequence.
Related to #16343
## Validation Steps Performed
* Host a ConPTY inside ConPTY (= double the trouble) with cmd.exe
* Paste random amounts of text
* In the old code spurious `[..._` strings are seen
* In the new code they're consistently gone ✅
(cherry picked from commit bc18348967)
Service-Card-Id: 91337332
Service-Version: 1.19
Added wrapping to highlighted selection when selecting a word, added
tests for it
## Detailed Description of the Pull Request / Additional comments
- Modified GetWordStart and GetWordEnd and their helpers to no longer be
bounded by the right and left viewport ranges
- Kept same functionality (does not wrap) when selecting wrapped
whitespace
- Added tests to TextBufferTests.cpp to include cases of wrapping text
## Validation Steps Performed
- Ran locally and verified selection works properly
- Tests passed locally
Closes#4009
Even with the previous fixes we still randomly encounter win32-
input-mode sequences that are broken up in exactly such a way that
e.g. lone escape keys are encounters. Those for instance clear the
current prompt. The remaining parts of the sequence are then visible.
This changeset fixes the issue by skipping the entire force-to-ground
code whenever we saw at least 1 win32-input-mode sequence.
Related to #16343
## Validation Steps Performed
* Host a ConPTY inside ConPTY (= double the trouble) with cmd.exe
* Paste random amounts of text
* In the old code spurious `[..._` strings are seen
* In the new code they're consistently gone ✅
This makes 3 improvements:
* 16x larger input buffer size improves behavior when pasting
clipboard contents while the win32-input-mode is enabled,
as each input character is roughly 15-20x longer after encoding.
* Translate UTF8 to UTF16 outside of the console lock.
* Preserve the UTF16 buffer between reads for less mallocs.
(cherry picked from commit 171a21ad48)
Service-Card-Id: 91347494
Service-Version: 1.19
This is just a minor, unimportant cleanup to remove code duplication
in `_flushBuffer`, which called `SetCursorPosition` twice each time
the cursor position changed.
17cc109 and e9de646 both made the same mistake: When cleaning up our
telemetry code they also removed the calls to `TraceLoggingRegister`
which also broke regular tracing. Windows Defender in particular uses
the "CookedRead" event to monitor for malicious shell commands.
This doesn't fix it the "right way", because destructors of statics
aren't executed when DLLs are unloaded. But I felt like that this is
fine because we have way more statics than that in conhost land,
all of which have the same kind of issue.
While #16444 left wavy lines in an amazing state already, there were
a few more things that could be done to make GDI look more consistent
with other well known Windows applications.
But before that, a couple unrelated, but helpful changes were made:
* `GdiEngine::UpdateFont` was heavily modified to do all calculations
in floats. All modern CPUs have fast FPUs and even the fairly slow
`lroundf` function is so fast (relatively) nowadays that in a cold
path like this, we can liberally call it to convert back to `int`s.
This makes intermediate calculation more accurate and consistent.
* `GdiEngine::PaintBufferGridLines` was exception-unsafe due to its
use of a `std::vector` with catch clause and this PR fixes that.
Additionally, the vector was swapped out with a `til::small_vector`
to reduce heap allocations. (Arena allocators!)
* RenderingTests was updated to cover styled underlines
With that in place, these improvements were done:
* Word's double-underline algorithm was ported over from `AtlasEngine`.
It uses a half underline-width (aka `thinLineWidth`) which will now
also be used for wavy lines to make them look a bit more filigrane.
* The Bézier curve for wavy/curly underlines was modified to use
control points at (0.5,0.5) and (0.5,-0.5) respectively. This results
in a maxima at y=0.1414 which is much closer to a sine curve with a
maxima at 1/(2pi) = 0.1592. Previously, the maxima was a lot higher
(roughly 4x) depending on the aspect ratio of the glyphs.
* Wavy underlines don't depend on the aspect ratio of glyphs anymore.
This previously led to several problems depending on the exact font.
The old renderer would draw exactly 3 periods of the wave into
each cell which would also ensure continuity between cells.
Unfortunately, this meant that waves could look inconsistent.
The new approach always uses the aforementioned sine-like waves.
* The wavy underline offset was clamped so that it's never outside of
bounds of a line. This avoids clipping.
## Validation Steps Performed
* Compile RenderingTests and run it
* Using Consolas, MS Gothic and Cascadia Code while Ctrl+Scrolling
up and down works as expected without clipping ✅
**FIRST TIME CONTRIBUTOR**
Follows the existing selection code as much as possible.
Updated logic that finds selection rectangles to also identify search
rectangles.
Right now, this feature only works in the new Atlas engine -- it uses
the background and foreground color bitmaps to quickly and efficiently
set the colors of a whole region of text.
Closes#7561
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
This makes 3 improvements:
* 16x larger input buffer size improves behavior when pasting
clipboard contents while the win32-input-mode is enabled,
as each input character is roughly 15-20x longer after encoding.
* Translate UTF8 to UTF16 outside of the console lock.
* Preserve the UTF16 buffer between reads for less mallocs.
The `GenRTF(...)` was using `\highlight` control word for sending
background text color in the RTF format during a copy command. This
doesn't work correctly, since many applications (E.g. MSWord) don't
support full RGB with `\highlight`, and instead uses an approximation of
what is received. For example, `rgb(197, 15, 31)` becomes `rgb(255, 0,
255)`. Also, the standard way of using background colors is `\cbN`
control word, which isn't supported as per the [RTF Spec 1.9.1]
in Word.
But it briefly mentioned a workaround at Pg. 23, which seems to work on
all the RTF editors I tested.
The PR makes the changes to use `\chshdng0\chcbpatN` for the background
coloring.
Also did some refactoring to make the implementation concise.
## Validation Steps Performed
Verified that the background is correctly copied on below editors:
- MSWord
- WordPad
- LibreOffice
- Outlook
[RTF Spec 1.9.1]: https://msopenspecs.azureedge.net/files/Archive_References/[MSFT-RTF].pdf
(cherry picked from commit 310814bb30)
Service-Card-Id: 91349195
Service-Version: 1.19
Fixes Curlyline being drawn as single underline in some cases
**Detailed Description**
- Curlyline is drawn at all font sizes.
- We might render a curlyline that is clipped in cases where we don't
have enough space to draw a full curlyline. This is to give users a
consistent view of Curlylines. Previously in those cases, it was drawn
as a single underline.
- Removed minimum threshold `minCurlyLinePeakHeight` for Curlyline
drawing.
- GDIRender changes:
- Underline offset now points to the (vertical) mid position of the
underline. Removes redundant `underlineMidY` calculation inside the draw
call.
Closes#16288
This prevents an issue in conhost where older versions of Windows
Terminal (including the ones currently inbox in Windows, as well as
stable and preview) will *still* cause WSL interop to hang on startup.
Since VT input is erroneously re-encoded as Win32 input events on those
versions, we need to make sure we request the cursor position *before*
enabling Win32 input mode. That way, the CPR we get back is properly
encoded.
(cherry picked from commit 17867af534)
Service-Card-Id: 91301135
Service-Version: 1.19
This prevents an issue in conhost where older versions of Windows
Terminal (including the ones currently inbox in Windows, as well as
stable and preview) will *still* cause WSL interop to hang on startup.
Since VT input is erroneously re-encoded as Win32 input events on those
versions, we need to make sure we request the cursor position *before*
enabling Win32 input mode. That way, the CPR we get back is properly
encoded.
Changes any references of `> **Note**\` with `> [!NOTE]` to match the
new syntax for markdown files in GitHub.
Fixes the 14 November 2023 update to the alerts syntax in markdown
files:
> ## Update - 14 November 2023
> * The initial syntax using e.g. **Note** isn't supported any longer.
>
> https://github.com/orgs/community/discussions/16925
This changeset fixes an issue caused by #15991 where "chunked" escape
sequences would get corrupted. The fix is to simply not flush eagerly
anymore. I tried my best to keep the input lag reduction from #15991,
but unfortunately this isn't possible for console APIs.
Closes#16079
## Validation Steps Performed
* `type ascii.com` produces soft font ASCII characters ✅
(cherry picked from commit bdf2f6f274)
Service-Card-Id: 91283205
Service-Version: 1.19
This is my proposal to avoid aborting ConPTY input parsing because a
read accidentally got split up into more than one chunk. This happens a
lot with WSL for me, as I often get (for instance) a
`\x1b[67;46;99;0;32;` input followed immediately by a `1_` input. The
current logic would cause both of these to be flushed out to the client
application.
This PR fixes the issue by only flushing either a standalone escape
character or a escape+character combination. It basically limits the
previous code to just `VTStates::Ground` and `VTStates::Escape`.
I'm not using the `_state` member, because `VTStates::OscParam` makes no
distinction between `\x1b]` and `\x1b]1234` and I only want to flush the
former. I felt like checking the contents of `run` directly is easier to
understand.
Related to #16343
## Validation Steps Performed
* win32-input-mode sequences are now properly buffered ✅
* Standalone alt-key combinations are still being flushed ✅
(cherry picked from commit 5f5ef10571)
Service-Card-Id: 91270261
Service-Version: 1.19
Upgrades check-spelling to [v0.0.22](https://github.com/check-spelling/check-spelling/releases/tag/v0.0.22)
* refreshes workflow
* enables dependabot PRs to trigger CI (so that in the future you'll be
able to see breaking changes to the dictionary paths)
* refreshes metadata
* built-in handling of `\n`/`\r`/`\t` is removed -- This means that the
`patterns/0_*.txt` files can be removed.
* this specific PR includes some shim content, in
`allow/check-spelling-0.0.21.txt` -- once it this PR merges, it can be
removed on a branch and the next CI will clean out items from
`expect.txt` relating to the `\r` stuff and suggest replacement content.
* talking to the bot is enabled for forks (but not the master
repository)
* SARIF reporting is enabled for PRs w/in a single repository (not
across forks)
* In job reports, there's a summary table (space permitting) linking to
instances (this is a poor man's SARIF report)
* When a pattern splits a thing that results in check-spelling finding
an unrecognized token, that's reported with a distinct category
* When there are items in expect that not longer match anything but more
specific items do (e.g. `microsoft` vs. `Microsoft`), there's now a
specific category with help/advice
* Fancier excludes suggestions (excluding directories, file types, ...)
* Refreshed dictionaries
* The comment now links to the job summary (which includes SARIF link if
available, the details view, and a generated commit that people can use
if they're ok w/ the expect changes and don't want to run perl)
Validation
----------
1. the branch was developed in
https://github.com/check-spelling-sandbox/terminal/actions?query=branch%3Acheck-spelling-0.0.22
2. ensuring compatibility with 0.0.21 was done in
https://github.com/check-spelling-sandbox/terminal/pull/3
3. this version has been in development for a year and has quite a few
improvements, we've been actively dogfooding it throughout this period 😄
Additional Fixes
----------------
spelling: the
spelling: shouldn't
spelling: no
spelling: macos
spelling: github
spelling: fine-grained
spelling: coarse-grained
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
## Summary of the Pull Request
Added some Punctuation Marks as Required.
## References and Relevant Issues
None.
## Detailed Description of the Pull Request / Additional comments
There were some missing Punctuation Marks(Ex: Colon(:) and Full
Stop(.)), so I have added them.
## Validation Steps Performed
This is my proposal to avoid aborting ConPTY input parsing because a
read accidentally got split up into more than one chunk. This happens a
lot with WSL for me, as I often get (for instance) a
`\x1b[67;46;99;0;32;` input followed immediately by a `1_` input. The
current logic would cause both of these to be flushed out to the client
application.
This PR fixes the issue by only flushing either a standalone escape
character or a escape+character combination. It basically limits the
previous code to just `VTStates::Ground` and `VTStates::Escape`.
I'm not using the `_state` member, because `VTStates::OscParam` makes no
distinction between `\x1b]` and `\x1b]1234` and I only want to flush the
former. I felt like checking the contents of `run` directly is easier to
understand.
Related to #16343
## Validation Steps Performed
* win32-input-mode sequences are now properly buffered ✅
* Standalone alt-key combinations are still being flushed ✅
`EnableScrollbar()` and especially `SetScrollInfo()` are prohibitively
expensive functions nowadays. This improves throughput of good old
`type` in cmd.exe by ~10x, by briefly releasing the console lock.
## Validation Steps Performed
* `type`ing a file in `cmd` is as fast while the window is scrolling
as it is while it isn't scrolling ✅
* Scrollbar pops in and out when scroll-forward is disabled ✅
When ConPTY exits it should attempt to restore the state as it was
before it started. This is particularly important for the win32
input mode sequences, as Linux shells don't know what to do with it.
Related to #16343
## Validation Steps Performed
* Replace conhost with this
* Launch a Win32 application inside WSL
* Exit that application
* Shell prompt doesn't get filled with win32 input mode sequences ✅
(cherry picked from commit 70e51ae28d)
Service-Card-Id: 91246943
Service-Version: 1.19
When ConPTY exits it should attempt to restore the state as it was
before it started. This is particularly important for the win32
input mode sequences, as Linux shells don't know what to do with it.
Related to #16343
## Validation Steps Performed
* Replace conhost with this
* Launch a Win32 application inside WSL
* Exit that application
* Shell prompt doesn't get filled with win32 input mode sequences ✅
The final parameter, `updateBottom`, controls not just whether the
`_virtualBottom` is updated, but also whether the position is clamped
to be within the existing `_virtualBottom`. Setting this to `false`
thus broke scroll-forward as the `_virtualBottom` was now a constant.
## Validation Steps Performed
* Disable scroll-foward
* Press and hold Ctrl+C
* It scrolls past the viewport bottom ✅
(cherry picked from commit ab7a2f10c5)
Service-Card-Id: 91258882
Service-Version: 1.19
This changeset avoids re-encoding output from `AdaptDispatch`
via the win32-input-mode mechanism when VT input is enabled.
That is, an `AdaptDispatch` output like `\x1b[C` would otherwise
result in dozens of characters of input.
Related to #16343
## Validation Steps Performed
* Replace conhost with this
* Launch a Win32 application inside WSL
* ASCII keyboard inputs are represented as single `INPUT_RECORD`s ✅
(cherry picked from commit 0da37a134a)
Service-Card-Id: 91246942
Service-Version: 1.19
tl;dr: A coroutine lambda does not hold onto captured variables.
This causes an AV crash when closing tabs. I randomly noticed this
in a Debug build as the memory contents got replaced with 0xCD.
In a Release build this bug is probably fairly subtle and not common.
(cherry picked from commit 91fd7d0101)
Service-Card-Id: 91258717
Service-Version: 1.19
This enables AtlasEngine by default in the 1.19 release branch.
A future change will remove the alternative DxEngine entirely.
(cherry picked from commit 204ebf3b19)
Service-Card-Id: 91158876
Service-Version: 1.19
The final parameter, `updateBottom`, controls not just whether the
`_virtualBottom` is updated, but also whether the position is clamped
to be within the existing `_virtualBottom`. Setting this to `false`
thus broke scroll-forward as the `_virtualBottom` was now a constant.
## Validation Steps Performed
* Disable scroll-foward
* Press and hold Ctrl+C
* It scrolls past the viewport bottom ✅
This changeset avoids re-encoding output from `AdaptDispatch`
via the win32-input-mode mechanism when VT input is enabled.
That is, an `AdaptDispatch` output like `\x1b[C` would otherwise
result in dozens of characters of input.
Related to #16343
## Validation Steps Performed
* Replace conhost with this
* Launch a Win32 application inside WSL
* ASCII keyboard inputs are represented as single `INPUT_RECORD`s ✅
tl;dr: A coroutine lambda does not hold onto captured variables.
This causes an AV crash when closing tabs. I randomly noticed this
in a Debug build as the memory contents got replaced with 0xCD.
In a Release build this bug is probably fairly subtle and not common.
During `!measureOnly` the old code would increment `distance` twice.
Now it doesn't. :)
Closes#16356
## Validation Steps Performed
See updated test instructions in `doc/COOKED_READ_DATA.md`
(cherry picked from commit 654b755161)
Service-Card-Id: 91232745
Service-Version: 1.19
Since all VT parameters are treated to be at least 1 (and 1 if they're
absent or 0), `modifierParam > 0` was always true. This meant that
`ENHANCED_KEY` was always being set. It's unclear why `ENHANCED_KEY`
was used there, but it's likely not needed in general.
Closes#16266
## Validation Steps Performed
* Can't test this unless we fix the win32 input mode issue #16343❌
(cherry picked from commit be9fc200c7)
Service-Card-Id: 91159301
Service-Version: 1.19
## Summary of the Pull Request
Cloud shell connection calls out to Azure to do a device code login.
When the polling interval is exceeded or the tab is closed, the method
doing the connection polling returns `nullptr`, and `AzureConnection`
immediately tries to `GetNamedString` from it, causing a crash. This
doesn't repro on Terminal Stable or Preview, suggesting it's pretty
recent related to the update of this azureconnection.
This is just a proposed fix, not sure if you want to do more extensive
changes to the affected class or not, so marking this as a draft.
## References and Relevant Issues
* N/A - encountered this while using the terminal myself
## PR Checklist/Validation
Tested out a local dev build:
- [x] Terminal doesn't crash when cloudshell polling interval exceeded
- [x] Terminal doesn't crash when cloudshell tab closed while polling
for Azure login
(cherry picked from commit 0c4751ba30)
Service-Card-Id: 91232784
Service-Version: 1.19
81b7e54 caused a regression in `SetConsoleWindowInfo` and any other
function that used the `WriteToScreen` helper. This is because it
assumes that it can place the viewport anywhere randomly and it was
written at a time where `TriggerScroll` didn't exist yet (there was
no need for that (also not today, but that's being worked on)).
Caching the viewport meant that `WriteToScreen`'s call to
`TriggerRedraw` would pick up the viewport from the last rendered
frame, which would cause the intersection of both to be potentially
empty and nothing to be drawn on the screen.
This commit reverts 81b7e54 as I found that it has no or negligible
impact on performance at this point, likely due to the overall
vastly better performance of conhost nowadays.
Closes#15932
## Validation Steps Performed
* Scroll the viewport by entire pages worth of content using
`SetConsoleWindowInfo` - see #15932
* The screen and scrollbars update immediately ✅
(cherry picked from commit 7a1b6f9d2a)
Service-Card-Id: 91152167
Service-Version: 1.19
Converts null byte to specific input event, so that it's properly
delivered to the program running in the terminal.
Closes#15939
(cherry picked from commit 8747a39a07)
Service-Card-Id: 91201444
Service-Version: 1.19
This fixes an issue where character-wise reading of an input like "abc"
would return "a" to the caller, store "b" as a partial translation
(= wrong) and return "c" for the caller to store it for the next call.
Closes#16223Closes#16299
## Validation Steps Performed
* `ReadFile` with a buffer size of 1 returns inputs character by
character without dropping any inputs ✅
---------
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
(cherry picked from commit 63b3820a18)
Service-Card-Id: 91122022
Service-Version: 1.19
Under normal circumstances this bug should be rare as far as I can
observe it on my system. However, it does occur randomly.
In short, WSL doesn't pass us anonymous pipes, but rather WSA sockets
and those signal their graceful shutdown first before being closed
later by returning a `lpNumberOfBytesRead` of 0 in the meantime.
Additionally, `VtIo` synchronously pumps the input pipe to get the
initial cursor position, but fails to check `_exitRequested`.
And so even with the pipe handling fixed, `VtIo` will also deadlock,
because it will never realize that `VtInputThread` is done reading.
## Validation Steps Performed
* Build commit 376737e with this change and replace conhost with it
Coincidentally it contains a bug (of as of yet unknown origin)
due to which the initial cursor position loop in `VtIo` never
completes. Thanks to this, we can easily provoke this issue.
* Launch WSL in conhost and run an .exe inside it
* Close the conhost window
* Task manager shows that all conhost instances exit immediately
(cherry picked from commit adb04729bc)
Service-Card-Id: 91152102
Service-Version: 1.19
The nearby font loading has to be outside of the try/catch of the
`_FindFontFace` call, because it'll throw for broken font files.
But in my previous PR I had overlooked that the font variant loop
modifies the only copy of the face name that we got and was in the
same try/catch. That's bad, because once we get to the nearby search
code, the face name will be invalid. This commit fixes the issue by
wrapping each individual `_FindFontFace` call in a try/catch block.
Closes#16322
## Validation Steps Performed
* Remove every single copy of Windows Terminal from your system
* Manually clean up Cascadia .ttf files because they aren't gone
* Destroy your registry by manually removing appx references (fun!)
* Put the 4 Cascadia .ttf files into the Dev app AppX directory
* Launch
* No warning ✅
(cherry picked from commit b780b44528)
Service-Card-Id: 91114951
Service-Version: 1.19
I find it somewhat silly that (1) this isn't documented anywhere and (2)
installing the "desktop experience" packages for Server doesn't
automatically add support for the `Windows.Desktop` platform...
Oh well.
I'm going to roll this one out via Preview first, because if the store
blows up on it I would rather it not be during Stable roll-out.
(cherry picked from commit 86fb9b4478)
Service-Card-Id: 91098597
Service-Version: 1.19
During `!measureOnly` the old code would increment `distance` twice.
Now it doesn't. :)
Closes#16356
## Validation Steps Performed
See updated test instructions in `doc/COOKED_READ_DATA.md`
Since all VT parameters are treated to be at least 1 (and 1 if they're
absent or 0), `modifierParam > 0` was always true. This meant that
`ENHANCED_KEY` was always being set. It's unclear why `ENHANCED_KEY`
was used there, but it's likely not needed in general.
Closes#16266
## Validation Steps Performed
* Can't test this unless we fix the win32 input mode issue #16343❌
I randomly came across this class, that I didn't even remember we had.
We don't use this class at the moment and won't need it any time soon.
Its current implementation is also fairly questionable. While
`til::u16state` isn't "perfect", it's vastly better than this.
## Summary of the Pull Request
Cloud shell connection calls out to Azure to do a device code login.
When the polling interval is exceeded or the tab is closed, the method
doing the connection polling returns `nullptr`, and `AzureConnection`
immediately tries to `GetNamedString` from it, causing a crash. This
doesn't repro on Terminal Stable or Preview, suggesting it's pretty
recent related to the update of this azureconnection.
This is just a proposed fix, not sure if you want to do more extensive
changes to the affected class or not, so marking this as a draft.
## References and Relevant Issues
* N/A - encountered this while using the terminal myself
## PR Checklist/Validation
Tested out a local dev build:
- [x] Terminal doesn't crash when cloudshell polling interval exceeded
- [x] Terminal doesn't crash when cloudshell tab closed while polling
for Azure login
81b7e54 caused a regression in `SetConsoleWindowInfo` and any other
function that used the `WriteToScreen` helper. This is because it
assumes that it can place the viewport anywhere randomly and it was
written at a time where `TriggerScroll` didn't exist yet (there was
no need for that (also not today, but that's being worked on)).
Caching the viewport meant that `WriteToScreen`'s call to
`TriggerRedraw` would pick up the viewport from the last rendered
frame, which would cause the intersection of both to be potentially
empty and nothing to be drawn on the screen.
This commit reverts 81b7e54 as I found that it has no or negligible
impact on performance at this point, likely due to the overall
vastly better performance of conhost nowadays.
Closes#15932
## Validation Steps Performed
* Scroll the viewport by entire pages worth of content using
`SetConsoleWindowInfo` - see #15932
* The screen and scrollbars update immediately ✅
After exiting the main loop in this function the invariant
`nFont <= NumberOfFonts` still holds true. Additionally,
preceding this removed code is this (paraphrased):
```cpp
if (nFont < NumberOfFonts) {
RtlMoveMemory(...);
}
```
It ensures that the given slot `nFont` is always unoccupied by moving
it and all following items upwards if needed. As such, the call to
`DeleteObject` is always incorrect, as the slot is always "empty",
but may contain a copy of the previous occupant due to the `memmove`.
This regressed in 154ac2b.
Closes#16297
## Validation Steps Performed
* All fonts have a unique look in the preview panel ✅
This PR fixes Issue #11875 by introducing a ScrollViewer and some logic
for the scrollbar.
The ScrollViewer prevents the scrollbar from scrolling to the top
whenever "Save" is clicked in the Settings. In addition, the scrollbar
is scrolled to the top of the page whenever navigating to another page
within Settings. The scrollbar will not reset if attempting to navigate
to the same page that is already navigated to.
## Validation Steps Performed
Manual testing of the Settings by building the Terminal app.
Closes#11875
This fixes an issue where character-wise reading of an input like "abc"
would return "a" to the caller, store "b" as a partial translation
(= wrong) and return "c" for the caller to store it for the next call.
Closes#16223Closes#16299
## Validation Steps Performed
* `ReadFile` with a buffer size of 1 returns inputs character by
character without dropping any inputs ✅
---------
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
Adds "Campbell Absolute" which has absolute black/white instead of
slightly greyish variants as discussed per #35. Also updates line
endings to adhere to the default Windows line endings (i.e. CRLF)
Closes#35
This changeset fixes an issue caused by #15991 where "chunked" escape
sequences would get corrupted. The fix is to simply not flush eagerly
anymore. I tried my best to keep the input lag reduction from #15991,
but unfortunately this isn't possible for console APIs.
Closes#16079
## Validation Steps Performed
* `type ascii.com` produces soft font ASCII characters ✅
Under normal circumstances this bug should be rare as far as I can
observe it on my system. However, it does occur randomly.
In short, WSL doesn't pass us anonymous pipes, but rather WSA sockets
and those signal their graceful shutdown first before being closed
later by returning a `lpNumberOfBytesRead` of 0 in the meantime.
Additionally, `VtIo` synchronously pumps the input pipe to get the
initial cursor position, but fails to check `_exitRequested`.
And so even with the pipe handling fixed, `VtIo` will also deadlock,
because it will never realize that `VtInputThread` is done reading.
## Validation Steps Performed
* Build commit 376737e with this change and replace conhost with it
Coincidentally it contains a bug (of as of yet unknown origin)
due to which the initial cursor position loop in `VtIo` never
completes. Thanks to this, we can easily provoke this issue.
* Launch WSL in conhost and run an .exe inside it
* Close the conhost window
* Task manager shows that all conhost instances exit immediately
Just like in https://github.com/microsoft/WSL/pull/10745
We're working with the WSL team to figure out if we can use a LLM to
help us triage. This _should_ just comment on issues, if it finds
something similar on the backlog.
Our CI seems to have had an update recently to around VS 17.7.
That version contains a faulty implementation for C26478 and C26494.
The issue has been fixed in VS 17.8 and later.
The nearby font loading has to be outside of the try/catch of the
`_FindFontFace` call, because it'll throw for broken font files.
But in my previous PR I had overlooked that the font variant loop
modifies the only copy of the face name that we got and was in the
same try/catch. That's bad, because once we get to the nearby search
code, the face name will be invalid. This commit fixes the issue by
wrapping each individual `_FindFontFace` call in a try/catch block.
Closes#16322
## Validation Steps Performed
* Remove every single copy of Windows Terminal from your system
* Manually clean up Cascadia .ttf files because they aren't gone
* Destroy your registry by manually removing appx references (fun!)
* Put the 4 Cascadia .ttf files into the Dev app AppX directory
* Launch
* No warning ✅
I find it somewhat silly that (1) this isn't documented anywhere and (2)
installing the "desktop experience" packages for Server doesn't
automatically add support for the `Windows.Desktop` platform...
Oh well.
I'm going to roll this one out via Preview first, because if the store
blows up on it I would rather it not be during Stable roll-out.
I've also removed all of the supporting code.
Related work items: MSFT-47555635
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 f8ad0110fd81d1b848224158c8f95724f34b1db2
Notes in #16217 have the investigation.
TL;DR: we'd always buffer text. Even if we're disabled (unfocused). When
we're
disabled, we'd _never_ clear the buffered text. Oops.
Closes#16217
(cherry picked from commit d14524cd4c)
Service-Card-Id: 91033138
Service-Version: 1.19
The Azure cloud shell team made some API changes that required us to
format our requests a little differently. This PR makes those changes
(more info in the comments in the code)
Closes#16098
(cherry picked from commit 5a9f3529d7)
Service-Card-Id: 90985893
Service-Version: 1.19
It makes the output less cluttered and more correct (for example:
ServicingPipeline no longer tries to service two copies of each commit
if there's a merge in the history...)
(cherry picked from commit 18b0ecbb2a)
Service-Card-Id: 91042450
Service-Version: 1.19
This fixes a number of bugs introduced in 4370da9, all of which are of
the same kind: Holding the terminal lock across `WriteFile` calls into
the ConPTY pipe. This is problematic, because the pipe has a tiny buffer
size of just 4KiB and ConPTY may respond on its output pipe, before the
entire buffer given to `WriteFile` has been emptied. When the ConPTY
output thread then tries to acquire the terminal lock to begin parsing
the VT output, we get ourselves a proper deadlock (cross process too!).
The solution is to tease `Terminal` further apart into code that is
thread-safe and code that isn't. Functions like `SendKeyEvent` so far
have mixed them into one, because when they get called by `ControlCore`
they both, processed the data (not thread-safe as it accesses VT state)
and also sent that data back into `ControlCore` through a callback
which then indirectly called into the `ConptyConnection` which calls
`WriteFile`. Instead, we now return the data that needs to be sent from
these functions, and `ControlCore` is free to release the lock and
then call into the connection, which may then block indefinitely.
## Validation Steps Performed
* Start nvim in WSL
* Press `i` to enter the regular Insert mode
* Paste 1MB of text
* Doesn't deadlock ✅
(cherry picked from commit 71a1a97a9a)
Service-Card-Id: 91043521
Service-Version: 1.19
Adds
```xml
<uap17:UpdateWhileInUse>defer</uap17:UpdateWhileInUse>
```
to our `Package.Properties` for all our packages.
This was added in the September 2023 OS release of Windows 11.
Apparently, this just works now? I did update VS,
but I don't _think_ that updated the SDK.
I have no idea how it updated the manifest definitions.
Closes#3915Closes#6726
(cherry picked from commit 077d63e6a3)
Service-Card-Id: 91033136
Service-Version: 1.19
Notes in #16217 have the investigation.
TL;DR: we'd always buffer text. Even if we're disabled (unfocused). When
we're
disabled, we'd _never_ clear the buffered text. Oops.
Closes#16217
Add support for underline style and color in the renderer
> [!IMPORTANT]
> The PR adds underline style and color feature to AtlasEngine (WT) and
GDIRenderer (Conhost) only.
After the underline style and color feature addition to Conpty, this PR
takes it further and add support for rendering them to the screen!
Out of five underline styles, we already supported rendering for 3 of
those types (Singly, Doubly, Dotted) in some form in our (Atlas)
renderer. The PR adds the remaining types, namely, Dashed and Curly
underlines support to the renderer.
- All renderer engines now receive both gridline and underline color,
and the latter is used for drawing the underlines. **When no underline
color is set, we use the foreground color.**
- Curly underline is rendered using `sin()` within the pixel shader.
- To draw underlines for DECDWL and DECDHL, we send the line rendition
scale within `QuadInstance`'s texcoord attribute.
- In GDI renderer, dashed and dotted underline is drawn using `HPEN`
with a desired style. Curly line is a cubic Bezier that draws one wave
per cell.
## PR Checklist
- ✅ Set the underline color to underlines only, without affecting the
gridline color.
- ❌ Port to DX renderer. (Not planned as DX renderer soon to be replaced
by **AtlasEngine**)
- ✅ Port underline coloring and style to GDI renderer (Conhost).
- ✅ Wide/Tall `CurlyUnderline` variant for `DECDWL`/`DECDHL`.
Closes#7228
This changes the appearance of the disclaimer text that is used on some
of the settings pages. The italic text style is replaced with a neutral
style that fits better with the rest of the UI.
## Detailed Description of the Pull Request / Additional comments
I also tried out these alternative styles but overall preferred the
default TextBlock style (BodyTextBlockStyle).
Closes#16264.
The Azure cloud shell team made some API changes that required us to
format our requests a little differently. This PR makes those changes
(more info in the comments in the code)
Closes#16098
It makes the output less cluttered and more correct (for example:
ServicingPipeline no longer tries to service two copies of each commit
if there's a merge in the history...)
This fixes a number of bugs introduced in 4370da9, all of which are of
the same kind: Holding the terminal lock across `WriteFile` calls into
the ConPTY pipe. This is problematic, because the pipe has a tiny buffer
size of just 4KiB and ConPTY may respond on its output pipe, before the
entire buffer given to `WriteFile` has been emptied. When the ConPTY
output thread then tries to acquire the terminal lock to begin parsing
the VT output, we get ourselves a proper deadlock (cross process too!).
The solution is to tease `Terminal` further apart into code that is
thread-safe and code that isn't. Functions like `SendKeyEvent` so far
have mixed them into one, because when they get called by `ControlCore`
they both, processed the data (not thread-safe as it accesses VT state)
and also sent that data back into `ControlCore` through a callback
which then indirectly called into the `ConptyConnection` which calls
`WriteFile`. Instead, we now return the data that needs to be sent from
these functions, and `ControlCore` is free to release the lock and
then call into the connection, which may then block indefinitely.
## Validation Steps Performed
* Start nvim in WSL
* Press `i` to enter the regular Insert mode
* Paste 1MB of text
* Doesn't deadlock ✅
Adds
```xml
<uap17:UpdateWhileInUse>defer</uap17:UpdateWhileInUse>
```
to our `Package.Properties` for all our packages.
This was added in the September 2023 OS release of Windows 11.
Apparently, this just works now? I did update VS,
but I don't _think_ that updated the SDK.
I have no idea how it updated the manifest definitions.
Closes#3915Closes#6726
- AtlasEngine: Minor bug fixes (GH-16219)
- Fix the fix for the fix of nearby font loading (GH-16196)
- Added selectionBackground to light color schemes (GH-16243)
- Another theoretical fix for a crash (GH-16267)
- Fix tabs being printed in cmd.exe prompts (GH-16273)
Related work items: MSFT-47266988
A late change in #16105 wrapped `_buffer` into a class to better track
its dirty state, but I failed to notice that in this one instance we
intentionally manipulated `_buffer` without marking it as dirty.
This fixes the issue by adding a call to `MarkAsClean()`.
This changeset also adds the test instructions from #15783 as a
document to this repository. I've extended the list with two
bugs we've found in the implementation since then.
## Validation Steps Performed
* In cmd.exe, with an empty prompt in an empty directory:
Pressing tab produces an audible bing and prints no text ✅
(cherry picked from commit 7a8dd90294)
Service-Card-Id: 91033502
Service-Version: 1.19
For history:
> This is MSFT:46763065 internally. Dumps show this repros on 1.19 too.
>
> This was previously #16061 which had a theoretical fix in #16065.
Looks like you're on Terminal Stable v1.18.2822.0, and
https://github.com/microsoft/terminal/releases/tag/v1.18.2822.0 is
supposed to have had that fix in it. Dang.
> well this is embarrassing ... I never actually checked if we _still
had a `_window`_. We're alive, yay! But we're still in the middle of
refrigerating. So, there's no HWND anymore
Attempt to fix this by actually ensuring there's a `_window` in
`AppHost::_WindowInitializedHandler`
Closes#16235
(cherry picked from commit 59dcbbe0e9)
Service-Card-Id: 91041359
Service-Version: 1.19
Add a selectionBackground property which is set to the scheme's
brightBlack too all 3 of the light color schemes.
Related to #8716
It does not close the bug because as mentioned in the issue, when you
input numbers, they seem to be invisible in the light color schemes and
selecting them with the cursor doesn't reveal them.
(cherry picked from commit a5c269b280)
Service-Card-Id: 91033167
Service-Version: 1.19
I still don't know how to reproduce it properly, but I'm slowly
wrapping my head around how and why it happens. The issue isn't that
`FindFamilyName` fails with `exists=FALSE`, but rather that any of the
followup calls like `GetDesignGlyphMetrics` fails, which results in an
exception and subsequently in an orderly fallback to Consolas.
I've always thought that the issue is that even with the nearby font
collection we get an `exists=FALSE`... I'm not sure why I thought that.
This changeset also drops the fallback iteration for Lucida Console and
Courier New, because I felt like the code looks neater that way and I
think it's a reasonable expectation that Consolas is always installed.
Closes#16058
(cherry picked from commit 9e86c9811f)
Service-Card-Id: 90885607
Service-Version: 1.19
This commit fixes 4 minor bugs:
* Forgot to set the maximum swap chain latency. Without it, it defaults
to up to 3 frames of latency. We don't need this, because our renderer
is simple and fast and is expected to draw frames within <1ms.
* ClearType treats the alpha channel as ignored, whereas custom shaders
can manipulate the alpha channel freely. This meant that using both
simultaneously would produce weird effects, like text having black
background. We now force grayscale AA instead.
* The builtin retro shader should not be effected by the previous point.
* When the cbuffer is entirely unused in a custom shader, it has so far
resulted in constant redraws. This happened because the D3D reflection
`GetDesc` call will then return `E_FAIL` in this situation.
The new code on the other hand will now assume that a failure
to get the description is equal to the variable being unused.
Closes#15960
## Validation Steps Performed
* A custom passthrough shader works with grayscale and ClearType AA
while also changing the opacity with Ctrl+Shift+Scroll ✅
* Same for the builtin retro shader, but ClearType works ✅
* The passthrough shader doesn't result in constant redrawing ✅
(cherry picked from commit 0289cb043c)
Service-Card-Id: 90915277
Service-Version: 1.19
A late change in #16105 wrapped `_buffer` into a class to better track
its dirty state, but I failed to notice that in this one instance we
intentionally manipulated `_buffer` without marking it as dirty.
This fixes the issue by adding a call to `MarkAsClean()`.
This changeset also adds the test instructions from #15783 as a
document to this repository. I've extended the list with two
bugs we've found in the implementation since then.
## Validation Steps Performed
* In cmd.exe, with an empty prompt in an empty directory:
Pressing tab produces an audible bing and prints no text ✅
For history:
> This is MSFT:46763065 internally. Dumps show this repros on 1.19 too.
>
> This was previously #16061 which had a theoretical fix in #16065.
Looks like you're on Terminal Stable v1.18.2822.0, and
https://github.com/microsoft/terminal/releases/tag/v1.18.2822.0 is
supposed to have had that fix in it. Dang.
> well this is embarrassing ... I never actually checked if we _still
had a `_window`_. We're alive, yay! But we're still in the middle of
refrigerating. So, there's no HWND anymore
Attempt to fix this by actually ensuring there's a `_window` in
`AppHost::_WindowInitializedHandler`
Closes#16235
Add a selectionBackground property which is set to the scheme's
brightBlack too all 3 of the light color schemes.
Related to #8716
It does not close the bug because as mentioned in the issue, when you
input numbers, they seem to be invisible in the light color schemes and
selecting them with the cursor doesn't reveal them.
The `Telemetry` class was implemented as a singleton which stood in
my long-term goal to remove all global variables from the project.
Most telemetry captured by it hasn't been looked at for a long time
and just as much is now pointless (e.g.,`_fCtrlPgUpPgDnUsed`).
This removes the code.
## Validation Steps Performed
* Still compiles ✅
I still don't know how to reproduce it properly, but I'm slowly
wrapping my head around how and why it happens. The issue isn't that
`FindFamilyName` fails with `exists=FALSE`, but rather that any of the
followup calls like `GetDesignGlyphMetrics` fails, which results in an
exception and subsequently in an orderly fallback to Consolas.
I've always thought that the issue is that even with the nearby font
collection we get an `exists=FALSE`... I'm not sure why I thought that.
This changeset also drops the fallback iteration for Lucida Console and
Courier New, because I felt like the code looks neater that way and I
think it's a reasonable expectation that Consolas is always installed.
Closes#16058
This commit fixes 4 minor bugs:
* Forgot to set the maximum swap chain latency. Without it, it defaults
to up to 3 frames of latency. We don't need this, because our renderer
is simple and fast and is expected to draw frames within <1ms.
* ClearType treats the alpha channel as ignored, whereas custom shaders
can manipulate the alpha channel freely. This meant that using both
simultaneously would produce weird effects, like text having black
background. We now force grayscale AA instead.
* The builtin retro shader should not be effected by the previous point.
* When the cbuffer is entirely unused in a custom shader, it has so far
resulted in constant redraws. This happened because the D3D reflection
`GetDesc` call will then return `E_FAIL` in this situation.
The new code on the other hand will now assume that a failure
to get the description is equal to the variable being unused.
Closes#15960
## Validation Steps Performed
* A custom passthrough shader works with grayscale and ClearType AA
while also changing the opacity with Ctrl+Shift+Scroll ✅
* Same for the builtin retro shader, but ClearType works ✅
* The passthrough shader doesn't result in constant redrawing ✅
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 3fc4bb99c75451d0ecadd7e4c8fe06ad67217574
Related work items: MSFT-47266988
As in the title. Also fixes a crash for refrigeration with the rainbow
border.
Closes#16211
Tested by manually forcing us into Windows 10 mode (to refrigerate the
window). That immediately repros the bug, which was simple enough to
fix.
(cherry picked from commit d8c7719bfb)
Service-Card-Id: 90928408
Service-Version: 1.19
The initial cooked read (= conhost readline) rewrite had two flaws:
* Using viewport scrolls under ConPTY to avoid emitting newlines
resulted in various bugs around marks, coloring, etc. It's still
somewhat unclear why this happened, but the next issue is related and
much worse.
* Rewriting the input line every time causes problems with accessibility
tools, as they'll re-announce unchanged parts again and again.
The solution to these is to simply stop writing the unchanged parts of
the prompt. To do this, code was added to measure the size of text
without actually inserting them into the buffer. Since this meant that
the "interactive" mode of `WriteCharsLegacy` would need to be duplicated
for the new code, I instead moved those parts into `COOKED_READ_DATA`.
That way we can now have the interactive transform of the prompt (=
Ctrl+C -> ^C) and the two text functions (measure text & actually write
text) are now agnostic to this transformation.
Closes#16034Closes#16044
## Validation Steps Performed
* A vision impaired user checked it out and it seemed fine ✅
(cherry picked from commit e1c69a99ce)
Service-Card-Id: 90891693
Service-Version: 1.19
cd6b083 had 2 issues:
* Improper testing with Ctrl+M instead of Edit > Mark.
* Wrong SelectionState function being used. When the selection is
initiated without keyboard or mouse, `IsKeyboardMarkSelection`
returns false. The proper function to use is `IsLineSelection`.
Closes#15153
## Validation Steps Performed
* Run Far
* Start selection via Edit>Mark
* Hold Alt while dragging to make a rectangular selection
* Right click
* Clipboard contains a rectangular copy ✅
(cherry picked from commit d496a5fb80)
Service-Card-Id: 90886368
Service-Version: 1.19
This restores the original code from before 821ae3a where
the `.GetMainBuffer()` call was accidentally removed.
Closes#16158
## Validation Steps Performed
* Run this Python script:
```py
import sys
while True:
sys.stdout.write("\033[?1049h")
sys.stdout.flush()
sys.stdin.readline()
sys.stdout.write("\033[?1049l")
```
* Press enter repeatedly
* Doesn't crash ✅
(cherry picked from commit 08f30330d1)
Service-Card-Id: 90861143
Service-Version: 1.19
eb871bf fails to properly handle REG_SZ strings, which are documented as
being null-terminated _and_ length restricted.
`wcsnlen` is the perfect fit for handling this situation as it returns
the position of the first \0, or the given length parameter.
As a drive by improvement, this also drops some redundant code:
* `to_environment_strings_w` which is the same as `to_string`
* Retrieving `USERNAME`/`USERDOMAIN` via `LookupAccountSidW` and
`COMPUTERNAME` via `GetComputerNameW` is not necessary as the
variables are "volatile" and I believe there's generally no
expectation that they change unless you log in again.
Closes#16051
## Validation Steps Performed
* Run this in PowerShell to insert a env value with \0:
```pwsh
$hklm = [Microsoft.Win32.RegistryKey]::OpenBaseKey(
[Microsoft.Win32.RegistryHive]::LocalMachine,
0
)
$key = $hklm.OpenSubKey(
'SYSTEM\CurrentControlSet\Control\Session Manager\Environment',
$true
)
$key.SetValue('test', "foo`0bar")
```
* All `EnvTests` still pass ✅
* (Don't forget to remove the above value again!)
(cherry picked from commit 64b5b2884a)
Service-Card-Id: 90879164
Service-Version: 1.19
Guess what _doesn't_ have the same layout as a bitmap? A `til::color`.
Noticed in 1.19.
Regressed in #16006
(cherry picked from commit 1745857407)
Service-Card-Id: 90758500
Service-Version: 1.19
This fixes a cosmetic issue with the version number in the unpackaged
builds and NuGet packages.
They were showing up as `-preview`, even when they were stable, because
the variable template didn't know about the branding.
(cherry picked from commit 544cdd78af)
Service-Card-Id: 90786432
Service-Version: 1.19
Wrap single quotes to drag and dropped paths in WSL
## References and Relevant Issues
#15646 , #8109
## Detailed Description of the Pull Request / Additional comments
First time contributor, from what I understand from reading #15646 and #8109 , issue is asking for single quotes added to a drag and dropped path always, regardless of whitespace and special characters, in WSL.
## Validation Steps Performed
Tested drag and drop changes in WSL and non WSL sources.
Closes#15646
As in the title. Also fixes a crash for refrigeration with the rainbow
border.
Closes#16211
Tested by manually forcing us into Windows 10 mode (to refrigerate the
window). That immediately repros the bug, which was simple enough to
fix.
The initial cooked read (= conhost readline) rewrite had two flaws:
* Using viewport scrolls under ConPTY to avoid emitting newlines
resulted in various bugs around marks, coloring, etc. It's still
somewhat unclear why this happened, but the next issue is related and
much worse.
* Rewriting the input line every time causes problems with accessibility
tools, as they'll re-announce unchanged parts again and again.
The solution to these is to simply stop writing the unchanged parts of
the prompt. To do this, code was added to measure the size of text
without actually inserting them into the buffer. Since this meant that
the "interactive" mode of `WriteCharsLegacy` would need to be duplicated
for the new code, I instead moved those parts into `COOKED_READ_DATA`.
That way we can now have the interactive transform of the prompt (=
Ctrl+C -> ^C) and the two text functions (measure text & actually write
text) are now agnostic to this transformation.
Closes#16034Closes#16044
## Validation Steps Performed
* A vision impaired user checked it out and it seemed fine ✅
cd6b083 had 2 issues:
* Improper testing with Ctrl+M instead of Edit > Mark.
* Wrong SelectionState function being used. When the selection is
initiated without keyboard or mouse, `IsKeyboardMarkSelection`
returns false. The proper function to use is `IsLineSelection`.
Closes#15153
## Validation Steps Performed
* Run Far
* Start selection via Edit>Mark
* Hold Alt while dragging to make a rectangular selection
* Right click
* Clipboard contains a rectangular copy ✅
This restores the original code from before 821ae3a where
the `.GetMainBuffer()` call was accidentally removed.
Closes#16158
## Validation Steps Performed
* Run this Python script:
```py
import sys
while True:
sys.stdout.write("\033[?1049h")
sys.stdout.flush()
sys.stdin.readline()
sys.stdout.write("\033[?1049l")
```
* Press enter repeatedly
* Doesn't crash ✅
eb871bf fails to properly handle REG_SZ strings, which are documented as
being null-terminated _and_ length restricted.
`wcsnlen` is the perfect fit for handling this situation as it returns
the position of the first \0, or the given length parameter.
As a drive by improvement, this also drops some redundant code:
* `to_environment_strings_w` which is the same as `to_string`
* Retrieving `USERNAME`/`USERDOMAIN` via `LookupAccountSidW` and
`COMPUTERNAME` via `GetComputerNameW` is not necessary as the
variables are "volatile" and I believe there's generally no
expectation that they change unless you log in again.
Closes#16051
## Validation Steps Performed
* Run this in PowerShell to insert a env value with \0:
```pwsh
$hklm = [Microsoft.Win32.RegistryKey]::OpenBaseKey(
[Microsoft.Win32.RegistryHive]::LocalMachine,
0
)
$key = $hklm.OpenSubKey(
'SYSTEM\CurrentControlSet\Control\Session Manager\Environment',
$true
)
$key.SetValue('test', "foo`0bar")
```
* All `EnvTests` still pass ✅
* (Don't forget to remove the above value again!)
This addresses the following issues:
* The JSON Schema spec doesn't actually define whether objects with
a "properties" key still require `"type": "object"` or not.
VS Code for instance largely pretends as if it's implied, but when it
encounters them inside a `oneOf` tree, then it behaves as if it isn't.
In other words, we need to always set `"type": "object"`.
* Declaring an `oneOf` containing a `"type": "string"` and an `enum`
doesn't work, because if one of the `enum` cases is given, it results
in both variants to match, since any `enum` is also a `string`.
We have to use `anyOf` instead.
* `SuggestionSource` used `"BuiltinSuggestionSource"` inside a `type`
key which doesn't work. We have to use `$ref` for that.
Closes#13387
## Validation Steps Performed
* VS Code stops complaining ✅
* https://www.jsonschemavalidator.net/✅
## Summary of the Pull Request
Dependency support is now GA in WinGet. Updating the instructions in
README
## References and Relevant Issues
## Detailed Description of the Pull Request / Additional comments
## Validation Steps Performed
## PR Checklist
- [ ] Closes #xxx
- [ ] 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 (if necessary)
This fixes a cosmetic issue with the version number in the unpackaged
builds and NuGet packages.
They were showing up as `-preview`, even when they were stable, because
the variable template didn't know about the branding.
If you're already in the "output" state, then an app requesting an
"output" mark probably shouldn't end the current mark and start a new
one. It should just keep on keepin' on.
The decision to end the previous one was arbitrary in the first place,
so let's arbitrarily change it back.
Especially noticable if you hit <kbd>Enter</kbd> during a command,
because the auto-mark prompt work will do a CommandEnd, so long-running
commands will get broken into multiple marks 🥲
This pull request also removes the original release and nightly
pipelines, but it does not remove the release pipeline _template_.
I had to demote the Azure job from being a _deployment_ to being a plain
old job, unfortunately. Alas! Review with whitespace disabled (or `git
diff -w`).
`GetAt` can throw if the index is out of range. We don't check that in
some places. This fixes some of those.
I don't think this will take care of #15689, but it might help?
(cherry picked from commit 5aadddaea9)
Service-Card-Id: 90731981
Service-Version: 1.19
`GetAt` can throw if the index is out of range. We don't check that in
some places. This fixes some of those.
I don't think this will take care of #15689, but it might help?
## Summary of the Pull Request
> ## Abstract
>
> Multiple related scenarios have come up where it would be beneficial
to display
> actionable UI to the user within the context of the active terminal
itself. This
> UI would be akin to the Intellisense UI in Visual Studio. It appears
right where
> the user is typing, and can help provide immediate content for the
user, based
> on some context. The "Suggestions UI" is this new ephemeral UI within
the
> Windows Terminal that can display different types of actions, from
different
> sources.
>
## Detailed Description of the Pull Request / Additional comments
_\*<sup>\*</sup><sub>\*</sub> read the spec
<sub>\*</sub><sup>\*</sup>\*_
Similar to #14792, a lot of this code is written. This stuff isn't
checked in though, so I'm presenting formally before I start yeeting PRs
out there.
## PR Checklist
- [x] This is a spec for #1595. It also references:
* #3121
* #10436
* #12927
* #12863
Well, Pane doesn't _only_ care if the connection isn't entering a
terminal state. It does need to update its own state first.
Regressed in #15335Closes#16068
(cherry picked from commit 4145f18768)
Service-Card-Id: 90731934
Service-Version: 1.19
Updated the function `TerminalPage::CloseWindow` to include logic for
closing context and flyout menus so that they are dismissed before the
warning is displayed.
Closes#16039
(cherry picked from commit aafb91745e)
Service-Card-Id: 90731989
Service-Version: 1.19
Fixes MSFT:46725264
don't explode trying to parse a URL, if the string wasn't one.
(cherry picked from commit 59aaba7c5b)
Service-Card-Id: 90687770
Service-Version: 1.19
See MSFT:46763065. Looks like we're in the middle of being
`Refrigerate`d, we're pumping messages, and as we pump messages, we get
to a `co_await` in `AppHost::_WindowInitializedHandler`. When we resume,
we just try to use `this` like everything's fine but OH NO, IT'S NOT.
To fix this, I'm
* Adding `enable_shared_from_this` to `AppHost`
* Holding the `AppHost` in a shared_ptr in WindowThread
- though, this is a singular owning `shared_ptr`. This is probably ripe
for other footguns, but there's little we can do about this.
* whenever we `co_await` in `AppHost`, make sure we grab a weak ref
first, and check it on the other side.
This is another "squint and yep that's a bug" fix, that I haven't been
able to verify locally. This is
[allegedly](https://media.tenor.com/VQi3bktwLdIAAAAC/allegedly-supposedly.gif)
about 10% of our 1.19 crashes after 3 days.
Closes#16061
(cherry picked from commit 8521aae889)
Service-Card-Id: 90731962
Service-Version: 1.19
Wow our preview population must just not use `startupActions`. This
obviously never worked in 1.18 Preview.
Closes#16050
(cherry picked from commit f6425dbd59)
Service-Card-Id: 90715243
Service-Version: 1.19
Well, Pane doesn't _only_ care if the connection isn't entering a
terminal state. It does need to update its own state first.
Regressed in #15335Closes#16068
Updated the function `TerminalPage::CloseWindow` to include logic for
closing context and flyout menus so that they are dismissed before the
warning is displayed.
Closes#16039
See MSFT:46763065. Looks like we're in the middle of being
`Refrigerate`d, we're pumping messages, and as we pump messages, we get
to a `co_await` in `AppHost::_WindowInitializedHandler`. When we resume,
we just try to use `this` like everything's fine but OH NO, IT'S NOT.
To fix this, I'm
* Adding `enable_shared_from_this` to `AppHost`
* Holding the `AppHost` in a shared_ptr in WindowThread
- though, this is a singular owning `shared_ptr`. This is probably ripe
for other footguns, but there's little we can do about this.
* whenever we `co_await` in `AppHost`, make sure we grab a weak ref
first, and check it on the other side.
This is another "squint and yep that's a bug" fix, that I haven't been
able to verify locally. This is
[allegedly](https://media.tenor.com/VQi3bktwLdIAAAAC/allegedly-supposedly.gif)
about 10% of our 1.19 crashes after 3 days.
Closes#16061
This pipeline does everything the existing release pipeline does, except
it does it using the OneBranch official templates.
Most of our existing build infrastructure has been reused, with the
following changes:
- We are no longer using `job-submit-windows-vpack`, as OneBranch does
this for us.
- `job-merge-msix-into-bundle` now supports afterBuildSteps, which we
use to stage the msixbundle into the right place for the vpack
- `job-build-project` supports deleting all non-signed files (which the
OneBranch post-build validation requires)
- `job-build-project` now deletes `console.dll`, which is unused in any
of our builds, because XFGCheck blows up on it for some reason on x86
- `job-publish-symbols` now supports two different types of PAT
ingestion
- I have pulled out the NuGet filename variables into a shared variables
template
I have also introduced a TSA config (which files bugs on us for binary
analysis failures as well as using the word 'sucks' and stuff.)
I have also baselined a number of control flow guard/binary analysis
failures.
(cherry picked from commit 6489f6b39d)
Service-Card-Id: 90706777
Service-Version: 1.19
This pipeline does everything the existing release pipeline does, except
it does it using the OneBranch official templates.
Most of our existing build infrastructure has been reused, with the
following changes:
- We are no longer using `job-submit-windows-vpack`, as OneBranch does
this for us.
- `job-merge-msix-into-bundle` now supports afterBuildSteps, which we
use to stage the msixbundle into the right place for the vpack
- `job-build-project` supports deleting all non-signed files (which the
OneBranch post-build validation requires)
- `job-build-project` now deletes `console.dll`, which is unused in any
of our builds, because XFGCheck blows up on it for some reason on x86
- `job-publish-symbols` now supports two different types of PAT
ingestion
- I have pulled out the NuGet filename variables into a shared variables
template
I have also introduced a TSA config (which files bugs on us for binary
analysis failures as well as using the word 'sucks' and stuff.)
I have also baselined a number of control flow guard/binary analysis
failures.
The version we were using requires .NET 2.1 (wow) which is way out of
support.
Task version 3 supports much newer versions.
(cherry picked from commit ac2b0e744c)
Service-Card-Id: 90688108
Service-Version: 1.19
Control Flow Guard requires both linker and compiler flags.
It turns out that the MSVC build rules determine whether to _link_ with
CFG based on whether it compiled anything with CFG.
It also turns out that when you don't compile anything (such as in our
DLL projects that only consume a static library!), the build rules can't
guess whether to link with CFG.
Whoops.
We need to force it.
(cherry picked from commit 1b143e34a8)
Service-Card-Id: 90688105
Service-Version: 1.19
Found this while looking through dumps for failure
`f544cf8e-1879-c59b-3f0b-1a364b92b974`. That's MSFT:45210947. (1% of our
1.19 crashes)
From the dump I looked at,
Looks like,
* we're on Windows 10
* We're refrigerating a window
* We are pumping the remaining XAML messages as we refrigerate
(`_pumpRemainingXamlMessages`)
* In there, we're finally getting the
`TerminalPage::_CompleteInitialization`
* that calls up to the `_root->Initialized` lambda set up in
`TerminalWindow::Initialize`
* There it tries to get the launch mode from the settings, and explodes.
Presumably _settings is null, but can't see in this dump.
so the window is closing before it's initialized.
When we `_warmWindow = std::move(_host->Refrigerate())`, we call
`AppHost::Refrigerate`, which will null out the TerminalWindow. So when
we're getting to `TerminalWindow::Initialize`, we're calling that on a
nullptr. That's the trick.
We need to revoke the internal Initialized callback. Which makes sense.
It's a lambda that binds _this_ 🤦
---
After more looking, it really doesn't _seem_ like the stacks that are
tracked in `f544cf8e-1879-c59b-3f0b-1a364b92b974` look like the same
stack that I was debugging, but this _is_ a realy issue regardless.
(cherry picked from commit 7073ec01bf)
Service-Card-Id: 90672654
Service-Version: 1.19
f1aa699 was fundamentally incorrect as it used `IdnToAscii` and
`IdnToUnicode` on the entire URL, even though these functions only work
on domain names. This commit fixes the issue by using the WinRT `Url`
class and its `AbsoluteUri` and `AbsoluteCanonicalUri` getters.
The algorithm still works the same way though.
Closes#16017
## Validation Steps Performed
* ``"`e]8;;https://www.xn--fcbook-3nf5b.com/`e\test`e]8;;`e\"``
still shows as two URLs in the popup ✅
* Shows the given URI if it's canonical and not an IDN ✅
* Works with >100 char long file:// URIs ✅
(cherry picked from commit 198c11f36d)
Service-Card-Id: 90642844
Service-Version: 1.19
One day into 1.19, and there's a LOT of hits here (**76.25%** of our
~300 crashes). A crash if the Theme doesn't have a `tab` member.
Regressed in #15948
Closes MSFT:46714723
(cherry picked from commit cf193858f6)
Service-Card-Id: 90670731
Service-Version: 1.19
Control Flow Guard requires both linker and compiler flags.
It turns out that the MSVC build rules determine whether to _link_ with
CFG based on whether it compiled anything with CFG.
It also turns out that when you don't compile anything (such as in our
DLL projects that only consume a static library!), the build rules can't
guess whether to link with CFG.
Whoops.
We need to force it.
Unfortunately, the appLicensing restricted capability we used to make
Canary installable without the store only works on Windows 11. Because
of that, we have to restrict the app package to Windows 11 and above.
I'd rather not leave Windows 10 users out in the cold, so this pull
request also publishes Canary builds to the public storage bucket with
the name `Microsoft.WindowsTerminalCanary_latest_x64.zip` (etc.)
The version number will be kept inside the archive. It remains to be
seen whether that is a good idea!
When combined with #16048, Canary builds from Azure will automatically
run in portable mode!
I also added support to the unpackaged distribution script to produce
portable mode packages. It is off by default for AppX->ZIP builds and
**on** by default for Layout->ZIP builds.
This constitutes a change in behavior.
Found this while looking through dumps for failure
`f544cf8e-1879-c59b-3f0b-1a364b92b974`. That's MSFT:45210947. (1% of our
1.19 crashes)
From the dump I looked at,
Looks like,
* we're on Windows 10
* We're refrigerating a window
* We are pumping the remaining XAML messages as we refrigerate
(`_pumpRemainingXamlMessages`)
* In there, we're finally getting the
`TerminalPage::_CompleteInitialization`
* that calls up to the `_root->Initialized` lambda set up in
`TerminalWindow::Initialize`
* There it tries to get the launch mode from the settings, and explodes.
Presumably _settings is null, but can't see in this dump.
so the window is closing before it's initialized.
When we `_warmWindow = std::move(_host->Refrigerate())`, we call
`AppHost::Refrigerate`, which will null out the TerminalWindow. So when
we're getting to `TerminalWindow::Initialize`, we're calling that on a
nullptr. That's the trick.
We need to revoke the internal Initialized callback. Which makes sense.
It's a lambda that binds _this_ 🤦
---
After more looking, it really doesn't _seem_ like the stacks that are
tracked in `f544cf8e-1879-c59b-3f0b-1a364b92b974` look like the same
stack that I was debugging, but this _is_ a realy issue regardless.
f1aa699 was fundamentally incorrect as it used `IdnToAscii` and
`IdnToUnicode` on the entire URL, even though these functions only work
on domain names. This commit fixes the issue by using the WinRT `Url`
class and its `AbsoluteUri` and `AbsoluteCanonicalUri` getters.
The algorithm still works the same way though.
Closes#16017
## Validation Steps Performed
* ``"`e]8;;https://www.xn--fcbook-3nf5b.com/`e\test`e]8;;`e\"``
still shows as two URLs in the popup ✅
* Shows the given URI if it's canonical and not an IDN ✅
* Works with >100 char long file:// URIs ✅
One day into 1.19, and there's a LOT of hits here (**76.25%** of our
~300 crashes). A crash if the Theme doesn't have a `tab` member.
Regressed in #15948
Closes MSFT:46714723
The `GenRTF(...)` was using `\highlight` control word for sending
background text color in the RTF format during a copy command. This
doesn't work correctly, since many applications (E.g. MSWord) don't
support full RGB with `\highlight`, and instead uses an approximation of
what is received. For example, `rgb(197, 15, 31)` becomes `rgb(255, 0,
255)`. Also, the standard way of using background colors is `\cbN`
control word, which isn't supported as per the [RTF Spec 1.9.1]
in Word.
But it briefly mentioned a workaround at Pg. 23, which seems to work on
all the RTF editors I tested.
The PR makes the changes to use `\chshdng0\chcbpatN` for the background
coloring.
Also did some refactoring to make the implementation concise.
## Validation Steps Performed
Verified that the background is correctly copied on below editors:
- MSWord
- WordPad
- LibreOffice
- Outlook
[RTF Spec 1.9.1]: https://msopenspecs.azureedge.net/files/Archive_References/[MSFT-RTF].pdf
Subjectively speaking, this commit makes 3 improvements:
* Most importantly, it now would work with arbitrary Unicode text.
(No more `IsGlyphFullWidth` or DBCS handling during reflow.)
* Due to the simpler implementation it hopefully makes review of
future changes and maintenance simpler. (~3x less LOC.)
* It improves perf. by 1-2 orders of magnitude.
(At 120x9001 with a full buffer I get 60ms -> 2ms.)
Unfortunately, I'm not confident that the new code replicates the old
code exactly, because I failed to understand it. During development
I simply tried to match its behavior with what I think reflow should do.
Closes#797Closes#3088Closes#4968Closes#6546Closes#6901Closes#15964
Closes MSFT:19446208
Related to #5800 and #8000
## Validation Steps Performed
* Unit tests ✅
* Feature tests ✅
* Reflow with a scrollback ✅
* Reflowing the cursor cell causes a forced line-wrap ✅
(Even at the end of the buffer. ✅)
* `color 8f` and reflowing retains the background color ✅
* Enter alt buffer, Resize window, Exit alt buffer ✅
(cherry picked from commit 74748394c1)
Service-Card-Id: 90642727
Service-Version: 1.19
A carriage return (enter key) will increase the _distanceEnd by up to
viewport-width many columns, since it increases the Y distance between
the start and end by 1 (it's a newline after all).
This will make _flushBuffer() think that the new _buffer is way longer
than the old one and so _erase() ends up not erasing the tail end of
the prompt, even if the new prompt is actually shorter.
This commit fixes the issue by separating the newline printing
out from the regular text printing loops.
## Validation Steps Performed
* Run cmd.exe
* Write "echo hello" and press Enter
* Write "foobar foo bar" (don't press Enter)
* Press F7, select "echo hello" and press Enter
* Previous prompt says "echo hello" ✅
(cherry picked from commit c7f30a86d7)
Service-Card-Id: 90642765
Service-Version: 1.19
With us adding a .appinstaller distribution of Canary, the Store
services update checker has beome insufficient to determine whether
there are package updates.
App Installer supports us checking for updates by using PackageManager
and the Package interfaces.
We'll use those instead of the Store services interface, and bail out
early if the App Installer gives us an answer.
(cherry picked from commit e0fc3bcd0a)
Service-Card-Id: 90644882
Service-Version: 1.19
Subjectively speaking, this commit makes 3 improvements:
* Most importantly, it now would work with arbitrary Unicode text.
(No more `IsGlyphFullWidth` or DBCS handling during reflow.)
* Due to the simpler implementation it hopefully makes review of
future changes and maintenance simpler. (~3x less LOC.)
* It improves perf. by 1-2 orders of magnitude.
(At 120x9001 with a full buffer I get 60ms -> 2ms.)
Unfortunately, I'm not confident that the new code replicates the old
code exactly, because I failed to understand it. During development
I simply tried to match its behavior with what I think reflow should do.
Closes#797Closes#3088Closes#4968Closes#6546Closes#6901Closes#15964
Closes MSFT:19446208
Related to #5800 and #8000
## Validation Steps Performed
* Unit tests ✅
* Feature tests ✅
* Reflow with a scrollback ✅
* Reflowing the cursor cell causes a forced line-wrap ✅
(Even at the end of the buffer. ✅)
* `color 8f` and reflowing retains the background color ✅
* Enter alt buffer, Resize window, Exit alt buffer ✅
A carriage return (enter key) will increase the _distanceEnd by up to
viewport-width many columns, since it increases the Y distance between
the start and end by 1 (it's a newline after all).
This will make _flushBuffer() think that the new _buffer is way longer
than the old one and so _erase() ends up not erasing the tail end of
the prompt, even if the new prompt is actually shorter.
This commit fixes the issue by separating the newline printing
out from the regular text printing loops.
## Validation Steps Performed
* Run cmd.exe
* Write "echo hello" and press Enter
* Write "foobar foo bar" (don't press Enter)
* Press F7, select "echo hello" and press Enter
* Previous prompt says "echo hello" ✅
With us adding a .appinstaller distribution of Canary, the Store
services update checker has beome insufficient to determine whether
there are package updates.
App Installer supports us checking for updates by using PackageManager
and the Package interfaces.
We'll use those instead of the Store services interface, and bail out
early if the App Installer gives us an answer.
After the nightly build completes, we'll automatically generate a
.appinstaller and publich it plus the msixbundle to an Azure Storage
account.
I had to add step/job customization to the publish step in the full
release pipeline template.
The .appinstaller hardcodes our XAML dependency, which makes it a bit of
a pain. We can revisit this later, and publish our dependencies
directly and automatically instead of hardcoding them.
I am considering moving the appinstaller generation step to the MSIX
bundling job, but this works right now and is not too terrible.
Closes#774
When launching a debug Terminal, `_initializedTerminal` might still be false and the scrollbar might still be 0px tall. This causes the `assert(false)` condition within `_throttledUpdateScrollbar` to be hit.
Regressed in #16006
Previously, all unknown escape sequences would lead to an immediate call
to `VtEngine::_Flush()`. This lead to problems with nushell which uses
FTCS marks that were unknown to us. Combined with the linewise redrawing
that nushell does, Terminal would get the prompt in two separate frames,
causing a slight flickering.
#14677 fixed this by suppressing the `_Flush()` call when unknown
sequences are encountered. Unfortunately, this triggered a bug due
to our somewhat "inconsistent" architecture in conhost:
`XtermEngine::WriteTerminalW` isn't just used to flush unknown sequences
but also used directly by `InputBuffer::PassThroughWin32MouseRequest`
to write its mouse sequence directly to the ConPTY host.
`VtEngine` already contains a number of specialized member functions
like `RequestWin32Input()` to ensure that `_Flush()` is called
immediately and another member could've been added to solve this issue.
This commit now adds `RequestMouseMode` in the same vein.
But I believe we can make the system more robust in general by using
eager flushing by default (= safe), similar to how a `write()` on a
TCP socket flushes by default, and instead only selectively pause and
unpause flushing with a system similar to `TCP_CORK`.
This seems to work fairly well, as it solves:
* The original nushell bug
* The new bug
* Improves overall throughput by ~33% (due to less flushing)
In particular the last point is noteworthy, as this commit removes
the last performance bottleneck in ConPTY that isn't `VtEngine`.
Around ~95% of all CPU and wall time is spent in there now and any
improvements to `VtEngine` should yield immediately results.
Closes#15711
## Validation Steps Performed
* Clone/Run https://github.com/chrisant996/repro_enable_mouse_input
* Hold Ctrl+Alt and circle with the mouse over the viewport
* Repro.exe prints the current cursor coordinates ✅
* Run nushell
* No flickering when typing in the prompt ✅
The Win32 API is significantly faster than the WinRT one, in the order
of around 300-1000x depending on the CPU and CPU load.
This might slightly improve the situation around #15315, but I suspect
that it requires many more fixes. For instance, we don't really have a
single text input "queue" into which we write. Multiple routines that
`resume_background` just to `WriteFile` into the input pipe are thus
racing against each other, contributing to the laggy feeling.
I also fear that the modern Windows text stack might be inherently
RPC based too, producing worse lag with rising CPU load.
This might fix#14323
## Validation Steps Performed
* Paste text from Edge ✅
* Paste text from Notepad ✅
* Right click the address bar in Explorer, choose "Copy address",
paste text into WT ✅
---------
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
This commit fixes 2 issues:
* `ControlCore::ScrollMarks()` would call `ResetIfStale`
again while the search prompt hasn't changed.
This has been fixed by using `_cachedSearchResultRows` as
the indicator for whether it needs to be recreated or not.
* While typing a search query, the selection would move among the
results with each typed character, because `MovePastCurrentSelection`
would do what its name indicates. It has been renamed and rewritten
to be `MoveToCurrentSelection`. To avoid breaking UIA, the previous
`MovePastPoint` implementation was kept.
Since the new `MoveToCurrentSelection` function would not move past the
current selection anymore, changing the direction would not move past
the current result either. To fix this, we now don't invalidate the
search cache when changing the direction.
Closes#15954
## Validation Steps Performed
* Run ``"helloworld`n"*20`` in pwsh
* Search for "helloworld"
* While typing the characters the selection doesn't move ✅
* ...nor when searching downwards ✅
* ...nor when erasing parts of it ✅
* ...and it behaves identical in conhost ✅
`ImmersiveColorSet` gets sent more often than just on a theme change. It notably gets sent when the PC is locked, or the UAC prompt opens.
## Validation Steps Performed
Tested manually by setting the font to `garbo`and:
* locking, then logging back in. No dialog ✅
* UAC via run dialog + `regedit`. No dialog ✅
* Actually changing the OS theme. Dialog ✅Closes#15732
As mentioned in #15760
> > When you right-click on a non-active pane, it becomes active, but the context menu may be displayed before this happens, thus showing the Restart Connection item based the wrong pane's status.
>
> As far as I can see, when a pane is (right)clicked:
>
> 1. If unfocused, `Focus` is called. This goes through the `GotFocus` handler which eventually calls `tab->_UpdateActivePane(sender);`
> 2. `PointerPressed` is raised which eventually shows the context menu
>
> The first point is done asynchronously, so may update the active pane too late when the menu is already displayed (despite both end up in the UI thread).
To fix this: we plumb the control that the context menu was opened for all the way through to where the event is actually handled (in `_PopulateContextMenu`)
* [x] Tested manually
Co-authored-by: Marco Pelagatti <1140981+mpela81@users.noreply.github.com>
Saving the SUI with an empty "keys" will persist `"keys": ""` to the
JSON.
The keychord parser tries to parse that.
`KeyChordSerialization.cpp@_fromString` returns a KeyChord with both
vkey and scancode set to 0, and the ctor asserts and explodes.
We shouldn't do that.
Closes#13221
I noticed this last week, but forgot to file. If you have a pair of
splits, and `exit -1` the first, you can't use `enter` to restart it.
This PR fixes that. Basically, `TerminalPage` registers it's
`_restartPaneConnection` handler when it makes a new `Pane` object. It
registers the callback straight to the `Pane`. However, when a `Pane`
gets split, it makes a _new_ `Pane` object, and moves the original
content into the new pane. `TerminalPage` however, would never hook up
its own callback to that newly created pane.
This fixes that.
This pull request moves HwndTerminal into Microsoft.Terminal.Control.Lib
and removes PublicTerminalCore completely.
Microsoft.Terminal.Control.dll now exports the C API from HwndTerminal.
This adds ~100kb to Microsoft.Terminal.Control.dll and ~1400kb to the
WPF package (per architecture) but with the coming interactivity
platform merge it's going to benefit us big time.
This replaces the use of a `<Canvas>` with an `<Image>` for drawing
scrollbar marks. Otherwise, WinUI struggles with the up to ~9000 UI
elements as they get dirtied every time the scrollbar moves.
(FWIW 9000 is not a lot and it should not struggle with that.)
The `<Image>` element has the benefit that we can get hold of a CPU-side
bitmap which we can manually draw our marks into and then swap them into
the UI tree. It draws the same 9000 elements, but now WinUI doesn't
struggle anymore because only 1 element gets invalidated every time.
Closes#15955
## Validation Steps Performed
* Fill the buffer with "e"
* Searching for "e" fills the entire thumb range with white ✅
* ...doesn't lag when scrolling around ✅
* ...updates quickly when adding newlines at the end ✅
* Marks sort of align with their scroll position ✅
Adding enum iconstyle for hiding the icon in the tab #8157
## Summary of the Pull Request
Please confirm if I am on the right track.
## References and Relevant Issues
## Detailed Description of the Pull Request / Additional comments
## Validation Steps Performed
## PR Checklist
- [ ] Closes#8157
- [x] 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 (if necessary)
This PR is a few things:
* part the first: Convert the `compatibility.reloadEnvironmentVariables`
setting to a per-profile one.
* The settings should migrate it from the user's old global place to the
new one.
* We also added it to "Profile>Advanced" while I was here.
* Adds a new pair of commandline flags to `new-tab` and `split-pane`:
`--inheritEnvironment` / `--reloadEnvironment`
* On `wt` launch, bundle the entire environment that `wt` was spawned
with, and put it into the `Remoting.CommandlineArgs`, and give them to
the monarch (and ultimately, down to `TerminalPage` with the
`AppCommandlineArgs`). DO THIS ALWAYS.
* As a part of this, we’ll default to _reloading_ if there’s no explicit
commandline set, and _inheriting_ if there is.
* For example, `wt -- cmd` would inherit, and `wt -p “Command Prompt”`
would reload.[^1]
* This is a little wacky, but we’re trying to separate out the
intentions here:
* `wt -- cmd` feels like “I want to run cmd.exe (in a terminal tab)”.
That feels like the user would _like_ environment variables from the
calling process. They’re doing something more manual, so they get more
refined control over it.
* `wt` (or `wt -p “Command Prompt”`) is more like, “I want to run the
Terminal (or, my Command Prompt profile) using whatever the Terminal
would normally do”. So that feels more like a situation where it should
just reload by default. (Of course, this will respect their settings
here)
## References and Relevant Issues
https://github.com/microsoft/terminal/issues/15496#issuecomment-1692450231
has more notes.
## Detailed Description of the Pull Request / Additional comments
This is so VERY much plumbing. I'll try to leave comments in the
interesting parts.
## PR Checklist
- [x] This is not _all_ of #15496. We're also going to do a `-E foo=bar`
arg on top of this.
- [x] Tests added/passed
- [x] Schema updated
[^1]: In both these cases, plus the `environment` setting, of course.
## Summary of the Pull Request
When a connection is Closed, show an indicator in the respective tab.
When the active pane's connection is Closed, show a "Restart Connection"
action in the right-click context menu and in the tab context menu.
## Validation Steps Performed
- Force close a connection, check the indicator is shown in the tab.
- Right-click on pane shows the Restart Connection action if its
connection is closed
- Right-click on tab shows the Restart Connection action if the active
pane's connection is closed
- Indicator is cleared after connection is restarted (no panes in closed
state)
## PR Checklist
- [x] Closes#14909
- [x] Tests added/passed
- [ ] Documentation updated
- [ ] Schema updated (if necessary)
Since the "delete color scheme" button is filled with an icon and a Text
Box, the text is not automatically exposed as the autoProp.Name for the
button. We have to do it manually just like we do for "delete profile".
Validated manually using accessibility insights
Closes#15984
`Terminal` is used concurrently by at least 4 threads. The table
below lists the class members and the threads that access them
to the best of my knowledge. Where:
* UI: UI Thread
* BG: Background worker threads (`winrt::resume_background`)
* RD: Render thread
* VT: VT connection thread
| | UI | BG | RD | VT |
|------------------------------------|----|----|----|----|
| `_pfnWriteInput` | x | x | | x |
| `_pfnWarningBell` | | | | x |
| `_pfnTitleChanged` | | | | x |
| `_pfnCopyToClipboard` | | | | x |
| `_pfnScrollPositionChanged` | x | x | | x |
| `_pfnCursorPositionChanged` | | | | x |
| `_pfnTaskbarProgressChanged` | | | | x |
| `_pfnShowWindowChanged` | | | | x |
| `_pfnPlayMidiNote` | | | | x |
| `_pfnCompletionsChanged` | | | | x |
| `_renderSettings` | x | | x | x |
| `_stateMachine` | x | | | x |
| `_terminalInput` | x | | | x |
| `_title` | x | | x | x |
| `_startingTitle` | x | | x | |
| `_startingTabColor` | x | | | |
| `_defaultCursorShape` | x | | | x |
| `_systemMode` | | x | x | x |
| `_snapOnInput` | x | x | | |
| `_altGrAliasing` | x | | | |
| `_suppressApplicationTitle` | x | | | x |
| `_trimBlockSelection` | x | | | |
| `_autoMarkPrompts` | x | | | |
| `_taskbarState` | x | | | x |
| `_taskbarProgress` | x | | | x |
| `_workingDirectory` | x | | | x |
| `_fontInfo` | x | | x | |
| `_selection` | x | x | x | x |
| `_blockSelection` | x | x | x | |
| `_wordDelimiters` | x | x | | |
| `_multiClickSelectionMode` | x | x | x | |
| `_selectionMode` | x | x | x | |
| `_selectionIsTargetingUrl` | x | x | x | |
| `_selectionEndpoint` | x | x | x | |
| `_anchorInactiveSelectionEndpoint` | x | x | x | |
| `_mainBuffer` | x | x | x | x |
| `_altBuffer` | x | x | x | x |
| `_mutableViewport` | x | | x | x |
| `_scrollbackLines` | x | | | |
| `_detectURLs` | x | | | |
| `_altBufferSize` | x | x | x | x |
| `_deferredResize` | x | | | x |
| `_scrollOffset` | x | x | x | x |
| `_patternIntervalTree` | x | x | x | x |
| `_lastKeyEventCodes` | x | | | |
| `_currentPromptState` | x | | | x |
Only 7 members are specific to one thread and don't require locking.
All other members require some for of locking to be safe for use.
To address the issue this changeset adds `LockForReading/LockForWriting`
calls everywhere `_terminal` is accessed in `ControlCore/HwndTerminal`.
Additionally, to ensure these issues don't pop up anymore, it adds to
all `Terminal` functions a debug assertion that the lock is being held.
Finally, because this changeset started off rather modest, it contains
changes that I initially made without being aware about the extent of
the issue. It simplifies the access around `_patternIntervalTree` by
making `_InvalidatePatternTree()` directly use that member.
Furthermore, it simplifies `_terminal->SetCursorOn(!IsCursorOn())` to
`BlinkCursor()`, allowing the code to be shared with `HwndTerminal`.
Ideally `Terminal` should not be that much of a class so that we don't
need such coarse locking. Splitting out selection and rendering state
should allow deduplicating code with conhost and use finer locking.
Closes#9617
## Validation Steps Performed
I tried to use as many Windows Terminal features as I could and fixed
every occurrence of `_assertLocked()` failures.
## Summary of the Pull Request
Resolves the following in #15812
> - [x] `toggleBroadcastInput` isn't in the default settings
> - [x] The cursors forget to keep blinking if you focus each pane and
then unfocus them
> - [x] They don't stop blinking when you unbroadcast
> - [x] Broadcast border doesn't appear when you make new panes, but
they ARE broadcasted-to!
## References and Relevant Issues
x-ref:
* #2634
* #14393
## Detailed Description of the Pull Request / Additional comments
There was literally no logic in the original PR for starting the cursor
blinking. It's entirely unknowable how that ever worked. This makes it
all much more explicit.
We're taking the hacky `DisplayCursorWhileBlurred` from #15363, and
promoting that to the less-hacky `CursorVisibility`. Broadcast input
mode can use that to force the cursor to be visible always.
The last checkbox in that issue is harder, and I didn't want to further
pollute this delta with the paste plumbing.
Previously, the duplication method considered only the next to the selected tab(`tab.TabViewIndex() + 1`) as the insert position. Changed that to consider the setting.
Closes#15776
This is a theoretical improvement for #15553 where Windows Terminal
crashed due to AtlasEngine accessing the soft font bitmap outside of
bounds. The problem is that the soft font cell size was non-zero.
This PR hardens against such situations by checking whether the
requested soft font index is inside the bounds of the bitmaps.
The improvement couldn't be tested as it couldn't be reproduced.
`SetConsoleWindowInfoImpl` calls `PostUpdateWindowSize`, which emits a
`CM_SET_WINDOW_SIZE` event, which causes `_InternalSetWindowSize` to be
called, which calls `ScreenBufferSizeChange` which then finally emits a
`WINDOW_BUFFER_SIZE_EVENT` event into the client input buffer.
This messes up applications like which make use of
`WINDOW_BUFFER_SIZE_EVENT` to perform potentially lossy operations.
In case of SSH this results in a resize (SIGWINCH) of the server-side
screen which similarly may result in a response by the shell, etc.
Since that happens over networks and is async, and because our conhost
VT viewport implementation appears to have a number of subtle bugs,
this results in duplicate output lines (sometimes hundreds).
Under Windows Terminal this issue is not as apparent, since ConPTY has
no viewport that can be moved and no scrollback. It only appears as an
issue if a terminal application reacts poorly to the SIGWINCH event.
Closes#15769
## Validation Steps Performed
* Set a breakpoint in `SynthesizeWindowBufferSizeEvent`
* Launch WSL and cause the viewport to move down
No calls to `SynthesizeWindowBufferSizeEvent` ✅
* Execute `tput reset`
Input line moves to row 0 ✅
WinAppDriver depends on a bunch of .NET assemblies that collide *big time*. Let's just quarantine it.
I kept the fallback to $TESTDIR\WinAppDriver.exe because there's a chance that the Windows build depends on it.
* `[[nodiscard]]` and `[[maybe_unused]]` must come before `virtual` and
`static` qualifiers
* We were calling the jsoncpp constructors directly (again) as functions
(again)
* Some of our preprocessor `#endif` lines were quite messed up
(`-Winvalid-token`)
* One of our test projects was using somebody else's `precomp.h`
Related to #14871
This commit fixes 3 bugs:
* `COOKED_READ_DATA` failed to initialize its `_distanceCursor` and
`_distanceEnd` members. I took this as an opportunity to make them
`ptrdiff_t`, to reduce the likelihood of overflows in the future.
* `COOKED_READ_DATA::_writeChars` added `scrollY` to the written
distance, even though `WriteCharsLegacy` writes a negative value into
that out parameter. This was fixed by changing `WriteCharsLegacy` to
write positive values and by adding a debug assertion.
* `StreamScrollRegion` calls `IncrementCircularBuffer` which causes a
synchronous (!) ConPTY flush to the output pipe (side note: this is
the primary reason why newlines are so slow in ConPTY).
Since cooked reads are supposed to behave like a pager and not write
into the scrollback, we temporarily mark the buffer as inactive
which prevents `TextBuffer` from snitching about it to VtEngine.
Even after this change, there's still some weird behavior left:
* You cannot move your cursor back beyond (0,0), because this isn't a
real pager-like implementation. That might be a neat future extension.
* Writing a lot of text and pressing Ctrl+C doesn't properly place the
cursor and scroll the buffer, unless the cursor is at the end.
That might also be worth investigating in the future (minor issue).
* When the viewport is full, backspacing more than 1 line of text
(using Ctrl+Backspace) doesn't erase all of the affected lines,
because `COOKED_READ_DATA::_erase` uses the same `WriteCharsLegacy`
function to write whitespace to erase that text. It's only gone
after typing one more character.
I've written the code to mostly fix this, but decided against it
as I considered the problem to be too niche to warrant extra code.
Closes#15899
## Validation Steps Performed
* Generate some text to paste in PowerShell:
```pwsh
"" + (0..512 | % { "word" + $_.ToString().PadLeft(4, "0") })
```
* Launch cmd.exe and paste that text
* No flickering ✅
* No writing into the scrollback ✅
* No weird behavior when backspacing ✅
This commit fixes the identity of our new canary packages.
Additionally, it slightly reorders one block so that the file is
almost entirely in the same layout as the preview appxmanifest,
allowing for a better direct comparison (with git diff, etc.).
Underline color sequence _SGR 58_ (unlike *SGR 38*, *SGR 48*) only works
with sub parameters, eg. `\e[58:5:<n>m` or `\e[58:2::<r>:<g>:<b>m` will
work, but something like `\e[58;5;<n>m` won't work. This is a
requirement for the implementation to avoid problems with VT clients
that don't support sub parameters.
## Detailed Description
- Added `underlineColor` to `TextAttribute`, and `UnderlineStyle` into
`CharacterAttributes`.
- Added two new entries in `GraphicOptions` namely, `UnderlineColor`
(58) and `UnderlineColorDefault` (59).
- _SGR 58_ renders a sequence with sub parameters in the VT renderer.
- _SGR 4:x_ renders a sequence with sub parameters in the VT renderer,
except for single, double, and no-underline, which still use
backward-compatible _SGR 4_, _SGR 21_ and _SGR 24_.
- `XtermEngine` will send `\e[4m` without any styling information. This
means all underline style (except NoUnderline) will be rendered as
single underline.
## Reference issues
- #7228
### PR Checklist
- [x] update DECRARA, DECCARA to respect underline color and style.
- [x] update DECRQSS to send underline color and style in the query
response.
- [x] update DECRQPSR/DECRSPS/DECCIR
- [x] Tests added
## Summary of the Pull Request
Closes#7158
Enabling Acrylic as both an appearance setting (with all the plumbing),
allowing it to be set differently in both focused and unfocused
terminals. EnableUnfocusedAcrylic Global Setting that controls if
unfocused acrylic is possible so that people can disable that behavior.
## References and Relevant Issues
#7158 , references: #15913 , #11092
## Detailed Description of the Pull Request / Additional comments
### Allowing Acrylic to be set differently in both focused and unfocused
terminals:
#### A
#### B
#### C
#### D
``` json
"profiles":
{
"list":
[
{
"commandline": "pwsh.exe",
"name": "A",
"unfocusedAppearance":
{
"useAcrylic": true,
},
"useAcrylic": true,
},
{
"commandline": "pwsh.exe",
"name": "B",
"unfocusedAppearance":
{
"useAcrylic": false,
},
"useAcrylic": true,
},
{
"commandline": "pwsh.exe",
"name": "C",
"unfocusedAppearance":
{
"useAcrylic": true,
},
"useAcrylic": false,
},
{
"commandline": "pwsh.exe",
"name": "D",
"unfocusedAppearance":
{
},
"useAcrylic": false,
},
]
}
```
- **A**: AcrylicBlur always on
- **B**: Acrylic when focused, not acrylic when unfocused
- **C**: Why the hell not. Not Acrylic when focused, Acrylic when
unfocused.
- **D:** Possible today by not using Acrylic.
### EnableUnfocusedACrylic global setting that controls if unfocused
acrylic is possible
So that people can disable that behavior:
### Alternate approaches I considered:
Using `_InitializeBackgroundBrush` call instead of
`_changeBackgroundColor(bg) in
``TermControl::_UpdateAppearanceFromUIThread`. Comments in this function
mentioned:
``` *.cs'
// In the future, this might need to be changed to a
// _InitializeBackgroundBrush call instead, because we may need to
// switch from a solid color brush to an acrylic one.
```
I considered using this to tackle to problem, but don't see the benefit.
The only time we need to update the brush is when the user changes the
`EnableUnfocusedAcrylic ` setting which is already covered by
`fire_and_forget TermControl::UpdateControlSettings`
### Supporting different Opacity in Focused and Unfocused Appearance???
This PR is split up in two parts #7158 covers allowing Acrylic to be set
differently in both focused and unfocused terminals. And
EnableUnfocusedAcrylic Global Setting that controls if unfocused acrylic
is possible so that people can disable that behavior.
#11092 will be about enabling opacity as both an appearance setting,
allowing it to be set differently in both focused and unfocused
terminals.
### Skipping the XAML for now:
“I actually think we may want to skip the XAML on this one for now.
We've been having some discussions about compatibility settings, global
settings, stuff like this, and it might be _more- confusing to have you
do something here. We can always add it in post when we decide where to
put it.”
-- Mike Griese
## Validation Steps Performed
#### When Scrolling Mouse , opacity changes appropriately, on opacity
100 there are no gray lines or artefacts
#### When Adjusting Opacity through command palette, opacity changes
appropriately, on opacity 100 there are no gray lines or artefacts
#### When opening command palette state goes to unfocused, the acrylic
and color change appropriately
#### Stumbled upon a new bug when performing validation steps #15913
## PR Checklist
- [x] Closes#7158
- [X] Tests added/passed
- [X] Documentation updated
- If checked, please file a pull request on [our docs
repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
- [x] Schema updated (if necessary)
---------
Co-authored-by: Mike Griese <migrie@microsoft.com>
`TerminalTab::_RecalculateAndApplyReadOnly` didn't know about whether a
tab should be closable or not, based on the theme settings. Similarly
(though, unreported), the theme update in
`TerminalPage::_updateAllTabCloseButtons` didn't really know about
readonly mode.
This fixes both these issues by moving responsibility for the tab close
button visibility into `TabBase` itself.
Closes#15902
I manually changed the permissions on `HKCU\Console` to deny "Create
subkey" to myself. Then confirmed that it explodes before this change,
and not after this change.
Closes#15458
When marking newly scrolled in rows as invalidated we used:
```
if (offset < 0)
...
else
...
```
But it should've been:
```
if (offset < 0)
...
else if (offset > 0)
...
```
Because now it always set the start of the invalidated rows range to 0.
Additionally, this includes a commented debug helper which I've used
to figure out an unrelated bug. During that search I found this bug.
This is a resurrection of #8588. That PR became painfully stale after
the `ControlCore` split. Original description:
> ## Summary of the Pull Request
> This is a PoC for:
> * Search status in SearchBox (aka number of matches + index of the
current match)
> * Live search (aka search upon typing)
> ## Detailed Description of the Pull Request / Additional comments
> * Introduced this optionally (global setting to enable it)
> * The approach is following:
> * Every time the filter changes, enumerate all matches
> * Upon navigation just take the relevant match and select it
>
I cleaned it up a bit, and added support for also displaying the
positions of the matches in the scrollbar (if `showMarksOnScrollbar` is
also turned on).
It's also been made SUBSTANTIALLY easier after #15858 was merged.
Similar to before, searching while there's piles of output running isn't
_perfect_. But it's pretty awful currently, so that's not the end of the
world.
Gifs below.
* closes#8631 (which is a bullet point in #3920)
* closes#6319
Co-authored-by: Don-Vito <khvitaly@gmail.com>
---------
Co-authored-by: khvitaly <khvitaly@gmail.com>
`PaintCursor()` is only called when the cursor is visible, but we need
to invalidate the cursor area even if it isn't. Otherwise a transition
from a visible to an invisible cursor wouldn't be rendered.
I'm confident that this closes#15199
## Validation Steps Performed
* Set blink duration extremely high
* Launch pwsh.exe
* Press Enter a few times
* Press Ctrl+L
* There are never 2 cursors visible, not even briefly ✅
Font features require us to skip the fast path via `GetTextComplexity`.
`IDWriteTextLayout` handles it the same way internally.
Closes#15896
## Validation Steps Performed
* Use Cascadia Code
* Set `features: { "ss19": 1 }`
* "0" has a dash in it instead of a dot ✅
This should allow the package to be installed without AppXSvc consulting
the store or the licensing service.
It's free and open-source. It shouldn't need a license to run.
I have been working feverishly to remove the use of undefined macros used in sources\dirs files (such as this one SDKTOOLS_INC_PATH) that our telemetry constantly catches and reports as an issue.
Related work items: MSFT-40126326
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 8589e01e23c4ec64ad270dbf0c1beb8a78b5a833
A customer reports that `wil::get_token_information` and its use of `wistd::unique_ptr<T>` is hitting a code analysis error in that dynamically-sized token information blocks are allocated with `operator new` but deleted with `delete T*`. This is a "mismatched allocator" error and should be removed.
## What changed?
The output type of token information changed from `wistd::unique_ptr<T>` to `wil::unique_tokeninfo_ptr<T>` which has a custom deleter that uses `operator delete(p)` to match the allocator.
As the new type is incompatible with the old type, all call sites for `wil::GetTokenInformation` were updated to use the new type.
## How was the change tested?
1. Ran the WIL unit tests
2. Prime build of impacted directories
Related: https://github.com/microsoft/wil/pull/306
Related: https://github.com/microsoft/wil/issues/276
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 d86d562b7559c2ca8de036085de6e52e80da8c93
I have been working feverishly to remove the use of undefined macros used in
sources\dirs files (such as this one ONECORE_PRIV_SDK_INC_PATH) that our
telemetry constantly catches and reports as an issue.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 bb08e422cfb4dc2f8b99a2c34bac67a61654a572
Related work items: MSFT-40126326
"Leak in font object 1952 times in last 2k GDI objects created, that lead console to run out of GDI objects."
Fixes MSFT-42906562
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 44f47bf7dbe4bff1986ba5fd8940b56f854c58b7
This leverages Microsoft.Windows.MidlRT to have a project that only
produces a .winmd file, plus a custom MSBuild target that allows for the
TerminalConnection *DLL* project to statically "link" and re-emit the
interface WinMD as its own.
In the most recent compiler ingestion into Windows ("LKG14"), we found
that this particular construction--checking an optional for a value
during this range-for loop--resulted in bad code generation.
When optimized, it generates code that looks effectively like this:
```c++
if (!newRowWidth.has_value()) {
while (true) {
// do the row stuff...
++it;
}
}
```
The loop never exits, and `_RefreshRowIDs` walks off the end of the
buffer. Whoops.
This commit fixes that issue by tricking the optimizer to go another
way. Leonard tells me it's harmless to call `Resize` a bunch of times,
even if it's a no-op, so I trust that this change results in the right
outcome with none of the crashing.
Fixes MSFT-41456525
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 c2b3697c867bddf5660da8b222e99ff4bfd1ea5b
Arm64EC does not support AVX and the usage of it in EC compilation is now an error with the LKG14 compiler update. The fix is to conditionalize using AVX for non-EC compilation.
Related work items: MSFT-42045281
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_we_adept_e4d2 31ca1e08e001988b95ff29a5e098441cae0363bd
By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
:warning: The command is written for posix shells. If it doesn't work for you, you can manually _add_ (one word per line) / _remove_ items to `expect.txt` and the `excludes.txt` files.
If the listed items are:
* ... **misspelled**, then please *correct* them instead of using the command.
@@ -36,7 +34,9 @@ https://www.regexplanet.com/advanced/perl/) yours before committing to verify it
* well-formed pattern.
If you can write a [pattern](https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples:-patterns) that would match it,
# Update Lorem based on your content (requires `ge` and `w` from https://github.com/jsoref/spelling; and `review` from https://github.com/check-spelling/check-spelling/wiki/Looking-for-items-locally )
# Update Lorem based on your content (requires `ge` and `w` from https://github.com/jsoref/spelling; and `review` from https://github.com/check-spelling/check-spelling/wiki/Looking-for-items-locally )
@@ -14,7 +14,7 @@ The point of doing all this work in public is to ensure that we are holding ours
The team triages new issues several times a week. During triage, the team uses labels to categorize, manage, and drive the project workflow.
We employ [a bot engine](https://github.com/microsoft/terminal/blob/main/doc/bot.md) to help us automate common processes within our workflow.
We employ [a bot engine](./doc/bot.md) to help us automate common processes within our workflow.
We drive the bot by tagging issues with specific labels which cause the bot engine to close issues, merge branches, etc. This bot engine helps us keep the repo clean by automating the process of notifying appropriate parties if/when information/follow-up is needed, and closing stale issues/PRs after reminders have remained unanswered for several days.
> * You may need to install the [VC++ v14 Desktop Framework Package](https://docs.microsoft.com/troubleshoot/cpp/c-runtime-packages-desktop-bridge#how-to-install-and-update-desktop-framework-packages).
@@ -72,8 +72,8 @@ package:
wingetinstall--idMicrosoft.WindowsTerminal-e
```
> **Note**\
> Due to [a dependency issue](https://github.com/microsoft/terminal/issues/15663), Terminal's current versions cannot be installed via the Windows Package Manager CLI. To install the stable release 1.17 or later, or the Preview release 1.18 or later, please use an alternative installation method.
> [!NOTE]
> Dependency support is available in WinGet version [1.6.2631 or later](https://github.com/microsoft/winget-cli/releases). To install the Terminal stable release 1.18 or later, please make sure you have the updated version of the WinGet client.
#### Via Chocolatey (unofficial)
@@ -118,9 +118,31 @@ repository.
---
## Installing Windows Terminal Canary
Windows Terminal Canary is a nightly build of Windows Terminal. This build has the latest code from our `main` branch, giving you an opportunity to try features before they make it to Windows Terminal Preview.
Windows Terminal Canary is our least stable offering, so you may discover bugs before we have had a chance to find them.
Windows Terminal Canary is available as an App Installer distribution and a Portable ZIP distribution.
The App Installer distribution supports automatic updates. Due to platform limitations, this installer only works on Windows 11.
The Portable ZIP distribution is a portable application. It will not automatically update and will not automatically check for updates. This portable ZIP distribution works on Windows 10 (19041+) and Windows 11.
`.../console/published/wincon.w` in the OS repo when you submit the PR.
The branch won't build without it.
* For now, you can update winconp.h with your consumable changes.
* Define registry name (ex `CONSOLE_REGISTRY_CURSORCOLOR`)
* Add the setting to `CONSOLE_STATE_INFO`
* Define registry name (ex:`CONSOLE_REGISTRY_CURSORCOLOR`)
* Add the setting to `CONSOLE_STATE_INFO`.
* Define the property key ID and the property key itself.
- Yes, the large majority of the `DEFINE_PROPERTYKEY` defs are the same, it's only the last byte of the guid that changes
- Yes, the large majority of the `DEFINE_PROPERTYKEY` defs are the same, it's only the last byte of the guid that changes.
2. Add matching fields to Settings.hpp
- Add getters, setters, the whole drill.
@@ -17,9 +17,9 @@
- We need to add it to *reading and writing* the registry from the propsheet, and *reading* the link from the propsheet. Yes, that's weird, but the propsheet is smart enough to re-use ShortcutSerialization::s_SetLinkValues, but not smart enough to do the same with RegistrySerialization.
-`src/propsheet/registry.cpp`
-`propsheet/registry.cpp@InitRegistryValues` should initialize the default value for the property.
-`propsheet/registry.cpp@GetRegistryValues` should make sure to read the property from the registry
-`propsheet/registry.cpp@GetRegistryValues` should make sure to read the property from the registry.
4. Add the field to the propslib registry map
4. Add the field to the propslib registry map.
5. Add the value to `ShortcutSerialization.cpp`
- Read the value in `ShortcutSerialization::s_PopulateV2Properties`
@@ -30,11 +30,11 @@ Now, your new setting should be stored just like all the other properties.
7. Update the feature test properties to get add the setting as well
-`ft_uia/Common/NativeMethods.cs@WinConP`:
-`Wtypes.PROPERTYKEY PKEY_Console_`
-`NT_CONSOLE_PROPS`
-`Wtypes.PROPERTYKEY PKEY_Console_`.
-`NT_CONSOLE_PROPS`.
8. Add the default value for the setting to `win32k-settings.man`
- If the setting shouldn't default to 0 or `nullptr`, then you'll need to set the default value of the setting in `win32k-settings.man`.
9. Update `Settings::InitFromStateInfo` and `Settings::CreateConsoleStateInfo` to get/set the value in a CONSOLE_STATE_INFO appropriately
9. Update `Settings::InitFromStateInfo` and `Settings::CreateConsoleStateInfo` to get/set the value in a CONSOLE_STATE_INFO appropriately.
@@ -27,6 +27,8 @@ I would highly recommend that Gulp convert to using PowerShell scripts and that
Original Source: https://github.com/microsoft/terminal/issues/217#issuecomment-404240443
_Addendum_: cmd.exe is the literal embodiment of [xkcd#1172]([url](https://xkcd.com/1172/)). Every change, no matter how small, will break _someone_.
## <a name="screenPerf"></a>Why is typing-to-screen performance better than every other app?
I really do not mind when someone comes by and decides to tell us that we're doing a good job at something. We hear so many complaints every day that a post like this is a breath of fresh air. Thanks for your thanks!
@@ -49,7 +51,7 @@ Will this UI enhancement come to other apps on Windows? Almost certainly not. Th
Will we try to keep it from regressing? Yes! Right now it's sort of a manual process. We identify that something is getting slow and then we go haul out [WPR](https://docs.microsoft.com/en-us/windows-hardware/test/wpt/windows-performance-recorder) and start taking traces. We stare down the hot paths and try to reason out what is going on and then improve them. For instance, in the last cycle or two, we focused on heap allocations as a major area where we could improve our end-to-end performance, changing a ton of our code to use stack-constructed iterator-like facades over the underlying request buffer instead of translating and allocating it into a new heap space for each level of processing.
As an aside, @bitcrazed wants us to automate performance tests in some conhost specific way, but I haven't quite figured out a controlled environment to do this in yet. The Windows Engineering System runs performance tests each night that give us a coarsegrained way of knowing if we messed something up for the whole operating system, and they technically offer a finegrained way for us to insert our own performance tests... but I just haven't got around to that yet. If you have an idea for a way for us to do this in an automated fashion, I'm all ears.
As an aside, @bitcrazed wants us to automate performance tests in some conhost specific way, but I haven't quite figured out a controlled environment to do this in yet. The Windows Engineering System runs performance tests each night that give us a coarse-grained way of knowing if we messed something up for the whole operating system, and they technically offer a fine-grained way for us to insert our own performance tests... but I just haven't got around to that yet. If you have an idea for a way for us to do this in an automated fashion, I'm all ears.
If there's anything else you'd like to know, let me know. I could go on all day. I deleted like 15 tangents from this reply before posting it....
*`/src/cascadia/TerminalApp` - This DLL represents the implementation of the Windows Terminal application. This includes parsing settings, hosting tabs & panes with Terminals in them, and displaying other UI elements. This DLL is almost entirely UWP-like code, and shouldn't be doing any Win32-like UI work.
*`/src/cascadia/WindowsTerminal` - This EXE provides Win32 hosting for the TerminalApp. It will set up XAML islands, and is responsible for drawing the window, either as a standard window or with content in the titlebar (non-client area).
*`/src/cascadia/CascadiaPackage` - This is a project for packaging the Windows Terminal and its dependencies into an .appx/.msix for deploying to the machine.
*`/src/cascadia/PublicTerminalCore` - This is a DLL wrapper for the TerminalCore and Renderer, similar to `TermControl`, which exposes some exported functions that so the Terminal can be used from C#.
*`/src/cascadia/WpfTerminalControl` - A DLL implementing a WPF version of the Terminal Control.
*`/src/host`– The meat of the windows console host. This includes buffer, input, output, windowing, server management, clipboard, and most interactions with the console host window that aren’t stated anywhere else. We’re trying to pull things out that are reusable into other libraries, but it’s a work in progress
*`/src/host/lib`– Builds the reusable LIB copy of the host
@@ -126,8 +125,6 @@
* Private calls into the Windows Window Manager to perform privileged actions related to the console process (working to eliminate) or for High DPI stuff (also working to eliminate)
*`Userprivapi.cpp`
*`Windowdpiapi.cpp`
* New UTF8 state machine in progress to improve Bash (and other apps) support for UTF-8 in console
*`Utf8ToWideCharParser.cpp`
* Window resizing/layout/management/window messaging loops and all that other stuff that has us interact with Windows to create a visual display surface and control the user interaction entry point
"description":"Use to set a path to a pixel shader to use with the Terminal when unfocused. Overrides `experimental.retroTerminalEffect`. This is an experimental feature, and its continued existence is not guaranteed.",
"type":"string"
},
"useAcrylic":{
"description":"When set to true, the window will have an acrylic material background when unfocused. When set to false, the window will have a plain, untextured background when unfocused.",
"type":"boolean"
},
"opacity":{
"default":100,
"description":"Sets the opacity of the window for the profile when unfocused. Accepts values from 0-100.",
"maximum":100,
"minimum":0,
"type":"number"
}
},
"type":"object"
@@ -374,8 +390,12 @@
},
"ShortcutActionName":{
"enum":[
"addMark",
"adjustFontSize",
"adjustOpacity",
"clearAllMarks",
"clearBuffer",
"clearMark",
"closeOtherPanes",
"closeOtherTabs",
"closePane",
@@ -386,6 +406,7 @@
"copy",
"duplicateTab",
"expandSelectionToWord",
"experimental.colorSelection",
"exportBuffer",
"find",
"findMatch",
@@ -393,64 +414,60 @@
"globalSummon",
"identifyWindow",
"identifyWindows",
"markMode",
"moveFocus",
"movePane",
"swapPane",
"markMode",
"moveTab",
"multipleActions",
"newTab",
"newWindow",
"nextTab",
"openAbout",
"openNewTabDropdown",
"openSettings",
"openSystemMenu",
"openTabColorPicker",
"openTabRenamer",
"openWindowRenamer",
"paste",
"prevTab",
"renameTab",
"openSystemMenu",
"openTabRenamer",
"quakeMode",
"quit",
"renameTab",
"renameWindow",
"resetFontSize",
"resizePane",
"renameWindow",
"restoreLastClosed",
"scrollDown",
"scrollDownPage",
"scrollToBottom",
"scrollToMark",
"scrollToTop",
"scrollUp",
"scrollUpPage",
"scrollToBottom",
"scrollToTop",
"searchWeb",
"selectAll",
"sendInput",
"setColorScheme",
"setFocusMode",
"setFullScreen",
"setMaximized",
"setTabColor",
"showSuggestions",
"splitPane",
"swapPane",
"switchSelectionEndpoint",
"switchToTab",
"tabSearch",
"toggleAlwaysOnTop",
"toggleBlockSelection",
"toggleFocusMode",
"selectAll",
"setFocusMode",
"switchSelectionEndpoint",
"toggleFullscreen",
"setFullScreen",
"setMaximized",
"togglePaneZoom",
"toggleSplitOrientation",
"toggleReadOnlyMode",
"toggleShaderEffects",
"toggleSplitOrientation",
"wt",
"quit",
"adjustOpacity",
"restoreLastClosed",
"addMark",
"scrollToMark",
"clearMark",
"clearAllMarks",
"searchWeb",
"experimental.colorSelection",
"unbound"
],
"type":"string"
@@ -640,6 +657,7 @@
"$ref":"#/$defs/NewTabMenuEntry"
},
{
"type":"object",
"properties":{
"type":{
"type":"string",
@@ -668,7 +686,7 @@
},
"allowEmpty":{
"description":"Whether to render a folder without entries, or to hide it",
"default":"false",
"default":false,
"type":"boolean"
}
}
@@ -682,6 +700,7 @@
"$ref":"#/$defs/NewTabMenuEntry"
},
{
"type":"object",
"properties":{
"type":{
"type":"string",
@@ -698,6 +717,7 @@
"$ref":"#/$defs/NewTabMenuEntry"
},
{
"type":"object",
"properties":{
"type":{
"type":"string",
@@ -719,6 +739,7 @@
"$ref":"#/$defs/NewTabMenuEntry"
},
{
"type":"object",
"properties":{
"type":{
"type":"string",
@@ -735,6 +756,7 @@
"$ref":"#/$defs/NewTabMenuEntry"
},
{
"type":"object",
"properties":{
"type":{
"type":"string",
@@ -775,6 +797,7 @@
]
},
"ShortcutAction":{
"type":"object",
"properties":{
"action":{
"description":"The action to execute",
@@ -783,8 +806,7 @@
},
"required":[
"action"
],
"type":"object"
]
},
"AdjustFontSizeAction":{
"description":"Arguments corresponding to an Adjust Font Size Action",
@@ -793,6 +815,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -817,6 +840,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -858,6 +882,7 @@
"$ref":"#/$defs/NewTerminalArgs"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -874,6 +899,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -898,6 +924,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -922,6 +949,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -946,6 +974,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -970,6 +999,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -994,6 +1024,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1018,6 +1049,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1045,6 +1077,7 @@
"$ref":"#/$defs/NewTerminalArgs"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1077,6 +1110,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1114,6 +1148,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1141,6 +1176,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1162,6 +1198,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1183,6 +1220,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1207,6 +1245,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1227,6 +1266,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1247,6 +1287,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1267,6 +1308,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1291,6 +1333,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1319,6 +1362,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1347,6 +1391,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1375,6 +1420,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1399,6 +1445,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1423,6 +1470,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1446,6 +1494,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1471,6 +1520,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1492,6 +1542,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1519,6 +1570,7 @@
"$ref":"#/$defs/NewTerminalArgs"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1535,6 +1587,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1556,6 +1609,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1577,6 +1631,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1598,6 +1653,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1619,6 +1675,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1641,6 +1698,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1662,6 +1720,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1683,6 +1742,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1734,6 +1794,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1750,6 +1811,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1773,6 +1835,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1801,6 +1864,7 @@
"$ref":"#/$defs/ShortcutAction"
},
{
"type":"object",
"properties":{
"action":{
"type":"string",
@@ -1887,7 +1951,11 @@
"properties":{
"applicationTheme":{
"description":"Which UI theme the Terminal should use for controls",
"enum":["light","dark","system"],
"enum":[
"light",
"dark",
"system"
],
"type":"string"
},
"useMica":{
@@ -1918,7 +1986,11 @@
"type":"string",
"description":"The name of the theme. This will be displayed in the settings UI.",
"not":{
"enum":["light","dark","system"]
"enum":[
"light",
"dark",
"system"
]
}
},
"tab":{
@@ -1935,6 +2007,7 @@
"ThemePair":{
"additionalProperties":false,
"description":"A pair of Theme names, to allow the Terminal to switch theme based on the OS theme",
"type":"object",
"properties":{
"light":{
"type":"string",
@@ -2116,16 +2189,16 @@
},
"name":{
"description":"The name that will appear in the command palette. If one isn't provided, the terminal will attempt to automatically generate a name.\nIf name is a string, it will be the name of the command.\nIf name is a object, the key property of the object will be used to lookup a localized string resource for the command",
"properties":{
"key":{
"type":"string"
}
},
"type":[
"string",
"object",
"null"
]
],
"properties":{
"key":{
"type":"string"
}
}
},
"iterateOn":{
"type":"string",
@@ -2162,6 +2235,7 @@
"Globals":{
"additionalProperties":true,
"description":"Properties that affect the entire window, regardless of the profile settings.",
"type":"object",
"properties":{
"alwaysOnTop":{
"default":false,
@@ -2173,6 +2247,11 @@
"description":"When set to true, tabs are always displayed. When set to false and \"showTabsInTitlebar\" is set to false, tabs only appear after opening a new tab.",
"type":"boolean"
},
"compatibility.enableUnfocusedAcrylic":{
"default":true,
"description":"When set to true, unfocused windows can have acrylic instead of opaque.",
"type":"boolean"
},
"centerOnLaunch":{
"default":false,
"description":"When set to `true`, the terminal window will auto-center itself on the display it opens on. The terminal will use the \"initialPosition\" to determine which display to open on.",
@@ -2270,11 +2349,6 @@
"description":"When set to true, the background image for the currently focused profile is expanded to encompass the entire window, beneath other panes.",
"type":"boolean"
},
"compatibility.reloadEnvironmentVariables":{
"default":true,
"description":"When set to true, when opening a new tab or pane it will get reloaded environment variables.",
"type":"boolean"
},
"initialCols":{
"default":120,
"description":"The number of columns displayed in the window upon first load. If \"launchMode\" is set to \"maximized\" (or \"maximizedFocus\"), this property is ignored.",
@@ -2376,10 +2450,17 @@
"theme":{
"default":"dark",
"description":"Sets the theme of the application. This value should be the name of one of the themes defined in `themes`. The Terminal also includes the themes `dark`, `light`, and `system`.",
"oneOf":[
"anyOf":[
{
"type":"string"
},
{
"enum":[
"dark",
"light",
"system"
]
},
{
"$ref":"#/$defs/ThemePair"
}
@@ -2489,12 +2570,12 @@
},
"required":[
"defaultProfile"
],
"type":"object"
]
},
"Profile":{
"description":"Properties specific to a unique profile.",
"additionalProperties":false,
"type":"object",
"properties":{
"acrylicOpacity":{
"default":0.5,
@@ -2523,6 +2604,11 @@
"null"
]
},
"compatibility.reloadEnvironmentVariables":{
"default":true,
"description":"When set to true, when opening a new tab or pane it will get reloaded environment variables.",
"type":"boolean"
},
"unfocusedAppearance":{
"$ref":"#/$defs/AppearanceConfig",
"description":"Sets the appearance of the terminal when it is unfocused.",
@@ -2541,7 +2627,7 @@
},
"backgroundImage":{
"description":"Sets the file location of the image to draw over the window background.",
"oneOf":[
"anyOf":[
{
"type":[
"string",
@@ -2553,10 +2639,6 @@
"desktopWallpaper"
]
}
],
"type":[
"string",
"null"
]
},
"backgroundImageAlignment":{
@@ -2700,15 +2782,15 @@
"description":"When set to true, marks added to the buffer via the addMark action will appear on the scrollbar.",
"type":"boolean"
},
"experimental.repositionCursorWithMouse":{
"default":false,
"description":"When set to true, you can move the text cursor by clicking with the mouse on the current commandline. This is an experimental feature - there are lots of edge cases where this will not work as expected.",
"type":"boolean"
},
"experimental.pixelShaderPath":{
"description":"Use to set a path to a pixel shader to use with the Terminal. Overrides `experimental.retroTerminalEffect`. This is an experimental feature, and its continued existence is not guaranteed.",
"type":"string"
},
"useAtlasEngine":{
"description":"Windows Terminal 1.16 and later ship with a new, performant text renderer. Set this to false to revert back to the old text renderer.",
"type":"boolean",
"default":true
},
"fontFace":{
"default":"Cascadia Mono",
"description":"[deprecated] Define 'face' within the 'font' object instead.",
@@ -2893,8 +2975,7 @@
"description":"When set to true, the window will have an acrylic material background. When set to false, the window will have a plain, untextured background.",
"type":"boolean"
}
},
"type":"object"
}
},
"ProfileList":{
"description":"A list of profiles and the properties specific to each.",
@@ -2909,6 +2990,7 @@
},
"ProfilesObject":{
"description":"A list of profiles and default settings that apply to all of them",
"type":"object",
"properties":{
"list":{
"$ref":"#/$defs/ProfileList"
@@ -2917,12 +2999,12 @@
"description":"The default settings that apply to every profile.",
"$ref":"#/$defs/Profile"
}
},
"type":"object"
}
},
"SchemeList":{
"description":"Properties are specific to each color scheme. ColorTool is a great tool you can use to create and explore new color schemes. All colors use hex color format.",
"items":{
"type":"object",
"additionalProperties":false,
"properties":{
"name":{
@@ -3011,8 +3093,7 @@
"$ref":"#/$defs/Color",
"description":"Sets the color used as ANSI yellow."
> This document has been superseded by the [Terminal 2023 Roadmap]. Please refer to that document for the updated roadmap.
## Overview
This document outlines the roadmap of features we're planning for the Windows Terminal during 2022. This serves as a successor to the [Terminal v2 Roadmap], to reflect changes to our planning going forward.
@@ -111,7 +115,7 @@ Incoming issues/asks/etc. are triaged several times a week, labeled appropriatel
This document outlines the roadmap of features we're planning for the Windows Terminal during 2023. This serves as a successor to the [2022 Roadmap], to reflect changes to our planning going forward.
## Release cadence
We've settled on a roughly quarterly release cycle - about once every three months. In May we released [Terminal 1.18]. We're targeting 1.19 for sometime in late September, and 1.20 likely in early January 2024. (These timelines are rough estimates, not strict rules. For example, 1.18's release was pushed back slightly to better align with Build 2023.)
New features will go into [Windows Terminal Preview](https://aka.ms/terminal-preview) first. Typically, one release after they've been in Preview, those features will move into [Windows Terminal](https://aka.ms/terminal) ("Terminal Stable"). In the case of some more risky or experimental features, we might hold them to only Preview builds for an extended period[^1].
| Quarter | Date | Release Version | Preview Release Blog Post |
Within a single milestone, we typically reserve the last month as "bake time", to polish off bugfixes and get the release ready to ship. In this last month, we'll likely slow down our ingestion of community PRs just to stabilize what's already in `main`. For example, a given release might look like:
```mermaid
gantt
title Proposed Terminal Releases 1.14-1.18
dateFormat YYYY-MM-DD
axisFormat %d %b
section Terminal 1.18
Lock down & bake :l18, 2023-05-09 , 2w
Release 1.18 :milestone, 2023-05-23, 0
1.18 becomes Stable :milestone, after l19, 0
section Terminal 1.19
Features :f19, after l18, 10w
Bugfix :b19, after f19 , 4w
Lock down & bake :l19, after b19 , 2w
Release 1.19 :milestone, after l19, 0
```
_informative, not normative_
## Up next in the Terminal
### Terminal 1.19
* Canary builds. Nightly builds of the Terminal from `main`. More unstable, but quicker access to experimental features.
* Terminal AI. While this will only be shipping in Canary builds to begin with, the v0 implementation will be available roughly at the same time as 1.19.
* The Suggestions UI. This is the starting point for shell completions [#3121], tasks [#1595], and probably Terminal AI at some point too.
* Unicode input for `cmd.exe` (and any other console app using "cooked reads"). See [#15567]
* Miscellaneous performance improvements. Conhost should be a _lot_ faster now.
* Broadcast input mode, for sending text to multiple panes at once.
## Team member "north stars"
For a more fluid take on what each of the team's personal goals are, head on over to [Core team North Stars]. This has a list of more long-term goals that each team member is working towards, but not things that are necessarily committed work.
[^1]: A conclusive list of these features can be found at [../src/features.xml](../src/features.xml). Note that this is a raw XML doc used to light up specific parts of the codebase, and not something authored for human consumption.
* Give me suggestions from my recent commands, using what I've typed
* Give me suggestions of directories I've recently been in
* _(After [Tasks] are implemented)_ Give me suggestions from recent commands,
commands I've saved, and commands for this project. Don't nest any, so they're
all in the top-level menu. Use what I've typed already to start filtering.
* Just open the Suggestions UI with all suggestions sources, and group them by
the source of the suggestions.
This should cover most of the basic use cases for suggestions.
#### Who owns this menu?
There was some discussion of who should own the suggestions menu. The control
itself? Or the app hosting the control?
A main argument for hosting this UI in the control itself is that any consumer
of the `TermControl` should be able to display the [shell-driven autocompletion]
menu. And they should get the UI from us "for free". Consumers shouldn't need to
reimplement it themselves. This probably could be done without many changes:
* Instead of operating on `Command`s and actions from the terminal settings,
the control could just know that all the entries in the menu are "send
input" "actions".
* The control could offer a method to manually invoke the Suggestions UI for a
list of {suggestion, name, description} objects.
* The app layer could easily translate between sendInput actions and these
pseudo-actions.
A big argument in favor of having the app layer host the control: Consider an
app like Visual Studio. When they embed the control, they'll want to style the
shell-completions UI in their own way. They already have their own intellisense
menu, and their own UI paradigm.
For now, we'll leave this as something that's owned by the app layer. When we
get around to finalizing the [shell-driven autocompletion] design, we can
iterate on ideas for supporting both consumers that want to use a pre-built
suggestions control, or consumers who want to bring their own.
## Tenets
<table>
<tr><td><strong>Compatibility</strong></td><td>
This shouldn't break any existing flows. This is a general purpose UI element,
to be extended in a variety of ways. Those customizations will all be opt-in by
the user, so I'm not expecting any breaking compatibility changes here.
</td></tr>
<tr><td><strong>Accessibility</strong></td><td>
The Suggestions UI was designed with the goal of making commandline shell
suggestions _more_ accessible. As Carlos previously wrote:
> Screen readers struggle with this because the entire menu is redrawn every time, making it harder to understand what exactly is "selected" (as the concept of selection in this instance is a shell-side concept represented by visual manipulation).
>
> ...
>
> _\[Shell driven suggestions\]_ can then be leveraged by Windows Terminal to create UI elements. Doing so leverages WinUI's accessible design.
This will allow the Terminal to provide more context-relevant information to
screen readers.
</td></tr>
<tr><td><strong>Sustainability</strong></td><td>
No sustainability changes expected.
</td></tr>
<tr><td><strong>Localization</strong></td><td>
The localization needs of the Suggestions UI will be effectively the same as the
needs of the Command Palette.
The Terminal will have no way to localize suggestions that are provided via
[shell-driven autocompletion]. These are just verbatim strings that the shell
told us to use. We don't consider this to be something to worry about, however.
This is no different than the fact that Terminal cannot localize the `Get-Help`
(or any other) output of PowerShell.
</td></tr>
</table>
## Implementation Plan
This is more of an informative outline, rather than a normative one. Many of the
things from Crawl, Walk, and Run are all already in PRs as of the time of this
spec's review.
### 🐣 Crawl
* [ ] Fork the Command palette to a new UI element, the `SuggestionsControl`
* [ ] Enable previewing `sendInput` actions in the Command Palette and `SuggestionsControl`
* [ ] Enable the `SuggestionsControl` to open top-down (aligned to the bottom of the cursor row) or bottom-up (aligned to the top of the cursor row).
* [ ] Disable sorting on the `SuggestionsControl` - elements should presumably be pre-sorted by the source.
* [ ] Expose the recent commands as a accessor on `TermControl`
* [ ] Add a `suggestions` action which accepts a single option `recentCommands`. These should be fed in MRU order to the `SuggestionsControl`.
* [ ] Expose the recent directories as an accessor on `TermControl`, and add a `recentDirectories` source.
### 🚶 Walk
* [ ] Add a `tasks` source to `suggestions` which opens the Suggestions UI with
a tree of all `sendInput` commands
* [ ] Enable the `SuggestionsControl` to open with or without a search box
* [ ] Plumb support for shell-driven completions through the core up to the app
* [ ] Expose the _current_ commandline from the `TermControl`
* [ ] Add a `useCommandline` property to `suggestions`, to pre-populate the search with the current commandline.
* [ ] Add a `TeachingTip` (or similar) to the Suggestions UI to display
descriptions (when available)
* [ ] Use the `ToolTip` property of shell-driven suggestions as the description
* [ ] Add a boolean `nesting` property which can be used to disable nesting on the `tasks` source.
* [ ] Add the ability for `nesting` to accept `enabled`/`disabled` as `true`/`false` equivalents
* [ ] Add the ability for `nesting` to accept `source`, which instead groups all
commands to the Suggestions UI by the source of that suggestion.
### 🚀 Sprint
The two "sprint" tasks here are much more ambitious than the other listed
scenarios, so breaking them down to atomic tasks sees less reasonable. We'd have
to spend a considerable amount more time figuring out _how_ to do each of these
first.
For example - extensions. We have yet to fully realize what extensions _are_.
Determining how extensions will provide suggestions is left as something we'll
need to do as a part of the Extensions spec.
## Conclusion
Here's a sample json schema for the settings discussed here.
```json
"OpenSuggestionsAction":{
"description":"Arguments corresponding to a Open Suggestions Action",
"allOf":[
{
"$ref":"#/$defs/ShortcutAction"
},
{
"properties":{
"action":{
"type":"string",
"const":"suggestions"
},
"source":{
"$ref":"#/$defs/SuggestionSource",
"description":"Which suggestion sources to filter."
},
"useCommandline":{
"default":false,
"description":"When set to `true`, the current commandline the user has typed will pre-populate the filter of the Suggestions UI. This requires that the user has enabled shell integration in their shell's config. When set to false, the filter will start empty."
},
"nesting":{
"default":true,
"description":"When set to `true`, suggestions will follow the provided nesting structure. For Tasks, these will follow the structure of the Command Palette. When set to `false`, no nesting will be used (and all suggestions will be in the top-level menu.",
"$comment":"This setting is a possible follow-up setting, not required for v1. "
}
}
}
]
},
"BuiltinSuggestionSource":{
"enum":[
"commandHistory",
"directoryHistory",
"tasks",
"local",
"all"
],
"type":"string"
},
"SuggestionSource":{
"default":"all",
"description":"Either a single suggestion source, or an array of sources to concatenate. Built-in sources include `commandHistory`, `directoryHistory`, `tasks`, and `local`. Extensions may provide additional values. The special value `all` indicates all suggestion sources should be included",
"$comment":"`tasks` and `local` are sources that would be added by the Tasks feature, as a follow-up"
> I'm using `Docker` as an example fragment extension that provides
> some `docker` commands. When grouping by `"source"`, we could pull those into
> a separate top-level entry. When not grouping by `"source"`, those would still
> show up with the rest of `tasks`. )
#### Store recent commands across sessions
> **Note**
> _I'm not sure we really want to put this in this spec or not, hence
> why it is in the "Future considerations" section. I think it is worth
> mentioning. This might be better served in the [shell integration] doc._
We'll probably want a way for recent commands to be saved across sessions. That way, your `cmd.exe` command history could persist across sessions. We'd need:
* A setting to enable this behavior
* A setting to control the context of these saved commandlines.
* Do we want them saved per-profile, or globally?
* If they're saved per-profile, maybe a profile can opt-in to loading all the commands?
* How does defterm play with this? Do we "layer" by concatenating per-profile commands with `profiles.defaults` ones?
* A button in the Settings UI for clearing these commands
* Should fragments be able to pre-populate "recent commands"?
* I'm just gonna say _no_. That would be a better idea for Tasks (aka just a `sendInput` Action that we load from the fragment normally as a Task), or a specific suggestion source for the fragment extension.
#### Inline mode
> **Note**
> _This is a half-baked idea with some potential. However, I don't
> think it needs to be a part of the v1 of the Suggestions UI, so I'm leaving it
> under future considerations for a future revision._
Do we want to have a suggestions UI "mode", that's just **one** inline
suggestion, "no" UI? Some UX ala the `PsReadline` recent command suggestion
feature. Imagine, we just display the IME ghost text thing for the first result,
[The Old New Thing: How can I launch an unelevated process from my elevated process, redux]: https://devblogs.microsoft.com/oldnewthing/20190425-00/?p=102443
[Process Model 2.0 Spec]: https://github.com/microsoft/terminal/blob/main/doc/specs/%235000%20-%20Process%20Model%202.0/%235000%20-%20Process%20Model%202.0.md
[Process Model 2.0 Spec]: ./doc/specs/%235000%20-%20Process%20Model%202.0/%235000%20-%20Process%20Model%202.0.md
[Process Model 2.0 Spec]: https://github.com/microsoft/terminal/blob/main/doc/specs/%235000%20-%20Process%20Model%202.0/%235000%20-%20Process%20Model%202.0.md
[Process Model 2.0 Spec]: ../%235000%20-%20Process%20Model%202.0/%235000%20-%20Process%20Model%202.0.md
@@ -23,7 +23,7 @@ contexts without needing to replicate an entire json blob.
This spec was largely inspired by the following diagram from @DHowett:
The goal is to introduce an `id` parameter by which actions could be uniquely
referred to. If we'd ever like to use an action outside the list of `actions`, we
@@ -36,38 +36,54 @@ Discussion with the team lead to the understanding that the name `actions` would
be even better, as a way of making the meaning of the "list of actions" more
obvious.
When we're parsing `actions`, we'll make three passes:
*The first pass will scan the list for objects with an `id` property. We'll
We will then restructure `defaults.json`, and also users' settings files (via `fixUpUserSettings`), in the following manner:
*Instead of each `command` json block containing both the `action` (along with additional arguments) and the `keys`, these will now be split up -
* There will now be one json block for just the `command`/`action`, which will also contain the `id`. These json blocks will be in their own list called `actions`.
* There will be another json block for the `keys`, which will refer to the action to be invoked by `id`. These json blocks will be in their own list called `keybindings`.
For example, let's take a look at the `split pane right` action in `defaults.json` as we currently have it:
Here is how we will parse settings file and construct the relevant settings model objects:
* We will first scan the `actions` list. We'll
attempt to parse those entries into `ActionAndArgs` which we'll store in the
global `id->ActionAndArgs` map. If any entry doesn't have an`id` set, we'll
skip it in this phase. If an entry doesn't have a `command` set, we'll ignore
it in this pass.
* The second pass will scan for _keybindings_. Any entries with `keys` set will
create a `KeyChord->ActionAndArgs` entry in the keybindings map. If the entry
has an `id` set, then we'll simply re-use the action we've already parsed for
the `id`, from the action map. If there isn't an `id`, then we'll parse the
action manually at this time. Entries without a `keys` set will be ignored in
this pass.
* The final pass will be to generate _commands_. Similar to the keybindings
pass, we'll attempt to lookup actions for entries with an `id` set. If there
isn't an `id`, then we'll parse the action manually at this time. We'll then
get the name for the entry, either from the `name` property if it's set, or
the action's `GenerateName` method.
global `id->ActionAndArgs` map. All actions defined in `defaults.json` must have an `id` specified, and all actions provided by fragments must also have `id`s. Any actions from the defaults or fragments that do not provide `id`s will be ignored. As for user-specified commands, if no`id` is set, we will auto-generate one for that command based on the action and any additional arguments. For example, the `split pane right` command above might result in an autogenerated id `User.SplitPaneRight`.
* Note: this step is also where we will generate _commands_. We will use the name provided in the entry if it's set or the action's `GenerateName` method.
* Next we will scan the `keybindings` list. These entries will
create a `KeyChord->ActionAndArgs` entry in the keybindings map. Since these objects should all contain an `id`, we will simply use the `id->ActionAndArgs` map we created in the previous step. Any object with `keys` set but no `id` will be ignored.
For a visual representation, let's assume the user has the following in their
For this case, having an `id` on the top level could potentially make sense when it comes to using that `id` in a menu, but not in the case of using that `id` for a keybinding. For the initial implementation, we will not support an `id` for these types of actions, which leaves us open to revisiting this in the future.
### Layering
When layering `actions`, if a later settings file contains an action with the
same `id`, it will replace the current value. In this way, users can redefine
@@ -87,6 +103,9 @@ As we add additional menus to the Terminal, like the customization for the new
tab dropdown, or the tab context menu, or the `TermControl` context menu, they
could all refer to these actions by `id`, rather than duplicating the same json.
As for fragments, all actions in fragments _must_ have an `id`. If a fragment provides an action without an `id`, or provides an `id` that clashes with one of the actions in `defaults.json`, that action will be ignored.
> 👉 NOTE: This will mean that actions will now need an `OriginTag`, similar to profiles and color schemes
| `ALLOC_CONSOLE_MODE_DEFAULT` | Allocate a console session if (and how) one was requested by the parent process. |
| `ALLOC_CONSOLE_MODE_NEW_WINDOW` | Allocate a console session with a window, even if this process was created with `CREATE_NO_CONSOLE` or `DETACHED_PROCESS`. |
| `ALLOC_CONSOLE_MODE_NO_WINDOW` | Allocate a console session _without_ a window, even if this process was created with `CREATE_NEW_WINDOW` or `DETACHED_PROCESS` |
###### Notes
Applications seeking backwards compatibility are encouraged to delay-load `AllocConsoleWithOptions` or check for its presence in
the `api-ms-win-core-console-l1` APISet.
## Inspiration
Fusion manifest entries are used to make application-scoped decisions like this all the time, like `longPathAware` and
`heapType`.
CUI applications that can spawn a UI (or GUI applications that can print to a console) are commonplace on other
platforms because there is no subsystem differentiation.
## UI/UX Design
There is no UI for this feature.
## Capabilities
### Accessibility
This should have no impact on accessibility.
### Security
One reviewer brought up the potential for a malicious actor to spawn an endless stream of headless daemon processes.
This proposal in no way changes the facilities available to malicious people for causing harm: they could have simply
used `IMAGE_SUBSYSTEM_WINDOWS_GUI` and not presented a UI--an option that has been available to them for 35 years.
### Reliability
This should have no impact on reliability.
### Compatibility
An existing application opting into **detached** may constitute a breaking change, but the scope of the breakage is
restricted to that application and is expected to be managed by the application.
All behavioral changes are opt-in.
> **EXAMPLE**: If Python updates python.exe to specify an allocation policy of **detached**, graphical python applications
> will become double-click runnable from the graphical shell without spawning a console window. _However_, console-based
> python applications will no longer spawn a console window when double-clicked from the graphical shell.
>
> In addition, if python.exe specifies **detached**, Console APIs will fail until a console is allocated.
Python could work around this by calling [`AllocConsole`] or [new API `AllocConsoleWithOptions`](#allocconsolewithoptions)
if it can be detected that console I/O is required.
#### Downlevel
On downlevel versions of Windows that do not understand (or expect) this manifest field, applications will allocate
consoles as specified by their image subsystem (described in the [abstract](#abstract) above).
### Performance, Power, and Efficiency
This should have no impact on performance, power or efficiency.
## Potential Issues
### Shell Hang
I am **not** proposing a change in how shells determine whether to wait for an application before returning to a prompt.
This means that a console subsystem application that intends to primarily present a UI but occasionally print text to a
console (therefore choosing the **detached** allocation policy) will cause the shell to "hang" and wait for it to
exit.
The decision to pause/wait is made entirely in the calling shell, and the console subsystem cannot influence that
decision.
Because the vast majority of shells on Windows "hang" by calling `WaitFor...Object` with a HANDLE to the spawned
process, an application that wants to be a "hybrid" CUI/GUI application will be forced to spawn a separate process to
detach from the shell and then terminate its main process.
This is very similar to the forking model seen in many POSIX-compliant operating systems.
### Launching interactively from Explorer, Task Scheduler, etc.
Applications like PowerShell may wish to retain automatic console allocation, and **detached** would be unsuitable for
them. If PowerShell specifies the `detached` console allocation policy, launching `pwsh.exe` from File Explorer it will
no longer spawn a console. This would almost certainly break PowerShell for all users.
Such applications can use `AllocConsole()` early in their startup.
At the same time, PowerShell wants `-WindowStyle Hidden` to suppress the console _before it's created_.
Applications in this category can use `AllocConsoleWithOptions()` to specify additional information about the new console window.
PowerShell, and any other shell that wishes to maintain interactive launch from the graphical shell, can start in
**detached** mode and then allocate a console as necessary. Therefore:
* PowerShell will set `<consoleAllocationPolicy>detached</consoleAllocationPolicy>`
* On startup, it will process its commandline arguments.
* If `-WindowStyle Hidden` is **not** present (the default case), it can:
* `AllocConsole()` or `AllocConsoleWithOptions(NULL)`
* Either of these APIs will present a console window (or not) based on the flags passed through `STARTUPINFO` during
[`CreateProcess`].
* If `-WindowStyle Hidden` is present, it can:
* `AllocConsoleWithOptions(&alloc)` where `alloc.mode` specifies `ALLOC_CONSOLE_MODE_HIDDEN`
## Future considerations
We're introducing a new manifest field today -- what if we want to introduce more? Should we have a `consoleSettings`
manifest block?
Are there other allocation policies we need to consider?
## Resources
### Rejected Solutions
- A new PE subsystem, `IMAGE_SUBSYSTEM_WINDOWS_HYBRID`
- it would behave like **inheritOnly**
- relies on shells to update and check for this
- checking a subsystem doesn't work right with app execution aliases[^3]
- This is not a new problem, but it digs the hole a little deeper.
- requires standardization outside of Microsoft because the PE format is a dependency of the UEFI specification[^4]
- requires coordination between tooling teams both within and without Microsoft (regarding any tool that operates on
or produces PE files)
- An exported symbol that shells can check for to determine whether to wait for the attached process to exit
- relies on shells to update and check for this
- cracking an executable to look for symbols is probably the last thing shells want to do
- we could provide an API to determine whether to wait or return?
- fragile, somewhat silly, exporting symbols from EXEs is annoying and uncommon
An earlier version of this specification offered the **always** allocation policy, with the following behaviors:
> **STRUCK FROM SPECIFICATION**
>
> * A GUI subsystem application would always get a console window.
> * A command-line shell would not wait for it to exit before returning a prompt.
It was cut because a GUI application that wants a console window can simply attach to an existing console session or
allocate a new one. We found no compelling use case that would require the forced allocation of a console session
outside of the application's code.
An earlier version of this specification offered the **inheritOnly** allocation policy, instead of the finer-grained
**hidden** and **detached** policies. We deemed it insufficient for PowerShell's use case because any application
launched by an **inheritOnly** PowerShell would immediately force the uncontrolled allocation of a console window.
> **STRUCK FROM SPECIFICATION**
>
> The move to **hidden** allows PowerShell to offer a fully-fledged console connection that can be itself inherited by a
> downstream application.
#### Additional allocation policies
An earlier revision of this specification suggested two allocation policies:
> **STRUCK FROM SPECIFICATION**
>
> **hidden** is intended to be used by console applications that want finer-grained control over the visibility of their
> console windows, but that still need a console host to service console APIs. This includes most scripting language
> interpreters.
>
> **detached** is intended to be used by primarily graphical applications that would like to operate against a console _if
> one is present_ but do not mind its absence. This includes any graphical tool with a `--help` or `/?` argument.
The `hidden` policy was rejected due to an incompatibility with modern console hosting, as `hidden` would require an
application to interact with the console window via `GetConsoleWindow()` and explicitly show it.
> **STRUCK FROM SPECIFICATION**
>
> ##### ShowWindow and ConPTY
>
> The pseudoconsole creates a hidden window to service `GetConsoleWindow()`, and it can be trivially shown using
> `ShowWindow`. If we recommend that applications `ShowWindow` on startup, we will need to guard the pseudoconsole's
> pseudo-window from being shown.
[^1]: [Powershell -WindowStyle Hidden still shows a window briefly]
[^2]: [StackOverflow: pythonw.exe or python.exe?]
[^3]: [PowerShell: Windows Store applications incorrectly assumed to be console applications]
[^4]: [UEFI spec 2.6 appendix Q.1]
[Powershell -WindowStyle Hidden still shows a window briefly]: https://github.com/PowerShell/PowerShell/issues/3028
[PowerShell: Windows Store applications incorrectly assumed to be console applications]: https://github.com/PowerShell/PowerShell/issues/9970
[StackOverflow: pythonw.exe or python.exe?]: https://stackoverflow.com/questions/9705982/pythonw-exe-or-python-exe
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
