From b3cc618af838685d01d2fadc9c58fa862ce75ea5 Mon Sep 17 00:00:00 2001 From: Dmitriy Fishman Date: Sat, 16 Oct 2021 01:46:26 +0300 Subject: [PATCH 01/18] doc: Fix a UKism in CONTRIBUTING.md (#11505) --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 580fc04ed..da6f3a476 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -125,7 +125,7 @@ Team members will be happy to help review specs and guide them to completion. ### Help Wanted -Once the team have approved an issue/spec, development can proceed. If no developers are immediately available, the spec can be parked ready for a developer to get started. Parked specs' issues will be labeled "Help Wanted". To find a list of development opportunities waiting for developer involvement, visit the Issues and filter on [the Help-Wanted label](https://github.com/microsoft/terminal/labels/Help%20Wanted). +Once the team has approved an issue/spec, development can proceed. If no developers are immediately available, the spec can be parked ready for a developer to get started. Parked specs' issues will be labeled "Help Wanted". To find a list of development opportunities waiting for developer involvement, visit the Issues and filter on [the Help-Wanted label](https://github.com/microsoft/terminal/labels/Help%20Wanted). --- From 1c8b71b6e7bfa7575bfc7520aacf9fb3d9a533b7 Mon Sep 17 00:00:00 2001 From: Dmitriy Fishman Date: Sat, 16 Oct 2021 01:46:49 +0300 Subject: [PATCH 02/18] Fix a typo in Niksa.md (#11506) --- doc/Niksa.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/Niksa.md b/doc/Niksa.md index e543191a6..c4c7e4798 100644 --- a/doc/Niksa.md +++ b/doc/Niksa.md @@ -189,7 +189,7 @@ I think there might be a bit of a misunderstanding here - there are two differen * shell applications, like `cmd.exe`, `powershell`, `zsh`, etc. These are text-only applications that emit streams of characters. They don't care at all about how they're eventually rendered to the user. These are also sometimes referred to as "commandline client" applications. * terminal applications, like the Windows Terminal, gnome-terminal, xterm, iterm2, hyper. These are graphical applications that can be used to render the output of commandline clients. -On Windows, if you just run `cmd.exe` directly, the OS will create an instance of `conhost.exe` as the _terminal_ for `cmd.exe`. The same thing happens for `powershell.exe`, the system will creates a new conhost window for any client that's not already connected to a terminal of some sort. This has lead to an enormous amount of confusion for people thinking that a conhost window is actually a "`cmd` window". `cmd` can't have a window, it's just a commandline application. Its window is always some other terminal. +On Windows, if you just run `cmd.exe` directly, the OS will create an instance of `conhost.exe` as the _terminal_ for `cmd.exe`. The same thing happens for `powershell.exe`, the system will create a new conhost window for any client that's not already connected to a terminal of some sort. This has lead to an enormous amount of confusion for people thinking that a conhost window is actually a "`cmd` window". `cmd` can't have a window, it's just a commandline application. Its window is always some other terminal. Any terminal can run any commandline client application. So you can use the Windows Terminal to run whatever shell you want. I use mine for both `cmd` and `powershell`, and also WSL: From 51c30119501e6fb7b8a29b891262e9c218651111 Mon Sep 17 00:00:00 2001 From: NotWearingPants <26556598+NotWearingPants@users.noreply.github.com> Date: Mon, 18 Oct 2021 19:42:08 +0300 Subject: [PATCH 03/18] Fix quoted boolean defaults in schema (#11517) ## Summary of the Pull Request The `settings.json` schema had `"default"`s for some boolean settings set as quoted strings (`"true"` / `"false"`), so I removed the quotes. ## PR Checklist * [ ] Closes #xxx * [x] CLA signed * [ ] Tests added/passed * [ ] Documentation updated * [x] Schema updated * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan ## Detailed Description of the Pull Request / Additional comments VSCode autocompletes the default value when you select the setting in intellisense, so it autocompleted a string which caused a schema error. Booleans should be JSON booleans, not quoted. ## Validation Steps Performed --- doc/cascadia/profiles.schema.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index f61e74f4e..49c8c4a46 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -459,7 +459,7 @@ }, "suppressApplicationTitle": { "type": "boolean", - "default": "false", + "default": false, "description": "When set to true, tabTitle overrides the default title of the tab and any title change messages from the application will be suppressed. When set to false, tabTitle behaves as normal" }, "colorScheme": { @@ -1575,22 +1575,22 @@ "deprecated": true }, "minimizeToNotificationArea": { - "default": "false", + "default": false, "description": "When set to true, minimizing a Terminal window will no longer appear in the taskbar. Instead, a Terminal icon will appear in the notification area through which the user can access their windows.", "type": "boolean" }, "alwaysShowNotificationIcon": { - "default": "false", + "default": false, "description": "When set to true, the Terminal's notification icon will always be shown in the notification area.", "type": "boolean" }, "showAdminShield": { - "default": "true", + "default": true, "description": "When set to true, the Terminal's tab row will display a shield icon when the Terminal is running with administrator privileges", "type": "boolean" }, "useAcrylicInTabRow": { - "default": "false", + "default": false, "description": "When set to true, the tab row will have an acrylic background with 50% opacity.", "type": "boolean" }, From 02dd463b35ab1bcc3bcf7409206641be43b38900 Mon Sep 17 00:00:00 2001 From: NotWearingPants <26556598+NotWearingPants@users.noreply.github.com> Date: Mon, 18 Oct 2021 19:42:31 +0300 Subject: [PATCH 04/18] Linked missing action command objects in schema (#11519) ## Summary of the Pull Request Currently when configuring the action ```json { "command": { "action": "commandPalette", "launchMode": "commandLine" }, "key": "ctrl+shift+p" } ``` or ```json { "command": { "action": "multipleActions", "actions": [{ "action": "paste" }] }, "key": "ctrl+shift+v" } ``` we get a schema error in VSCode. These object variants of the actions were not configured properly in the schema, so I fixed it. ## PR Checklist * [ ] Closes #xxx * [x] CLA signed * [ ] Tests added/passed * [ ] Documentation updated * [x] Schema updated * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan ## Detailed Description of the Pull Request / Additional comments In the schema there is a big `oneOf` for the `command` of an action under `actions`. Commands that also accept extra arguments have an object type defined for it. The `commandPalette` and `multipleActions` commands accept extra arguments, and also have matching `CommandPaletteAction` and `MultipleActionsAction` object types defined, but they are unused. So I added them to the `oneOf` array in the correct placement. ## Validation Steps Performed --- doc/cascadia/profiles.schema.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 49c8c4a46..37fddd7af 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -1333,6 +1333,12 @@ { "$ref": "#/$defs/MoveTabAction" }, + { + "$ref": "#/$defs/MultipleActionsAction" + }, + { + "$ref": "#/$defs/CommandPaletteAction" + }, { "$ref": "#/$defs/FindMatchAction" }, From 2cf31ac72db26d6272415bed5d411cd922ca1cfd Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 18 Oct 2021 18:55:38 +0200 Subject: [PATCH 05/18] Remove last remaining winrt::hstring allocation during text output (#11487) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ControlCore::FontFaceName() is called 10/s by TSFInputControl. The getter was modified to cache the STL string in a hstring allowing us to return a value without temporary allocations during runtime. ## PR Checklist * [x] I work here * [x] Tests added/passed ## Validation Steps Performed * Font face and size changes properly update TSFInputControl ✔️ --- src/cascadia/TerminalControl/ControlCore.cpp | 13 +++++++++---- src/cascadia/TerminalControl/ControlCore.h | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index c287cdbd7..a402a2d5e 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -600,6 +600,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // not, but DX doesn't use that info at all. // The Codepage is additionally not actually used by the DX engine at all. _actualFont = { fontFace, 0, fontWeight.Weight, { 0, fontHeight }, CP_UTF8, false }; + _actualFontFaceName = { fontFace }; _desiredFont = { _actualFont }; // Update the terminal core with its new Core settings @@ -742,6 +743,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation const auto fontFace = _settings.FontFace(); const auto fontWeight = _settings.FontWeight(); _actualFont = { fontFace, 0, fontWeight.Weight, { 0, newSize }, CP_UTF8, false }; + _actualFontFaceName = { fontFace }; _desiredFont = { _actualFont }; auto lock = _terminal->LockForWriting(); @@ -1020,7 +1022,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::Windows::Foundation::Size ControlCore::FontSize() const noexcept { - const auto fontSize = GetFont().GetSize(); + const auto fontSize = _actualFont.GetSize(); return { ::base::saturated_cast(fontSize.X), ::base::saturated_cast(fontSize.Y) @@ -1028,17 +1030,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation } winrt::hstring ControlCore::FontFaceName() const noexcept { - return winrt::hstring{ GetFont().GetFaceName() }; + // This getter used to return _actualFont.GetFaceName(), however GetFaceName() returns a STL + // string and we need to return a WinRT string. This would require an additional allocation. + // This method is called 10/s by TSFInputControl at the time of writing. + return _actualFontFaceName; } uint16_t ControlCore::FontWeight() const noexcept { - return static_cast(GetFont().GetWeight()); + return static_cast(_actualFont.GetWeight()); } til::size ControlCore::FontSizeInDips() const { - const til::size fontSize{ GetFont().GetSize() }; + const til::size fontSize{ _actualFont.GetSize() }; return fontSize.scale(til::math::rounding, 1.0f / ::base::saturated_cast(_compositionScale)); } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 9b96f6c7e..5beebf9e9 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -195,6 +195,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation FontInfoDesired _desiredFont; FontInfo _actualFont; + winrt::hstring _actualFontFaceName; // storage location for the leading surrogate of a utf-16 surrogate pair std::optional _leadingSurrogate{ std::nullopt }; From c1d326693c2d16a32f9d55f04647c84561926185 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 18 Oct 2021 19:20:34 +0200 Subject: [PATCH 06/18] Remove debug logging for PSReadline on newlines (#11486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Up until this commit PSReadline caused OutputDebugString to be called with a complex log message to on every newline. At the time of writing, Visual Studio's Output window is fairly slow and after this change newlines feel a fair bit snappier when running under Visual Studio's debugger. ## Validation Steps Performed * pwsh.exe continues to work correctly ✔️ --- src/host/getset.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 36f2ebbd8..c4e32b015 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -387,7 +387,7 @@ void ApiRoutines::GetNumberOfConsoleMouseButtonsImpl(ULONG& buttons) noexcept RETURN_HR_IF(E_INVALIDARG, WI_IsAnyFlagSet(mode, ~(INPUT_MODES | PRIVATE_MODES))); // ECHO on with LINE off is invalid. - RETURN_HR_IF(E_INVALIDARG, WI_IsFlagSet(mode, ENABLE_ECHO_INPUT) && WI_IsFlagClear(mode, ENABLE_LINE_INPUT)); + RETURN_HR_IF_EXPECTED(E_INVALIDARG, WI_IsFlagSet(mode, ENABLE_ECHO_INPUT) && WI_IsFlagClear(mode, ENABLE_LINE_INPUT)); } return S_OK; From fd93c54ae3d974d5e61dd106ff3c1899b0c9c082 Mon Sep 17 00:00:00 2001 From: NotWearingPants <26556598+NotWearingPants@users.noreply.github.com> Date: Tue, 19 Oct 2021 00:24:35 +0300 Subject: [PATCH 07/18] Change action names in schema to match without regex (#11520) ## Summary of the Pull Request Currently when configuring the action ```json { "command": { "action": "closeTabsAfter" } } ``` we get a schema error in VSCode: `Matches multiple schemas when only one must validate.`. The problem is that it matches both `closeTabsAfter` and `closeTab`, since the schema uses regex patterns to match instead of plain strings. I swapped the usage of `"pattern"` with `"const"` for all actions. ## PR Checklist * [ ] Closes #xxx * [x] CLA signed * [ ] Tests added/passed * [ ] Documentation updated * [x] Schema updated * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed I checked and this action configuration no longer errors. --- doc/cascadia/profiles.schema.json | 62 +++++++++++++++---------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 37fddd7af..98f3de653 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -506,7 +506,7 @@ "properties": { "action": { "type": "string", - "pattern": "adjustFontSize" + "const": "adjustFontSize" }, "delta": { "type": "integer", @@ -530,7 +530,7 @@ "properties": { "action": { "type": "string", - "pattern": "copy" + "const": "copy" }, "singleLine": { "type": "boolean", @@ -566,7 +566,7 @@ "properties": { "action": { "type": "string", - "pattern": "newTab" + "const": "newTab" } } } @@ -582,7 +582,7 @@ "properties": { "action": { "type": "string", - "pattern": "switchToTab" + "const": "switchToTab" }, "index": { "type": "integer", @@ -606,7 +606,7 @@ "properties": { "action": { "type": "string", - "pattern": "movePane" + "const": "movePane" }, "index": { "type": "integer", @@ -630,7 +630,7 @@ "properties": { "action": { "type": "string", - "pattern": "moveFocus" + "const": "moveFocus" }, "direction": { "$ref": "#/$defs/FocusDirection", @@ -654,7 +654,7 @@ "properties": { "action": { "type": "string", - "pattern": "swapPane" + "const": "swapPane" }, "direction": { "$ref": "#/$defs/FocusDirection", @@ -678,7 +678,7 @@ "properties": { "action": { "type": "string", - "pattern": "resizePane" + "const": "resizePane" }, "direction": { "$ref": "#/$defs/ResizeDirection", @@ -702,7 +702,7 @@ "properties": { "action": { "type": "string", - "pattern": "sendInput" + "const": "sendInput" }, "input": { "type": "string", @@ -729,7 +729,7 @@ "properties": { "action": { "type": "string", - "pattern": "splitPane" + "const": "splitPane" }, "split": { "$ref": "#/$defs/SplitDirection", @@ -761,7 +761,7 @@ "properties": { "action": { "type": "string", - "pattern": "openSettings" + "const": "openSettings" }, "target": { "type": "string", @@ -788,7 +788,7 @@ "properties": { "action": { "type": "string", - "pattern": "setTabColor" + "const": "setTabColor" }, "color": { "$ref": "#/$defs/Color", @@ -809,7 +809,7 @@ "properties": { "action": { "type": "string", - "pattern": "setColorScheme" + "const": "setColorScheme" }, "colorScheme": { "type": "string", @@ -833,7 +833,7 @@ "properties": { "action": { "type": "string", - "pattern": "wt" + "const": "wt" }, "commandline": { "type": "string", @@ -857,7 +857,7 @@ "properties": { "action": { "type": "string", - "pattern": "closeOtherTabs" + "const": "closeOtherTabs" }, "index": { "oneOf": [ @@ -885,7 +885,7 @@ "properties": { "action": { "type": "string", - "pattern": "closeTabsAfter" + "const": "closeTabsAfter" }, "index": { "oneOf": [ @@ -913,7 +913,7 @@ "properties": { "action": { "type": "string", - "pattern": "closeTab" + "const": "closeTab" }, "index": { "oneOf": [ @@ -941,7 +941,7 @@ "properties": { "action": { "type": "string", - "pattern": "scrollUp" + "const": "scrollUp" }, "rowsToScroll": { "type": [ @@ -965,7 +965,7 @@ "properties": { "action": { "type": "string", - "pattern": "scrollDown" + "const": "scrollDown" }, "rowsToScroll": { "type": [ @@ -989,7 +989,7 @@ "properties": { "action": { "type": "string", - "pattern": "moveTab" + "const": "moveTab" }, "direction": { "$ref": "#/$defs/MoveTabDirection", @@ -1012,7 +1012,7 @@ "properties": { "action": { "type": "string", - "pattern": "multipleActions" + "const": "multipleActions" }, "actions": { "$ref": "#/$defs/ShortcutAction", @@ -1037,7 +1037,7 @@ "properties": { "action": { "type": "string", - "pattern": "commandPalette" + "const": "commandPalette" }, "launchMode": { "$ref": "#/$defs/CommandPaletteLaunchMode", @@ -1058,7 +1058,7 @@ "properties": { "action": { "type": "string", - "pattern": "findMatch" + "const": "findMatch" }, "direction": { "$ref": "#/$defs/FindMatchDirection", @@ -1085,7 +1085,7 @@ "properties": { "action": { "type": "string", - "pattern": "newWindow" + "const": "newWindow" } } } @@ -1101,7 +1101,7 @@ "properties": { "action": { "type": "string", - "pattern": "prevTab" + "const": "prevTab" }, "tabSwitcherMode": { "$ref": "#/$defs/SwitchToAdjacentTabArgs", @@ -1122,7 +1122,7 @@ "properties": { "action": { "type": "string", - "pattern": "nextTab" + "const": "nextTab" }, "tabSwitcherMode": { "$ref": "#/$defs/SwitchToAdjacentTabArgs", @@ -1143,7 +1143,7 @@ "properties": { "action": { "type": "string", - "pattern": "renameTab" + "const": "renameTab" }, "title": { "type": "string", @@ -1164,7 +1164,7 @@ "properties": { "action": { "type": "string", - "pattern": "renameWindow" + "const": "renameWindow" }, "name": { "type": "string", @@ -1185,7 +1185,7 @@ "properties": { "action": { "type": "string", - "pattern": "focusPane" + "const": "focusPane" }, "id": { "type": "integer", @@ -1207,7 +1207,7 @@ "properties": { "action": { "type": "string", - "pattern": "globalSummon" + "const": "globalSummon" }, "desktop": { "type": "string", @@ -1258,7 +1258,7 @@ "properties": { "action": { "type": "string", - "pattern": "quakeMode" + "const": "quakeMode" } } } From 0d5af3fedcb8d23986ae75a014bc65e46fefd2a2 Mon Sep 17 00:00:00 2001 From: NotWearingPants <26556598+NotWearingPants@users.noreply.github.com> Date: Tue, 19 Oct 2021 00:25:13 +0300 Subject: [PATCH 08/18] Remove double-space in defaults.json (#11518) There was a double-space after a colon in `defaults.json` and `defaults-universal.json`. --- src/cascadia/TerminalSettingsModel/defaults-universal.json | 2 +- src/cascadia/TerminalSettingsModel/defaults.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/defaults-universal.json b/src/cascadia/TerminalSettingsModel/defaults-universal.json index 4aa5128d7..22507f5c0 100644 --- a/src/cascadia/TerminalSettingsModel/defaults-universal.json +++ b/src/cascadia/TerminalSettingsModel/defaults-universal.json @@ -22,7 +22,7 @@ // Miscellaneous "confirmCloseAllTabs": true, - "startOnUserLogin": false, + "startOnUserLogin": false, "theme": "system", "snapToGridOnResize": true, diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index fb2078966..1c7f734f7 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -24,7 +24,7 @@ // Miscellaneous "confirmCloseAllTabs": true, - "startOnUserLogin": false, + "startOnUserLogin": false, "theme": "system", "snapToGridOnResize": true, "disableAnimations": false, From 284257a38392c85a2f7d7ac7e77e20a05242ec64 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 19 Oct 2021 15:29:18 -0500 Subject: [PATCH 09/18] Add even MORE logging for defterm (#11537) Considering the number of reports of "defterm isn't working (mysteriously)", I figured more logging current hurt. I also added a wprp profile for the defterm logging as well, which should capture conhost side things as well. From an elevated conhost: ``` wpr -start path\to\Terminal.wprp!Defterm.Verbose wpr -stop %USERPROFILE%\defterm-trace.etl ``` * [x] I work here * [x] relevant to: #10594, #11529, #11524. --- src/Terminal.wprp | 33 ++++++++++- .../TerminalConnection/ConptyConnection.cpp | 10 ++++ src/cascadia/WindowsTerminal/AppHost.cpp | 23 +++++--- src/host/srvinit.cpp | 57 +++++++++++++++++++ 4 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/Terminal.wprp b/src/Terminal.wprp index 80a6e730b..129d83ca0 100644 --- a/src/Terminal.wprp +++ b/src/Terminal.wprp @@ -13,6 +13,15 @@ + + + + + + + + + @@ -32,5 +41,27 @@ + + + + + + + + + + + + + + + + + + + + + + - \ No newline at end of file + diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index 3792724d5..9135f8024 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -372,6 +372,16 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation // window is expecting it to be on the first layout. else { +#pragma warning(suppress : 26477 26485 26494 26482 26446) // We don't control TraceLoggingWrite + TraceLoggingWrite( + g_hTerminalConnectionProvider, + "ConPtyConnectedToDefterm", + TraceLoggingDescription("Event emitted when ConPTY connection is started, for a defterm session"), + TraceLoggingGuid(_guid, "SessionGuid", "The WT_SESSION's GUID"), + TraceLoggingWideString(_clientName.c_str(), "Client", "The attached client process"), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + THROW_IF_FAILED(ConptyResizePseudoConsole(_hPC.get(), dimensions)); } diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index aa84b9e9e..c5d7418a0 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -1323,14 +1323,23 @@ void AppHost::_WindowMoved() const auto root{ _logic.GetRoot() }; - // This is basically DismissAllPopups which is also in - // TerminalSettingsEditor/Utils.h - // There isn't a good place that's shared between these two files, but - // it's only 5 LOC so whatever. - const auto popups{ Media::VisualTreeHelper::GetOpenPopupsForXamlRoot(root.XamlRoot()) }; - for (const auto& p : popups) + try { - p.IsOpen(false); + // This is basically DismissAllPopups which is also in + // TerminalSettingsEditor/Utils.h + // There isn't a good place that's shared between these two files, but + // it's only 5 LOC so whatever. + const auto popups{ Media::VisualTreeHelper::GetOpenPopupsForXamlRoot(root.XamlRoot()) }; + for (const auto& p : popups) + { + p.IsOpen(false); + } + } + catch (...) + { + // We purposely don't log here, because this is exceptionally noisy, + // especially on startup, when we're moving the window into place + // but might not have a real xamlRoot yet. } } } diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index b91a12873..49acf6f6a 100644 --- a/src/host/srvinit.cpp +++ b/src/host/srvinit.cpp @@ -28,6 +28,8 @@ #include "../inc/conint.h" #include "../propslib/DelegationConfig.hpp" +#include "tracing.hpp" + #if TIL_FEATURE_RECEIVEINCOMINGHANDOFF_ENABLED #include "ITerminalHandoff.h" #endif // TIL_FEATURE_RECEIVEINCOMINGHANDOFF_ENABLED @@ -69,10 +71,20 @@ try if (SUCCEEDED(DelegationConfig::s_GetDefaultConsoleId(delegationClsid))) { Globals.handoffConsoleClsid = delegationClsid; + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_FoundDelegationConsole", + TraceLoggingGuid(Globals.handoffConsoleClsid.value(), "ConsoleClsid"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); } if (SUCCEEDED(DelegationConfig::s_GetDefaultTerminalId(delegationClsid))) { Globals.handoffTerminalClsid = delegationClsid; + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_FoundDelegationTerminal", + TraceLoggingGuid(Globals.handoffTerminalClsid.value(), "TerminalClsid"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); } } @@ -400,7 +412,16 @@ HRESULT ConsoleCreateIoThread(_In_ HANDLE Server, [[maybe_unused]] PCONSOLE_API_MSG connectMessage) try { + // Create a telemetry instance here - this singleton is responsible for + // setting up the g_hConhostV2EventTraceProvider, which is otherwise not + // initialized in the defterm handoff at this point. + (void)Telemetry::Instance(); + #if !TIL_FEATURE_RECEIVEINCOMINGHANDOFF_ENABLED + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_ReceiveHandoff_Disabled", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED); #else // TIL_FEATURE_RECEIVEINCOMINGHANDOFF_ENABLED auto& g = ServiceLocator::LocateGlobals(); @@ -418,9 +439,19 @@ try if (!g.handoffTerminalClsid) { + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_ReceiveHandoff_NoTerminal", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); return E_NOT_SET; } + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_ReceiveHandoff", + TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // Capture handle to the inbox process into a unique handle holder. g.handoffInboxConsoleHandle.reset(hostProcessHandle); @@ -453,9 +484,19 @@ try RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(outPipeTheirSide.addressof(), outPipeOurSide.addressof(), nullptr, 0)); + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_ReceiveHandoff_OpenedPipes", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + wil::unique_handle clientProcess{ OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ | SYNCHRONIZE, TRUE, static_cast(connectMessage->Descriptor.Process)) }; RETURN_LAST_ERROR_IF_NULL(clientProcess.get()); + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_ReceiveHandoff_OpenedClient", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + wil::unique_handle refHandle; RETURN_IF_NTSTATUS_FAILED(DeviceHandle::CreateClientHandle(refHandle.addressof(), Server, @@ -466,7 +507,18 @@ try ::Microsoft::WRL::ComPtr handoff; + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_PrepareToCreateDelegationTerminal", + TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + RETURN_IF_FAILED(CoCreateInstance(g.handoffTerminalClsid.value(), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff))); + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_CreatedDelegationTerminal", + TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); RETURN_IF_FAILED(handoff->EstablishPtyHandoff(inPipeTheirSide.get(), outPipeTheirSide.get(), @@ -475,6 +527,11 @@ try serverProcess, clientProcess.get())); + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_DelegateToTerminalSucceeded", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + inPipeTheirSide.reset(); outPipeTheirSide.reset(); signalPipeTheirSide.reset(); From 5a23029dacf091acf668a8d234af7b8c0e5ab4c8 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 20 Oct 2021 01:52:00 +0200 Subject: [PATCH 10/18] Further reduce number of generated VS profiles (#11489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit reduces the number of generated VS profiles from 6 down to just 2 per VS instance. The reason we did this is out of concern of overwhelming or annoying new users with too many profiles. Especially since it's far easier at the moment to add new generators compared to removing them. As before only the latest instance is not hidden by default. ## PR Checklist * [x] I work here * [x] Tests added/passed * [x] As discussed in a Team Sync meeting ## Validation Steps Performed * Installed Visual Studio 2019 and 2022 Preview * A profile for both is generated, while the 2019 one is hidden by default ✔️ * $env:VSCMD_ARG_TGT_ARCH is x64 on my AMD64 machine ✔️ --- ...crosoft.Terminal.Settings.ModelLib.vcxproj | 4 +- ...Terminal.Settings.ModelLib.vcxproj.filters | 8 +- .../VcDevCmdGenerator.cpp | 94 ------------------- .../TerminalSettingsModel/VcDevCmdGenerator.h | 40 -------- .../VisualStudioGenerator.cpp | 4 - .../VsDevCmdGenerator.cpp | 11 ++- .../VsDevShellGenerator.cpp | 17 +++- .../VsSetupConfiguration.cpp | 12 +-- .../VsSetupConfiguration.h | 8 +- 9 files changed, 30 insertions(+), 168 deletions(-) delete mode 100644 src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.cpp delete mode 100644 src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.h diff --git a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj index a5d82e34d..60de4fd1c 100644 --- a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj +++ b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj @@ -14,7 +14,6 @@ - DefaultTerminal.idl @@ -85,7 +84,6 @@ - DefaultTerminal.idl @@ -267,4 +265,4 @@ - \ No newline at end of file + diff --git a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters index 873dcb28c..a99f5e654 100644 --- a/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters +++ b/src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters @@ -46,9 +46,6 @@ profileGeneration - - profileGeneration - @@ -95,9 +92,6 @@ profileGeneration - - profileGeneration - @@ -129,4 +123,4 @@ {81a6314f-aa5b-4533-a499-13bc3a5c4af0} - \ No newline at end of file + diff --git a/src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.cpp b/src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.cpp deleted file mode 100644 index 633e5b94c..000000000 --- a/src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.cpp +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" -#include "DynamicProfileUtils.h" -#include "VcDevCmdGenerator.h" - -using namespace winrt::Microsoft::Terminal::Settings::Model; - -void VcDevCmdGenerator::GenerateProfiles(const VsSetupConfiguration::VsSetupInstance& instance, bool hidden, std::vector>& profiles) const -{ - try - { - const std::filesystem::path root{ GetVcCmdScriptDirectory(instance) }; - if (!std::filesystem::exists(root)) - { - return; - } - -// x64 environments only installed on 64-bit machines. -#ifdef _WIN64 - const auto vcvars64 = root / L"vcvars64.bat"; - if (std::filesystem::exists(vcvars64)) - { - auto profile = CreateProfile(instance, L"x64", vcvars64, hidden); - profiles.emplace_back(std::move(profile)); - - // Only the VC environment for the matching architecture should be shown by default. - hidden = true; - } - - const auto vcvarsamd64_x86 = root / L"vcvarsamd64_x86.bat"; - if (std::filesystem::exists(vcvarsamd64_x86)) - { - auto profile = CreateProfile(instance, L"x64_x86", vcvarsamd64_x86, true); - profiles.emplace_back(std::move(profile)); - } - - const auto vcvarsx86_amd64 = root / L"vcvarsx86_amd64.bat"; - if (std::filesystem::exists(vcvarsx86_amd64)) - { - auto profile = CreateProfile(instance, L"x86_x64", vcvarsx86_amd64, true); - profiles.emplace_back(std::move(profile)); - } -#endif // _WIN64 - - const auto vcvars32 = root / L"vcvars32.bat"; - if (std::filesystem::exists(vcvars32)) - { - auto profile = CreateProfile(instance, L"x86", vcvars32, hidden); - profiles.emplace_back(std::move(profile)); - } - } - CATCH_LOG(); -} - -winrt::com_ptr VcDevCmdGenerator::CreateProfile(const VsSetupConfiguration::VsSetupInstance& instance, const std::wstring_view& prefix, const std::filesystem::path& path, bool hidden) const -{ - const auto seed = GetProfileGuidSeed(instance, path); - const winrt::guid profileGuid{ ::Microsoft::Console::Utils::CreateV5Uuid(TERMINAL_PROFILE_NAMESPACE_GUID, gsl::as_bytes(gsl::make_span(seed))) }; - - auto profile = winrt::make_self(profileGuid); - profile->Name(winrt::hstring{ GetProfileName(instance, prefix) }); - profile->Commandline(winrt::hstring{ GetProfileCommandLine(path) }); - profile->StartingDirectory(winrt::hstring{ instance.GetInstallationPath() }); - profile->Icon(winrt::hstring{ GetProfileIconPath() }); - profile->Hidden(hidden); - - return profile; -} - -std::wstring VcDevCmdGenerator::GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance& instance, const std::filesystem::path& path) const -{ - return L"VsDevCmd" + instance.GetInstanceId() + path.native(); -} - -std::wstring VcDevCmdGenerator::GetProfileName(const VsSetupConfiguration::VsSetupInstance& instance, const std::wstring_view& prefix) const -{ - std::wstring name{ prefix + L" Native Tools Command Prompt for VS " }; - name.append(instance.GetProfileNameSuffix()); - return name; -} - -std::wstring VcDevCmdGenerator::GetProfileCommandLine(const std::filesystem::path& path) const -{ - std::wstring commandLine{ L"cmd.exe /k \"" + path.native() + L"\"" }; - - return commandLine; -} - -std::wstring VcDevCmdGenerator::GetVcCmdScriptDirectory(const VsSetupConfiguration::VsSetupInstance& instance) const -{ - return instance.ResolvePath(L"VC\\Auxiliary\\Build\\"); -} diff --git a/src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.h b/src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.h deleted file mode 100644 index 8e30cf28b..000000000 --- a/src/cascadia/TerminalSettingsModel/VcDevCmdGenerator.h +++ /dev/null @@ -1,40 +0,0 @@ -/*++ -Copyright (c) Microsoft Corporation -Licensed under the MIT license. - -Module Name: -- VcDevCmdGenerator - -Abstract: -- Dynamic profile generator for Visual C++ development environments. - -Author(s): -- Heath Stewart - September 2021 - ---*/ - -#pragma once -#include "VisualStudioGenerator.h" -#include "VsSetupConfiguration.h" - -namespace winrt::Microsoft::Terminal::Settings::Model -{ - class VcDevCmdGenerator final : public VisualStudioGenerator::IVisualStudioProfileGenerator - { - public: - void GenerateProfiles(const VsSetupConfiguration::VsSetupInstance& instance, bool hidden, std::vector>& profiles) const override; - - private: - winrt::com_ptr CreateProfile(const VsSetupConfiguration::VsSetupInstance& instance, const std::wstring_view& prefix, const std::filesystem::path& path, bool hidden) const; - - std::wstring GetProfileIconPath() const - { - return L"ms-appx:///ProfileIcons/{0caa0dad-35be-5f56-a8ff-afceeeaa6101}.png"; - } - - std::wstring GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance& instance, const std::filesystem::path& path) const; - std::wstring GetProfileName(const VsSetupConfiguration::VsSetupInstance& instance, const std::wstring_view& prefix) const; - std::wstring GetProfileCommandLine(const std::filesystem::path& path) const; - std::wstring GetVcCmdScriptDirectory(const VsSetupConfiguration::VsSetupInstance& instance) const; - }; -}; diff --git a/src/cascadia/TerminalSettingsModel/VisualStudioGenerator.cpp b/src/cascadia/TerminalSettingsModel/VisualStudioGenerator.cpp index 9e883205e..2c1069b3f 100644 --- a/src/cascadia/TerminalSettingsModel/VisualStudioGenerator.cpp +++ b/src/cascadia/TerminalSettingsModel/VisualStudioGenerator.cpp @@ -4,7 +4,6 @@ #include "pch.h" #include "DynamicProfileUtils.h" #include "VisualStudioGenerator.h" -#include "VcDevCmdGenerator.h" #include "VsDevCmdGenerator.h" #include "VsDevShellGenerator.h" @@ -20,7 +19,6 @@ void VisualStudioGenerator::GenerateProfiles(std::vector