diff --git a/README.md b/README.md index 3d88994521a..f43556bb03d 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,8 @@ Related repositories include: ## Installing and running Windows Terminal -> **Note**: Windows Terminal requires Windows 10 2004 (build 19041) or later +> **Note**\ +> Windows Terminal requires Windows 10 2004 (build 19041) or later ### Microsoft Store [Recommended] @@ -52,9 +53,10 @@ fails for any reason, you can try the following command at a PowerShell prompt: Add-AppxPackage Microsoft.WindowsTerminal_.msixbundle ``` -> **Note**: If you install Terminal manually: +> **Note**\ +> If you install Terminal manually: > -> * 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). +> * 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). > This should only be necessary on older builds of Windows 10 and only if you get an error about missing framework packages. > * Terminal will not auto-update when new builds are released so you will need > to regularly install the latest Terminal release to receive all the latest @@ -70,7 +72,8 @@ package: winget install --id Microsoft.WindowsTerminal -e ``` -> **Note** Due to a dependency issue, 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**\ +> Due to a dependency issue, 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. #### Via Chocolatey (unofficial) @@ -237,7 +240,8 @@ Cause: You're launching the incorrect solution in Visual Studio. Solution: Make sure you're building & deploying the `CascadiaPackage` project in Visual Studio. -> **Note**: `OpenConsole.exe` is just a locally-built `conhost.exe`, the classic +> **Note**\ +> `OpenConsole.exe` is just a locally-built `conhost.exe`, the classic > Windows Console that hosts Windows' command-line infrastructure. OpenConsole > is used by Windows Terminal to connect to and communicate with command-line > applications (via diff --git a/build/pipelines/ci.yml b/build/pipelines/ci.yml index 7e225d7cb89..3b833d8e718 100644 --- a/build/pipelines/ci.yml +++ b/build/pipelines/ci.yml @@ -29,64 +29,36 @@ variables: # 0.0.1904.0900 name: 0.0.$(Date:yyMM).$(Date:dd)$(Rev:rr) -stages: - - stage: Audit_x64 - displayName: Audit Mode - dependsOn: [] - condition: succeeded() - jobs: - - template: ./templates/build-console-audit-job.yml - parameters: - platform: x64 - - - stage: Build_x64 - displayName: Build x64 - dependsOn: [] - condition: succeeded() - jobs: - - template: ./templates/build-console-ci.yml - parameters: - platform: x64 - - stage: Build_x86 - displayName: Build x86 - dependsOn: [] - jobs: - - template: ./templates/build-console-ci.yml - parameters: - platform: x86 - - stage: Build_ARM64 - displayName: Build ARM64 - dependsOn: [] - condition: not(eq(variables['Build.Reason'], 'PullRequest')) - jobs: - - template: ./templates/build-console-ci.yml - parameters: - platform: ARM64 - - - stage: Test_x64 - displayName: Test x64 - dependsOn: [Build_x64] - condition: succeeded() - jobs: - - template: ./templates/test-console-ci.yml - parameters: - platform: x64 - - stage: Test_x86 - displayName: Test x86 - dependsOn: [Build_x86] - jobs: - - template: ./templates/test-console-ci.yml - parameters: - platform: x86 +parameters: + - name: auditMode + displayName: "Build in Audit Mode (x64)" + type: boolean + default: true + - name: runTests + displayName: "Run Unit and Feature Tests" + type: boolean + default: true + - name: submitHelix + displayName: "Submit to Helix for testing and PGO" + type: boolean + default: true + - name: buildPlatforms + type: object + default: + - x64 + - x86 + - arm64 - - stage: Helix_x64 - displayName: Helix x64 - dependsOn: [Build_x64] - condition: and(succeeded(), not(eq(variables['Build.Reason'], 'PullRequest'))) - jobs: - - template: ./templates/console-ci-helix-job.yml - parameters: - platform: x64 +stages: + - ${{ if eq(parameters.auditMode, true) }}: + - stage: Audit_x64 + displayName: Audit Mode + dependsOn: [] + condition: succeeded() + jobs: + - template: ./templates/build-console-audit-job.yml + parameters: + platform: x64 - stage: Scripts displayName: Code Health Scripts @@ -95,10 +67,38 @@ stages: jobs: - template: ./templates/check-formatting.yml + - ${{ each platform in parameters.buildPlatforms }}: + - stage: Build_${{ platform }} + displayName: Build ${{ platform }} + dependsOn: [] + condition: succeeded() + jobs: + - template: ./templates/build-console-ci.yml + parameters: + platform: ${{ platform }} + - ${{ if eq(parameters.runTests, true) }}: + - stage: Test_${{ platform }} + displayName: Test ${{ platform }} + dependsOn: + - Build_${{ platform }} + condition: succeeded() + jobs: + - template: ./templates/test-console-ci.yml + parameters: + platform: ${{ platform }} - - stage: CodeIndexer - displayName: Github CodeNav Indexer - dependsOn: [Build_x64] - condition: and(succeeded(), not(eq(variables['Build.Reason'], 'PullRequest'))) - jobs: - - template: ./templates/codenav-indexer.yml + - ${{ if and(containsValue(parameters.buildPlatforms, 'x64'), eq(parameters.submitHelix, true), ne(variables['Build.Reason'], 'PullRequest')) }}: + - stage: Helix_x64 + displayName: Helix x64 + dependsOn: [Build_x64] + jobs: + - template: ./templates/console-ci-helix-job.yml + parameters: + platform: x64 + + - ${{ if and(containsValue(parameters.buildPlatforms, 'x64'), ne(variables['Build.Reason'], 'PullRequest')) }}: + - stage: CodeIndexer + displayName: Github CodeNav Indexer + dependsOn: [Build_x64] + jobs: + - template: ./templates/codenav-indexer.yml diff --git a/build/pipelines/release.yml b/build/pipelines/release.yml index 8429f0abe0f..cea5c7d0fc5 100644 --- a/build/pipelines/release.yml +++ b/build/pipelines/release.yml @@ -3,8 +3,8 @@ trigger: none pr: none pool: - name: WinDevPool-L - demands: ImageOverride -equals WinDevVS17-latest + name: SHINE-INT-S # By default, send jobs to the small agent pool. + demands: ImageOverride -equals SHINE-VS17-Latest parameters: - name: branding @@ -91,6 +91,9 @@ resources: ref: main jobs: - job: Build + pool: + name: SHINE-INT-L # Run the compilation on the large agent pool, rather than the default small one. + demands: ImageOverride -equals SHINE-VS17-Latest strategy: matrix: ${{ each config in parameters.buildConfigurations }}: diff --git a/build/pipelines/templates/build-console-audit-job.yml b/build/pipelines/templates/build-console-audit-job.yml index 1ed556eff8f..69b0b05ce31 100644 --- a/build/pipelines/templates/build-console-audit-job.yml +++ b/build/pipelines/templates/build-console-audit-job.yml @@ -10,10 +10,10 @@ jobs: BuildPlatform: ${{ parameters.platform }} pool: ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPoolOSS-L + name: SHINE-OSS-L ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPool-L - demands: ImageOverride -equals WinDevVS17-latest + name: SHINE-INT-L + demands: ImageOverride -equals SHINE-VS17-Latest steps: - checkout: self diff --git a/build/pipelines/templates/build-console-ci.yml b/build/pipelines/templates/build-console-ci.yml index 0ccf90f10f6..48c27052e29 100644 --- a/build/pipelines/templates/build-console-ci.yml +++ b/build/pipelines/templates/build-console-ci.yml @@ -14,10 +14,10 @@ jobs: EnableRichCodeNavigation: true pool: ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPoolOSS-L + name: SHINE-OSS-L ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPool-L - demands: ImageOverride -equals WinDevVS17-latest + name: SHINE-INT-L + demands: ImageOverride -equals SHINE-VS17-Latest steps: - template: build-console-steps.yml diff --git a/build/pipelines/templates/build-console-fuzzing.yml b/build/pipelines/templates/build-console-fuzzing.yml index e115fbb3751..8277ab9dc3a 100644 --- a/build/pipelines/templates/build-console-fuzzing.yml +++ b/build/pipelines/templates/build-console-fuzzing.yml @@ -11,10 +11,10 @@ jobs: BuildPlatform: ${{ parameters.platform }} pool: ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPoolOSS-L + name: SHINE-OSS-L ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPool-L - demands: ImageOverride -equals WinDevVS17-latest + name: SHINE-INT-L + demands: ImageOverride -equals SHINE-VS17-Latest steps: - checkout: self diff --git a/build/pipelines/templates/build-console-pgo.yml b/build/pipelines/templates/build-console-pgo.yml index 1187146ccf2..b1b4c501633 100644 --- a/build/pipelines/templates/build-console-pgo.yml +++ b/build/pipelines/templates/build-console-pgo.yml @@ -14,10 +14,10 @@ jobs: PGOBuildMode: 'Instrument' pool: ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPoolOSS-L + name: SHINE-OSS-L ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPool-L - demands: ImageOverride -equals WinDevVS17-latest + name: SHINE-INT-L + demands: ImageOverride -equals SHINE-VS17-Latest steps: - template: build-console-steps.yml diff --git a/build/pipelines/templates/test-console-ci.yml b/build/pipelines/templates/test-console-ci.yml index b2eea094f13..5a7db6475bd 100644 --- a/build/pipelines/templates/test-console-ci.yml +++ b/build/pipelines/templates/test-console-ci.yml @@ -13,14 +13,19 @@ jobs: BuildPlatform: ${{ parameters.platform }} pool: ${{ if eq(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPoolOSS-L + ${{ if ne(parameters.platform, 'ARM64') }}: + name: SHINE-OSS-Testing-x64 + ${{ else }}: + name: SHINE-OSS-Testing-arm64 ${{ if ne(variables['System.CollectionUri'], 'https://dev.azure.com/ms/') }}: - name: WinDevPool-L - demands: ImageOverride -equals WinDevVS17-latest + ${{ if ne(parameters.platform, 'ARM64') }}: + name: SHINE-INT-Testing-x64 + ${{ else }}: + name: SHINE-INT-Testing-arm64 steps: - checkout: self - submodules: true + submodules: false clean: true fetchDepth: 1 @@ -43,15 +48,16 @@ jobs: targetType: filePath filePath: build\scripts\Run-Tests.ps1 arguments: -MatchPattern '*unit.test*.dll' -Platform '$(RationalizedBuildPlatform)' -Configuration '$(BuildConfiguration)' -LogPath '${{ parameters.testLogPath }}' -Root "$(System.ArtifactsDirectory)\\${{ parameters.artifactName }}\\$(BuildConfiguration)\\$(BuildPlatform)\\test" - condition: and(and(succeeded(), ne(variables['PGOBuildMode'], 'Instrument')), or(eq(variables['BuildPlatform'], 'x64'), eq(variables['BuildPlatform'], 'x86'))) + condition: and(succeeded(), ne(variables['PGOBuildMode'], 'Instrument')) - - task: PowerShell@2 - displayName: 'Run Feature Tests (x64 only)' - inputs: - targetType: filePath - filePath: build\scripts\Run-Tests.ps1 - arguments: -MatchPattern '*feature.test*.dll' -Platform '$(RationalizedBuildPlatform)' -Configuration '$(BuildConfiguration)' -LogPath '${{ parameters.testLogPath }}' -Root "$(System.ArtifactsDirectory)\\${{ parameters.artifactName }}\\$(BuildConfiguration)\\$(BuildPlatform)\\test" - condition: and(and(succeeded(), ne(variables['PGOBuildMode'], 'Instrument')), eq(variables['BuildPlatform'], 'x64')) + - ${{ if or(eq(parameters.platform, 'x64'), eq(parameters.platform, 'arm64')) }}: + - task: PowerShell@2 + displayName: 'Run Feature Tests' + inputs: + targetType: filePath + filePath: build\scripts\Run-Tests.ps1 + arguments: -MatchPattern '*feature.test*.dll' -Platform '$(RationalizedBuildPlatform)' -Configuration '$(BuildConfiguration)' -LogPath '${{ parameters.testLogPath }}' -Root "$(System.ArtifactsDirectory)\\${{ parameters.artifactName }}\\$(BuildConfiguration)\\$(BuildPlatform)\\test" + condition: and(succeeded(), ne(variables['PGOBuildMode'], 'Instrument')) - task: PowerShell@2 displayName: 'Convert Test Logs from WTL to xUnit format' @@ -59,7 +65,7 @@ jobs: targetType: filePath filePath: build\Helix\ConvertWttLogToXUnit.ps1 arguments: -WttInputPath '${{ parameters.testLogPath }}' -WttSingleRerunInputPath 'unused.wtl' -WttMultipleRerunInputPath 'unused2.wtl' -XUnitOutputPath 'onBuildMachineResults.xml' -TestNamePrefix '$(BuildConfiguration).$(BuildPlatform)' - condition: and(ne(variables['PGOBuildMode'], 'Instrument'),or(eq(variables['BuildPlatform'], 'x64'), eq(variables['BuildPlatform'], 'x86'))) + condition: ne(variables['PGOBuildMode'], 'Instrument') - task: PublishTestResults@2 displayName: 'Upload converted test logs' @@ -67,13 +73,9 @@ jobs: inputs: testResultsFormat: 'xUnit' # Options: JUnit, NUnit, VSTest, xUnit, cTest testResultsFiles: '**/onBuildMachineResults.xml' - #searchFolder: '$(System.DefaultWorkingDirectory)' # Optional - #mergeTestResults: false # Optional - #failTaskOnFailedTests: false # Optional testRunTitle: 'On Build Machine Tests' # Optional buildPlatform: $(BuildPlatform) # Optional buildConfiguration: $(BuildConfiguration) # Optional - #publishRunAttachments: true # Optional - task: CopyFiles@2 displayName: 'Copy result logs to Artifacts' diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index d3ccfa8d122..c681ef29267 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -260,7 +260,8 @@ "description": "Name of the scheme to use when the app is using dark theme", "type": "string" } - } + }, + "type": "object" }, "FontConfig": { "properties": { diff --git a/doc/specs/#11000 - Marks/Shell-Integration-Marks.md b/doc/specs/#11000 - Marks/Shell-Integration-Marks.md new file mode 100644 index 00000000000..1d81157ea99 --- /dev/null +++ b/doc/specs/#11000 - Marks/Shell-Integration-Marks.md @@ -0,0 +1,499 @@ +--- +author: Mike Griese +created on: 2022-03-28 +last updated: 2023-07-19 +issue id: 11000, 1527, 6232 +--- + +##### [Original issue: [#1527]] [experimental PR [#12948]] [remaining marks [#14341]] + +# Windows Terminal - Shell Integration (Marks) + +## Abstract + +_"Shell integration" refers to a broad category of ways by which a commandline +shell can drive richer integration with the terminal. This spec in particular is +most concerned with "marks" and other semantic markup of the buffer._ + +Marks are a new buffer-side feature that allow the commandline application or +user to add a bit of metadata to a range of text. This can be used for marking a +region of text as a prompt, marking a command as succeeded or failed, quickly +marking errors in the output. These marks can then be exposed to the user as +pips on the scrollbar, or as icons in the margins. Additionally, the user can +quickly scroll between different marks, to allow easy navigation between +important information in the buffer. + +Marks in the Windows Terminal are a combination of functionality from a variety +of different terminal emulators. "Marks" attempts to unify these different, but +related pieces of functionality. + +## Background + +There's a large amount of prior art on this subject. I've attempted to collect +as much as possible in the ["Relevant external docs"](#Relevant-external-docs) +section below. "Marks" have been used in different scenarios by different +emulators for different purposes. The common thread running between them of +marking a region of text in the buffer with a special meaning. + +* iTerm2, ConEmu, FinalTerm et.al. support emitting a VT sequence to indicate + that a line is a "prompt" line. This is often used for quick navigation + between these prompts. +* FinalTerm (and xterm.js) also support marking up more than just the prompt. + They go so far as to differentiate the start/end of the prompt, the start of + the commandline input, and the start/end of the command output. + `FTCS_COMMAND_FINISHED` is even designed to include metadata indicating + whether a command succeeded or failed. +* Additionally, Terminal.app allows users to "bookmark" lines via the UI. That + allows users to quickly come back to something they feel is important. +* Consider also editors like Visual Studio. VS also uses little marks on the + scrollbar to indicate where other matches are for whatever the given search + term is. + +### "Elevator" pitch + +The Terminal provides a way for command line shells to semantically mark parts +of the command-line output. By marking up parts of the output, the Terminal can +provide richer experiences. The Terminal will know where each command starts and +stops, what the actual command was and what the output of that command is. This +allows the terminal to expose quick actions for: + +* Quickly navigating the history by scrolling between commands +* Re-running a previous command in the history +* Copying all the output of a single command +* A visual indicator to separate out one command-line from the next, for quicker + mental parsing of the output of the command-line. +* Collapsing the output of a command, as to reduce noise +* Visual indicators that highlight commands that succeeded or failed. +* Jumping to previously used directories + +### User Stories + +This is a bit of a unusual section, as this feature was already partially +implemented when this spec was written. + +Story | Size | Description +--|-----------|-- +A | ✅ Done | The user can use mark each prompt and have a mark displayed on the scrollbar +B | ✅ Done | The user can perform an action to scroll between marks +C | ✅ Done | The user can manually add marks to the buffer +D | ✅ Done | The shell can emit different marks to differentiate between prompt, command, and output +E | ✅ Done | Clearing the buffer clears marks +F | 🐣 Crawl | Marks stay in the same place you'd expect after resizing the buffer. +G | ✅ Done | Users can perform an action to select the previous command's output +H | 🚶 Walk | The find dialog can display marks on the scrollbar indicating the position of search matches +I | 🏃‍♂️ Run | The terminal can display icons in the gutter, with quick actions for that command (re-run, copy output, etc) +J | 🏃‍♂️ Run | The terminal can display a faint horizontal separator between commands in the buffer. +K | 🚀 Sprint | The terminal can "collapse" content between two marks. +L | 🚀 Sprint | The terminal can display a sticky header above the control which displays the most recent command +M | 🚀 Sprint | The user can open a dialog to manually bookmark a line with a custom comment + +## Solution Design + +### Supported VT sequences + +* [x] iTerm2's OSC `SetMark` (in [#12948]) +* [x] FinalTerm prompt markup sequences + - [x] **FTCS_PROMPT** was added in [#13163] + - [x] The rest in [#14341] +* [ ] additionally, VsCode's FinalTerm prompt markup variant, `OSC 663` +* [ ] [ConEmu's + `OSC9;12`](https://conemu.github.io/en/AnsiEscapeCodes.html#ConEmu_specific_OSC) +* [ ] Any custom OSC we may want to author ourselves. + +The FinalTerm prompt sequences are probably the most complicated version of all +these, so it's important to give these a special callout. Almost all the other +VT sequences are roughly equivalent to **FTCS_PROMPT**. The xterm.js / VsCode +version has additional cases, that they ironically added to work around conpty +not understanding these sequences originally. + +#### FinalTerm sequences + +The relevant FinalTerm sequences for marking up the prompt are as follows. + + +![image](FTCS-diagram.png) + +* **FTCS_PROMPT**: `OSC 133 ; A ST` + - The start of a prompt. Internally, this sets a marker in the buffer + indicating we started a prompt at the current cursor position, and that + marker will be used when we get a **FTCS_COMMAND_START** +* **FTCS_COMMAND_START**: `OSC 133 ; B ST` + - The start of a commandline (READ: the end of the prompt). When it follows a + **FTCS_PROMPT**, it creates a mark in the buffer from the location of the + **FTCS_PROMPT** to the current cursor position, with the category of + `prompt` +* **FTCS_COMMAND_EXECUTED**: `OSC 133 ; C ST` + - The start of the command output / the end of the commandline. +* **FTCS_COMMAND_FINISHED**: `OSC 133 ; D ; [Ps] ST` + - the end of a command. + +Same deal for the **FTCS_COMMAND_EXECUTED**/**FTCS_COMMAND_FINISHED** ones. +**FTCS_COMMAND_EXECUTED** does nothing until we get a **FTCS_COMMAND_FINISHED**, +and the `[Ps]` parameter determines the category. + - `[Ps] == 0`: success + - anything else: error + +This whole sequence will get turned into a single mark. + +When we get the **FTCS_COMMAND_FINISHED**, set the category of the prompt mark +that preceded it, so that the `prompt` becomes an `error` or a `success`. + + +### Buffer implementation + +In the initial PR ([#12948]), marks were stored simply as a `vector`, +where a mark had a start and end point. These wouldn't reflow on resize, and +didn't support all of the FTCS sequences. + +There's ultimately three types of region here we need to mark: +* The prompt (starting from A) +* the command (starting from B) +* the output (starting from C) + +That intuitively feels a bit like a text attribute almost. Additionally, the +prompt should be connected to its subsequent command and output, s.t. we can +* Select command output +* re-run command + +easily. Supposedly, we could do this by iterating through the whole buffer to +find the previous/next {whatever}[[1](#footnote-1)]. Additionally, the prompt +needs to be able to contain the status / category, and a `133;D` needs to be +able to change the category of the previous prompt/command. + +If we instead do a single mark for each command, from `133;A` to `133;A`, and +have sub-points for elements within the command +* `133;A` starts a mark on the current line, at the current position. +* `133;B` sets the end of the mark to the current position. +* `133;C` updates the mark's `commandStart` to the current end, then sets the + end of the mark to the current position. +* `133;D` updates the mark's `outputStart` to the current end, then sets the end + of the mark to the current position. It also updates the category of the mark, + if needed. + +Each command then only shows up as a single mark on the scrollbar. Jumping +between commands is easy, `scrollToMark` operates on `mark.start`, which is +where the prompt started. "Bookmarks", i.e. things started by the user wouldn't +have `commandStart` or `outputStart` in them. Getting the text of the command, +of the output is easy - it's just the text between sub-points. + +Reflow still sucks though - we'd need to basically iterate over all the marks as +we're reflowing, to make sure we put them into the right place in the new +buffer. This is annoying and tedious, but shouldn't realistically be a +performance problem. + +#### Cmd.exe considerations + +`cmd.exe` is generally a pretty bad shell, and doesn't have a lot of the same +hooks that other shells do, that might allow for us to emit the +**FTCS_COMMAND_EXECUTED** sequence. However, cmd.exe also doesn't allow +multiline prompts, so we can be relatively certain that when the user presses +enter, that's the end of the prompt. We will treat the +`autoMarkPrompts` setting (which auto-marks enter) as the _end of the +prompt_. That would at least allow cmd.exe to emit a {command finished}{prompt +start}{prompt...}{command start} in the prompt, and have us add the command +executed. It is not perfect (we wouldn't be able to get error information), but +it does work well enough. + +```cmd +PROMPT $e]133;D$e\$e]133;A$e\$e]9;9;$P$e\%PROMPT%$e]133;B$e\ +``` + +## Settings proposals + +The below are the proposed additions to the settings for supporting marks and +interacting with them. Some of these have already been added as experimental +settings - these would be promoted to no longer be experimental. + +Many of the sub-points on these settings are definitely "Future Consideration" +level settings. For example, the `scrollToMark` `"highlight"` property. That one +is certainly not something we need to ship with. + +### Actions + +In addition to driving marks via the output, we will also want to support adding +marks manually. These can be thought of like "bookmarks" - a user indicated +region that means something to the user. + +* [ ] `addMark`: add a mark to the buffer. If there's a selection, place the + mark covering at the selection. Otherwise, place the mark on the cursor row. + - [x] `color`: a color for the scrollbar mark. (in [#12948]) + - [ ] `category`: one of `{"prompt", "error", "warning", "success", "info"}` +* [ ] `scrollToMark` + - [x] `direction`: `["first", "previous", "next", "last"]` (in [#12948]) + - [ ] `category`: `flags({categories}...)`, default `"all"`. Only scroll to + one of the categories specified (e.g. only scroll to the previous error, + only the previous prompt, or just any mark) + - [ ] [#13449] - `center` or some other setting that controls how the mark is scrolled in. + - Maybe `top` (current) /`center` (as proposed) /`nearestEdge` (when + scrolling down, put the mark at the bottom line of viewport , up -> top + line)? + - [ ] [#13455] - `highlight`: `bool`, default true. Display a temporary + highlight around the mark when scrolling to it. ("Highlight" != "select") + - If the mark has prompt/command/output sections, only select the prompt and command. + - If the mark has zero width (i.e. the user just wanted to bookmark a line), + then highlight the entire row. +* [x] `clearMark`: Remove any marks in the selected region (or at the cursor + position) (in [#12948]) +* [x] `clearAllMarks`: Remove all the marks from the buffer. (in [#12948]) + + +#### Selecting commands & output + +_Inspired by a long weekend of manually copying .csv output from the Terminal to +a spreadsheet, only to discover that we rejected [#4588] some years ago._ + +* [x] `selectCommand(direction=[prev, next])`: Starting at the active selection + anchor, (or the cursor if there's no selection), select the command that + starts before/after this point (exclusive). Probably shouldn't wrap around + the buffer. + * Since this will create a selection including the start of the command, + performing this again will select the _next_ command (in whatever + direction). +* [x] `selectOutput(direction=[prev, next])`: same as above, but with command outputs. + +A convenient workflow might be a `multipleActions([selectOutput(prev), +copy()])`, to quickly select the previous commands output. + +### Per-profile settings + +* [x] `autoMarkPrompts`: `bool`, default `false`. (in [#12948]) +* [ ] `showFindMatchesOnScrollbar`: `bool`, default `true`. +* [ ] `showMarksOnScrollbar`: `bool` or `flags({categories}...)` + * As an example: `"showMarksOnScrollbar": ["error", "success"]`). + * Controls if marks should be displayed on the scrollbar. + * If `true`/`"all"`, then all marks are displayed. + * If `false`/`"none"`, then no marks are displayed. + * If a set of categories are provided, only display marks from those categories. + * [x] the bool version is (in [#12948]) + * [ ] The `flags({categories}...)` version is not done yet. +* [ ] `showGutterIcons`, for displaying gutter icons. + +## UX Design + +An example of what colored marks look like: + +![Select the entire output of a command](https://user-images.githubusercontent.com/18356694/207696859-a227abe2-ccd4-4b81-8a2c-8a22219cd0dd.gif) + +This gif demos both prompt marks and marks for search results: + +![](https://user-images.githubusercontent.com/18356694/191330278-3f6bc207-1bd5-4ebd-bb0e-1f84b0170f49.gif) + + +### Gutter icons + +![](vscode-shell-integration-gutter-mark.png) +_An example of what the icons in the VsCode gutter look like_ + +### Multiple marks on the same line + +When it comes to displaying marks on the scrollbar, or in the margins, the +relative priority of these marks matters. Marks are given the following +priority, with errors being the highest priority. +* Error +* Warning +* Success +* Prompt +* Info (default) + +## Work needed to get marks to v1 + +* [x] Clearing the screen leaves marks behind + * [x] Make sure `ED2` works to clear/move marks + * [x] Same with `ED3` + * [x] Same with `cls` / `Clear-Host` + * [x] Clear Buffer action too. +* [X] Circling doesn't update scrollbar + * I think this was fixed in [#14341], or in [#14045] +* [ ] Resizing / reflowing marks +* [x] marks should be stored in the `TextBuffer` + +## Future Considerations +* adding a timestamp for when a line was marked? +* adding a comment to the mark. How do we display that comment? a TeachingTip on + the scrollbar maybe (actually that's a cool idea) +* adding a shape to the mark? Terminal.app marks prompt lines with brackets in + the margins +* Marks are currently just displayed as "last one in wins", they should have a + real sort order +* Should the height of a mark on the scrollbar be dependent on font size & + buffer height? I think I've got it set to like, a static 2dip, but maybe it + should represent the actual height of the row (down to a min 1dip) +* [#13455] - highlight a mark when scrolled to with the `scrollToMark` action. + This is left as a future consideration to figure out what kind of UI we want + here. Do we want to highlight + - the prompt? + - the whole row of the prompt? + - the prompt and the command? + - The whole prompt & command & output? +* an `addBookmark` action: This one's basically just `addMark`, but opens a prompt + (like the window renamer) to add some text as a comment. Automatically + populated with the selected text (if there was some). + - A dedicated transient pane for displaying non-terminal content might be + useful for such a thing. + - This might just make more sense as a parameter to `addMark`. +* Other ideas for `addMark` parameters: + - `icon`: This would require us to better figure out how we display gutter + icons. This would probably be like, a _shape_ rather than an arbitrary + image. + - `note`: a note to stick on the mark, as a comment. Might be more valuable + with something like `addBookmark`. + +### Gutter icons + +VsCode implements a set of gutter icons to the left of the buffer lines, to +provide a UI element for exposing some quick actions to perform, powered by +shell integration. + +Gutter icons don't need to implement app-level actions at all. They _should_ be +part of the control. At least, part of the UWP `TermControl`. These are some +basic "actions" we could add to that menu. Since these are all attached to a +mark anyways, we already know what mark the user interacted with, and where the +start/end already is. +* Copy command +* Copy output +* Re-run command +* Save as task +* Explain this (for errors) + +To allow comments in marks (ala "bookmarks"), we can use +the gutter flyout to display the comment, and have the tooltip display that +comment. + +This is being left as a future consideration for now. We need to really sit and +consider what the UX is like for this. +* Do we just stick the gutter icons in the margin/padding? +* Have it be a separate space in the "buffer" + - If it's in the buffer itself, we can render it with the renderer, which by + all accounts, we probably should. + +### Edge cases where these might not work as expected + +Much of the benefits of shell integration come from literal buffer text parsing. +This can lead to some rough edge cases, such as: + +* the user presses Ctrl+VEscape to input an ESC character + and the shell displays it as `^[` +* the user presses Ctrl+VCtrl+J to input an LF character + and the shell displays it as a line break +* the user presses Enter within a quoted string and the shell + displays a continuation prompt +* the user types a command including an exclamation point, and the shell invokes + history expansion and echoes the result of expansion before it runs the + command +* The user has a prompt with a right-aligned status, ala + ![](https://user-images.githubusercontent.com/189190/254475719-5007df07-6cc3-42e8-baf7-2572579eb2b9.png) + +In these cases, the effects of shell integration will likely not work as +intended. There are various possible solutions that are being explored. We might +want to in the future also use [VsCode's extension to the FTCS sequences] to +enable the shell to tell the terminal the literal resulting commandline. + +There's been [other proposals] to extend shell integration features as well. + +### Rejected ideas + +There was originally some discussion as to whether this is a design that should +be unified with generic pattern matchers. Something like the URL detection, +which identifies part of the buffer and then "marks" it. Prototypes for both of +these features are going in very different directions, however. Likely best to +leave them separate. + +## Resources + +### Other related issues + +Not necessarily marks related, but could happily leverage this functionality. + +* [#5916] and [#12366], which are likely to be combined into a single thread + - Imagine a trigger that automatically detects `error:.*` and then marks the line +* [#9583] + - Imagine selecting some text, colorizing & marking it all at once + - `addMark(selection:false)`, above, was inspired by this. +* [#7561] (and broadly, [#3920]) + - Search results should maybe show up here on the scrollbar too. +* [#13455] +* [#13449] +* [#4588] +* [#14754] - A "sticky header" for the `TermControl` that could display the + previous command at the top of the buffer. +* [#2226] - a scrollable "minimap" in te scrollbar, as opposed to marks + +### Relevant external docs +* **GREAT** summary of the state of the ecosystem: https://gitlab.freedesktop.org/terminal-wg/specifications/-/issues/28 +* https://iterm2.com/documentation-escape-codes.html + - `OSC 1337 ; SetMark ST` under "Set Mark" + - under "Shell Integration/FinalTerm +* https://support.apple.com/en-ca/guide/terminal/trml135fbc26/mac + - discusses auto-marked lines on `enter`/`^C`/`^D` + - allows bookmarking lines with selection + - bookmarks can have a name (maybe not super important) +* [How to Use Marks in OS X’s Terminal for Easier Navigation](https://www.howtogeek.com/256548/how-to-use-marks-in-os-xs-terminal-for-easier-navigation/) +* [Terminal: The ‘\[‘ Marks the Spot](https://scriptingosx.com/2017/03/terminal-the-marks-the-spot/) +* Thread with VsCode (xterm.js) implementation notes: https://github.com/microsoft/terminal/issues/1527#issuecomment-1076455642 +* [xterm.js prompt markup sequences](https://github.com/microsoft/vscode/blob/39cc1e1c42b2e53e83b1846c2857ca194848cc1d/src/vs/workbench/contrib/terminal/browser/xterm/shellIntegrationAddon.ts#L50-L52) +* [VsCode command tracking release notes](https://code.visualstudio.com/updates/v1_22#_command-tracking), also [Terminal shell integration](https://code.visualstudio.com/updates/v1_65#_terminal-shell-integration) +* ConEMU: + Sequence | Description + -- | -- + ESC ] 9 ; 12 ST | Let ConEmu treat current cursor position as prompt start. Useful with PS1. + +* https://iterm2.com/documentation-one-page.html#documentation-triggers.html" + +### Footnotes + +[1]: Intuitively, this feels prohibitively expensive, +but you'd be mistaken. + +An average device right now (I mean something that was alright about 5 years +ago, like an 8700k with regular DDR4) does about 4GB/s of random, un-cached +memory access. While low-end devices are probably a bit slower, I think 4GB/s is +a good estimate regardless. That's because non-random memory access is way way +faster than that at around 20GB/s (DDR4 2400 - what most laptops had for the +last decade). + +Assuming a 120 column * 32k line buffer (our current maximum), the buffer would +be about 21MB large. Going through the entire buffer linearly at 20GB/s would +take just 1ms (including all text and metadata). If we assume that each row has +a mark, that marks are 36 byte large and assuming the worst case of random +access, we can go through all 32k within about 0.3ms. + + + +_(Thanks lhecker for these notes)_ + + +[#1527]: https://github.com/microsoft/terminal/issues/1527 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#1527]: https://github.com/microsoft/terminal/issues/1527 +[#6232]: https://github.com/microsoft/terminal/issues/6232 +[#2226]: https://github.com/microsoft/terminal/issues/2226 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#13163]: https://github.com/microsoft/terminal/issues/13163 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#13455]: https://github.com/microsoft/terminal/issues/13455 +[#13449]: https://github.com/microsoft/terminal/issues/13449 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#4588]: https://github.com/microsoft/terminal/issues/4588 +[#5804]: https://github.com/microsoft/terminal/issues/5804 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#12948]: https://github.com/microsoft/terminal/issues/12948 +[#13455]: https://github.com/microsoft/terminal/issues/13455 +[#5916]: https://github.com/microsoft/terminal/issues/5916 +[#12366]: https://github.com/microsoft/terminal/issues/12366 +[#9583]: https://github.com/microsoft/terminal/issues/9583 +[#7561]: https://github.com/microsoft/terminal/issues/7561 +[#3920]: https://github.com/microsoft/terminal/issues/3920 +[#13455]: https://github.com/microsoft/terminal/issues/13455 +[#13449]: https://github.com/microsoft/terminal/issues/13449 +[#4588]: https://github.com/microsoft/terminal/issues/4588 +[#14341]: https://github.com/microsoft/terminal/issues/14341 +[#14045]: https://github.com/microsoft/terminal/issues/14045 +[#14754]: https://github.com/microsoft/terminal/issues/14754 +[#14341]: https://github.com/microsoft/terminal/issues/14341 + +[VsCode's extension to the FTCS sequences]: https://code.visualstudio.com/docs/terminal/shell-integration#_vs-code-custom-sequences-osc-633-st + +[other proposals]: https://gitlab.freedesktop.org/terminal-wg/specifications/-/merge_requests/6#f6de1e5703f5806d0821d92b0274e895c4b6d850 diff --git a/doc/specs/#11000 - Marks/ftcs-diagram.png b/doc/specs/#11000 - Marks/ftcs-diagram.png new file mode 100644 index 00000000000..f36112e0ea3 Binary files /dev/null and b/doc/specs/#11000 - Marks/ftcs-diagram.png differ diff --git a/doc/specs/#11000 - Marks/vscode-shell-integration-gutter-mark.png b/doc/specs/#11000 - Marks/vscode-shell-integration-gutter-mark.png new file mode 100644 index 00000000000..d73a5c7b14a Binary files /dev/null and b/doc/specs/#11000 - Marks/vscode-shell-integration-gutter-mark.png differ diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index c9269f08c43..8ab633361fd 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -441,11 +441,20 @@ void TextBuffer::_PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute } } -void TextBuffer::ConsumeGrapheme(std::wstring_view& chars) noexcept +// Given the character offset `position` in the `chars` string, this function returns the starting position of the next grapheme. +// For instance, given a `chars` of L"x\uD83D\uDE42y" and a `position` of 1 it'll return 3. +// GraphemePrev would do the exact inverse of this operation. +// In the future, these functions are expected to also deliver information about how many columns a grapheme occupies. +// (I know that mere UTF-16 code point iteration doesn't handle graphemes, but that's what we're working towards.) +size_t TextBuffer::GraphemeNext(const std::wstring_view& chars, size_t position) noexcept { - // This function is supposed to mirror the behavior of ROW::Write, when it reads characters off of `chars`. - // (I know that a UTF-16 code point is not a grapheme, but that's what we're working towards.) - chars = til::utf16_pop(chars); + return til::utf16_iterate_next(chars, position); +} + +// It's the counterpart to GraphemeNext. See GraphemeNext. +size_t TextBuffer::GraphemePrev(const std::wstring_view& chars, size_t position) noexcept +{ + return til::utf16_iterate_prev(chars, position); } // This function is intended for writing regular "lines" of text as it'll set the wrap flag on the given row. diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 49c8bc5f29e..855aaca7ff3 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -131,8 +131,10 @@ class TextBuffer final TextBufferTextIterator GetTextLineDataAt(const til::point at) const; TextBufferTextIterator GetTextDataAt(const til::point at, const Microsoft::Console::Types::Viewport limit) const; + static size_t GraphemeNext(const std::wstring_view& chars, size_t position) noexcept; + static size_t GraphemePrev(const std::wstring_view& chars, size_t position) noexcept; + // Text insertion functions - static void ConsumeGrapheme(std::wstring_view& chars) noexcept; void Write(til::CoordType row, const TextAttribute& attributes, RowWriteState& state); void FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes); diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 3b33f8b5897..cad581983f3 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -1021,6 +1021,16 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } + void TerminalPage::_HandleDisplayWorkingDirectory(const IInspectable& /*sender*/, + const ActionEventArgs& args) + { + if (_settings.GlobalSettings().DebugFeaturesEnabled()) + { + ShowTerminalWorkingDirectory(); + args.Handled(true); + } + } + void TerminalPage::_HandleSearchForText(const IInspectable& /*sender*/, const ActionEventArgs& args) { diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 1a20db37695..80f0a9df574 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -216,7 +216,7 @@ Web Search - + Color... @@ -836,4 +836,8 @@ Moves tab to a new window - + + Run as Administrator + This text is displayed on context menu for profile entries in add new tab button. + + \ No newline at end of file diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index b38a0d82daa..8140af22b3e 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1057,6 +1057,15 @@ namespace winrt::TerminalApp::implementation } }); + // Using the static method on the base class seems to do what we want in terms of placement. + WUX::Controls::Primitives::FlyoutBase::SetAttachedFlyout(profileMenuItem, _CreateRunAsAdminFlyout(profileIndex)); + + // Since we are not setting the ContextFlyout property of the item we have to handle the ContextRequested event + // and rely on the base class to show our menu. + profileMenuItem.ContextRequested([profileMenuItem](auto&&, auto&&) { + WUX::Controls::Primitives::FlyoutBase::ShowAttachedFlyout(profileMenuItem); + }); + return profileMenuItem; } @@ -4105,6 +4114,33 @@ namespace winrt::TerminalApp::implementation } } + winrt::fire_and_forget TerminalPage::ShowTerminalWorkingDirectory() + { + auto weakThis{ get_weak() }; + co_await wil::resume_foreground(Dispatcher()); + if (auto page{ weakThis.get() }) + { + // If we haven't ever loaded the TeachingTip, then do so now and + // create the toast for it. + if (page->_windowCwdToast == nullptr) + { + if (auto tip{ page->FindName(L"WindowCwdToast").try_as() }) + { + page->_windowCwdToast = std::make_shared(tip); + // Make sure to use the weak ref when setting up this + // callback. + tip.Closed({ page->get_weak(), &TerminalPage::_FocusActiveControl }); + } + } + _UpdateTeachingTipTheme(WindowCwdToast().try_as()); + + if (page->_windowCwdToast != nullptr) + { + page->_windowCwdToast->Open(); + } + } + } + // Method Description: // - Called when the user hits the "Ok" button on the WindowRenamer TeachingTip. // - Will raise an event that will bubble up to the monarch, asking if this @@ -4932,4 +4968,42 @@ namespace winrt::TerminalApp::implementation // _RemoveTab will make sure to null out the _stashed.draggedTab _RemoveTab(*_stashed.draggedTab); } + + /// + /// Creates a sub flyout menu for profile items in the split button menu that when clicked will show a menu item for + /// Run as Administrator + /// + /// The index for the profileMenuItem + /// MenuFlyout that will show when the context is request on a profileMenuItem + WUX::Controls::MenuFlyout TerminalPage::_CreateRunAsAdminFlyout(int profileIndex) + { + // Create the MenuFlyout and set its placement + WUX::Controls::MenuFlyout profileMenuItemFlyout{}; + profileMenuItemFlyout.Placement(WUX::Controls::Primitives::FlyoutPlacementMode::BottomEdgeAlignedRight); + + // Create the menu item and an icon to use in the menu + WUX::Controls::MenuFlyoutItem runAsAdminItem{}; + WUX::Controls::FontIcon adminShieldIcon{}; + + adminShieldIcon.Glyph(L"\xEA18"); + adminShieldIcon.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); + + runAsAdminItem.Icon(adminShieldIcon); + runAsAdminItem.Text(RS_(L"RunAsAdminFlyout/Text")); + + // Click handler for the flyout item + runAsAdminItem.Click([profileIndex, weakThis{ get_weak() }](auto&&, auto&&) { + if (auto page{ weakThis.get() }) + { + NewTerminalArgs args{ profileIndex }; + args.Elevate(true); + page->_OpenNewTerminalViaDropdown(args); + } + }); + + profileMenuItemFlyout.Items().Append(runAsAdminItem); + + return profileMenuItemFlyout; + } + } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index bde39cca453..7a6866e20ec 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -147,6 +147,7 @@ namespace winrt::TerminalApp::implementation winrt::fire_and_forget IdentifyWindow(); winrt::fire_and_forget RenameFailed(); + winrt::fire_and_forget ShowTerminalWorkingDirectory(); winrt::fire_and_forget ProcessStartupActions(Windows::Foundation::Collections::IVector actions, const bool initial, @@ -256,6 +257,7 @@ namespace winrt::TerminalApp::implementation std::shared_ptr _windowIdToast{ nullptr }; std::shared_ptr _windowRenameFailedToast{ nullptr }; + std::shared_ptr _windowCwdToast{ nullptr }; winrt::Windows::UI::Xaml::Controls::TextBox::LayoutUpdated_revoker _renamerLayoutUpdatedRevoker; int _renamerLayoutCount{ 0 }; @@ -530,7 +532,7 @@ namespace winrt::TerminalApp::implementation void _ContextMenuOpened(const IInspectable& sender, const IInspectable& args); void _SelectionMenuOpened(const IInspectable& sender, const IInspectable& args); void _PopulateContextMenu(const IInspectable& sender, const bool withSelection); - + winrt::Windows::UI::Xaml::Controls::MenuFlyout _CreateRunAsAdminFlyout(int profileIndex); #pragma region ActionHandlers // These are all defined in AppActionHandlers.cpp #define ON_ALL_ACTIONS(action) DECLARE_ACTION_HANDLER(action); diff --git a/src/cascadia/TerminalApp/TerminalPage.xaml b/src/cascadia/TerminalApp/TerminalPage.xaml index ade311fcde7..00fb12f86b4 100644 --- a/src/cascadia/TerminalApp/TerminalPage.xaml +++ b/src/cascadia/TerminalApp/TerminalPage.xaml @@ -205,5 +205,11 @@ Text="{x:Bind WindowProperties.WindowName, Mode=OneWay}" /> + + diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.xaml b/src/cascadia/TerminalSettingsEditor/Appearances.xaml index ec20d9e418b..e37cdd2e1bf 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.xaml +++ b/src/cascadia/TerminalSettingsEditor/Appearances.xaml @@ -59,8 +59,7 @@ - - - + + + - + - - + + - + - + diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp index 757b440c82c..7c9802c07cb 100644 --- a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp @@ -72,6 +72,7 @@ static constexpr std::string_view IdentifyWindowKey{ "identifyWindow" }; static constexpr std::string_view IdentifyWindowsKey{ "identifyWindows" }; static constexpr std::string_view RenameWindowKey{ "renameWindow" }; static constexpr std::string_view OpenWindowRenamerKey{ "openWindowRenamer" }; +static constexpr std::string_view DisplayWorkingDirectoryKey{ "debugTerminalCwd" }; static constexpr std::string_view SearchForTextKey{ "searchWeb" }; static constexpr std::string_view GlobalSummonKey{ "globalSummon" }; static constexpr std::string_view QuakeModeKey{ "quakeMode" }; @@ -404,6 +405,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { ShortcutAction::IdentifyWindows, RS_(L"IdentifyWindowsCommandKey") }, { ShortcutAction::RenameWindow, RS_(L"ResetWindowNameCommandKey") }, { ShortcutAction::OpenWindowRenamer, RS_(L"OpenWindowRenamerCommandKey") }, + { ShortcutAction::DisplayWorkingDirectory, RS_(L"DisplayWorkingDirectoryCommandKey") }, { ShortcutAction::GlobalSummon, MustGenerate }, { ShortcutAction::SearchForText, MustGenerate }, { ShortcutAction::QuakeMode, RS_(L"QuakeModeCommandKey") }, diff --git a/src/cascadia/TerminalSettingsModel/AllShortcutActions.h b/src/cascadia/TerminalSettingsModel/AllShortcutActions.h index 571c20632a4..53a260f46ea 100644 --- a/src/cascadia/TerminalSettingsModel/AllShortcutActions.h +++ b/src/cascadia/TerminalSettingsModel/AllShortcutActions.h @@ -85,6 +85,7 @@ ON_ALL_ACTIONS(IdentifyWindows) \ ON_ALL_ACTIONS(RenameWindow) \ ON_ALL_ACTIONS(OpenWindowRenamer) \ + ON_ALL_ACTIONS(DisplayWorkingDirectory) \ ON_ALL_ACTIONS(SearchForText) \ ON_ALL_ACTIONS(GlobalSummon) \ ON_ALL_ACTIONS(QuakeMode) \ diff --git a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw index 1cd8b5d51f5..8de5d841734 100644 --- a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw @@ -511,6 +511,9 @@ Rename window... + + Display Terminal's current working directory + Show/Hide the Terminal window diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 9f267746511..76065f93c95 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -2891,8 +2891,7 @@ void ConptyRoundtripTests::TestResizeWithCookedRead() m_state->PrepareReadHandle(); // TODO GH#5618: This string will get mangled, but we don't really care // about the buffer contents in this test, so it doesn't really matter. - const std::string_view cookedReadContents{ "This is some cooked read data" }; - m_state->PrepareCookedReadData(cookedReadContents); + m_state->PrepareCookedReadData(L"This is some cooked read data"); Log::Comment(L"Painting the frame"); VERIFY_SUCCEEDED(renderer.PaintFrame()); diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 34b69947e54..1de5441ab94 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -962,8 +962,6 @@ winrt::fire_and_forget AppHost::_peasantNotifyActivateWindow() // - The window layout as a json string. winrt::Windows::Foundation::IAsyncOperation AppHost::_GetWindowLayoutAsync() { - winrt::apartment_context peasant_thread; - winrt::hstring layoutJson = L""; // Use the main thread since we are accessing controls. co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher()); @@ -974,9 +972,6 @@ winrt::Windows::Foundation::IAsyncOperation AppHost::_GetWindowL } CATCH_LOG() - // go back to give the result to the peasant. - co_await peasant_thread; - co_return layoutJson; } @@ -1052,9 +1047,6 @@ void AppHost::_DisplayWindowId(const winrt::Windows::Foundation::IInspectable& / winrt::fire_and_forget AppHost::_RenameWindowRequested(const winrt::Windows::Foundation::IInspectable /*sender*/, const winrt::TerminalApp::RenameWindowRequestedArgs args) { - // Capture calling context. - winrt::apartment_context ui_thread; - // Switch to the BG thread - anything x-proc must happen on a BG thread co_await winrt::resume_background(); @@ -1064,12 +1056,9 @@ winrt::fire_and_forget AppHost::_RenameWindowRequested(const winrt::Windows::Fou _peasant.RequestRename(requestArgs); - // Switch back to the UI thread. Setting the WindowName needs to happen - // on the UI thread, because it'll raise a PropertyChanged event - co_await ui_thread; - if (requestArgs.Succeeded()) { + co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher()); _windowLogic.WindowName(args.ProposedName()); } else diff --git a/src/cascadia/WindowsTerminal_UIATests/Elements/TerminalApp.cs b/src/cascadia/WindowsTerminal_UIATests/Elements/TerminalApp.cs index 291c608e087..ac944525e98 100644 --- a/src/cascadia/WindowsTerminal_UIATests/Elements/TerminalApp.cs +++ b/src/cascadia/WindowsTerminal_UIATests/Elements/TerminalApp.cs @@ -49,7 +49,15 @@ public TerminalApp(TestContext context, string shellToLaunch = "powershell.exe") { this.context = context; - // If running locally, set WTPath to where we can find a loose deployment of Windows Terminal + // If running locally, set WTPath to where we can find a loose + // deployment of Windows Terminal. That means you'll need to build + // the Terminal appx, then use + // New-UnpackagedTerminalDistribution.ps1 to build an unpackaged + // layout that can successfully launch. Then, point the tests at + // that WindowsTerminal.exe like so: + // + // te.exe WindowsTerminal.UIA.Tests.dll /p:WTPath=C:\the\path\to\the\unpackaged\layout\WindowsTerminal.exe + // // On the build machines, the scripts lay it out at the terminal-0.0.1.0\ subfolder of the test deployment directory string path = Path.GetFullPath(Path.Combine(context.TestDeploymentDir, @"terminal-0.0.1.0\WindowsTerminal.exe")); if (context.Properties.Contains("WTPath")) diff --git a/src/host/ApiRoutines.h b/src/host/ApiRoutines.h index 3492ae234a8..a46f1397ec6 100644 --- a/src/host/ApiRoutines.h +++ b/src/host/ApiRoutines.h @@ -57,27 +57,17 @@ class ApiRoutines : public IApiRoutines const bool IsPeek, std::unique_ptr& waiter) noexcept override; - [[nodiscard]] HRESULT ReadConsoleAImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept override; - - [[nodiscard]] HRESULT ReadConsoleWImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept override; + [[nodiscard]] HRESULT ReadConsoleImpl(IConsoleInputObject& context, + std::span buffer, + size_t& written, + std::unique_ptr& waiter, + const std::wstring_view initialData, + const std::wstring_view exeName, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool IsUnicode, + const HANDLE clientHandle, + const DWORD controlWakeupMask, + DWORD& controlKeyState) noexcept override; [[nodiscard]] HRESULT WriteConsoleAImpl(IConsoleOutputObject& context, const std::string_view buffer, diff --git a/src/host/CommandListPopup.cpp b/src/host/CommandListPopup.cpp index 62abadb379d..b530ca2760d 100644 --- a/src/host/CommandListPopup.cpp +++ b/src/host/CommandListPopup.cpp @@ -34,9 +34,9 @@ static til::size calculatePopupSize(const CommandHistory& history) // find the widest command history item and use it for the width size_t width = minSize.width; - for (size_t i = 0; i < history.GetNumberOfCommands(); ++i) + for (CommandHistory::Index i = 0; i < history.GetNumberOfCommands(); ++i) { - const auto& historyItem = history.GetNth(gsl::narrow(i)); + const auto& historyItem = history.GetNth(i); width = std::max(width, historyItem.size() + padding); } if (width > SHRT_MAX) @@ -53,7 +53,7 @@ static til::size calculatePopupSize(const CommandHistory& history) CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const CommandHistory& history) : Popup(screenInfo, calculatePopupSize(history)), _history{ history }, - _currentCommand{ std::min(history.LastDisplayed, static_cast(history.GetNumberOfCommands() - 1)) } + _currentCommand{ std::min(history.LastDisplayed, history.GetNumberOfCommands() - 1) } { FAIL_FAST_IF(_currentCommand < 0); _setBottomIndex(); @@ -65,7 +65,7 @@ CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const Command { try { - short Index = 0; + CommandHistory::Index Index = 0; const auto shiftPressed = WI_IsFlagSet(modifiers, SHIFT_PRESSED); switch (wch) { @@ -107,17 +107,17 @@ CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const Command break; case VK_END: // Move waaay forward, UpdateCommandListPopup() can handle it. - _update((SHORT)(cookedReadData.History().GetNumberOfCommands())); + _update(cookedReadData.History().GetNumberOfCommands()); break; case VK_HOME: // Move waaay back, UpdateCommandListPopup() can handle it. - _update(-(SHORT)(cookedReadData.History().GetNumberOfCommands())); + _update(-cookedReadData.History().GetNumberOfCommands()); break; case VK_PRIOR: - _update(-(SHORT)Height()); + _update(-Height()); break; case VK_NEXT: - _update((SHORT)Height()); + _update(Height()); break; case VK_DELETE: return _deleteSelection(cookedReadData); @@ -125,7 +125,7 @@ CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const Command case VK_RIGHT: Index = _currentCommand; CommandLine::Instance().EndCurrentPopup(); - SetCurrentCommandLine(cookedReadData, (SHORT)Index); + SetCurrentCommandLine(cookedReadData, Index); return CONSOLE_STATUS_WAIT_NO_BLOCK; default: break; @@ -137,13 +137,13 @@ CommandListPopup::CommandListPopup(SCREEN_INFORMATION& screenInfo, const Command void CommandListPopup::_setBottomIndex() { - if (_currentCommand < (SHORT)(_history.GetNumberOfCommands() - Height())) + if (_currentCommand < _history.GetNumberOfCommands() - Height()) { - _bottomIndex = std::max(_currentCommand, gsl::narrow(Height() - 1)); + _bottomIndex = std::max(_currentCommand, Height() - 1); } else { - _bottomIndex = (SHORT)(_history.GetNumberOfCommands() - 1); + _bottomIndex = _history.GetNumberOfCommands() - 1; } } @@ -152,7 +152,7 @@ void CommandListPopup::_setBottomIndex() try { auto& history = cookedReadData.History(); - history.Remove(static_cast(_currentCommand)); + history.Remove(_currentCommand); _setBottomIndex(); if (history.GetNumberOfCommands() == 0) @@ -160,9 +160,9 @@ void CommandListPopup::_setBottomIndex() // close the popup return CONSOLE_STATUS_READ_COMPLETE; } - else if (_currentCommand >= static_cast(history.GetNumberOfCommands())) + else if (_currentCommand >= history.GetNumberOfCommands()) { - _currentCommand = static_cast(history.GetNumberOfCommands() - 1); + _currentCommand = history.GetNumberOfCommands() - 1; _bottomIndex = _currentCommand; } @@ -204,7 +204,7 @@ void CommandListPopup::_setBottomIndex() { auto& history = cookedReadData.History(); - if (history.GetNumberOfCommands() <= 1 || _currentCommand == gsl::narrow(history.GetNumberOfCommands()) - 1) + if (history.GetNumberOfCommands() <= 1 || _currentCommand == history.GetNumberOfCommands() - 1) { return STATUS_SUCCESS; } @@ -218,12 +218,12 @@ void CommandListPopup::_setBottomIndex() void CommandListPopup::_handleReturn(COOKED_READ_DATA& cookedReadData) { - short Index = 0; + CommandHistory::Index Index = 0; auto Status = STATUS_SUCCESS; DWORD LineCount = 1; Index = _currentCommand; CommandLine::Instance().EndCurrentPopup(); - SetCurrentCommandLine(cookedReadData, (SHORT)Index); + SetCurrentCommandLine(cookedReadData, Index); cookedReadData.ProcessInput(UNICODE_CARRIAGERETURN, 0, Status); // complete read if (cookedReadData.IsEchoInput()) @@ -268,13 +268,13 @@ void CommandListPopup::_handleReturn(COOKED_READ_DATA& cookedReadData) void CommandListPopup::_cycleSelectionToMatchingCommands(COOKED_READ_DATA& cookedReadData, const wchar_t wch) { - short Index = 0; + CommandHistory::Index Index = 0; if (cookedReadData.History().FindMatchingCommand({ &wch, 1 }, _currentCommand, Index, CommandHistory::MatchOptions::JustLooking)) { - _update((SHORT)(Index - _currentCommand), true); + _update(Index - _currentCommand, true); } } @@ -345,7 +345,7 @@ void CommandListPopup::_drawList() auto api = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().api; WriteCoord.y = _region.top + 1; - auto i = gsl::narrow(std::max(_bottomIndex - Height() + 1, 0)); + auto i = std::max(_bottomIndex - Height() + 1, 0); for (; i <= _bottomIndex; i++) { CHAR CommandNumber[COMMAND_NUMBER_SIZE]; @@ -447,7 +447,7 @@ void CommandListPopup::_drawList() // Arguments: // - originalDelta - The number of lines to move up or down // - wrap - Down past the bottom or up past the top should wrap the command list -void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) +void CommandListPopup::_update(const CommandHistory::Index originalDelta, const bool wrap) { auto delta = originalDelta; if (delta == 0) @@ -457,7 +457,7 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) const auto Size = Height(); auto CurCmdNum = _currentCommand; - SHORT NewCmdNum = CurCmdNum + delta; + CommandHistory::Index NewCmdNum = CurCmdNum + delta; if (wrap) { @@ -466,9 +466,9 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) } else { - if (NewCmdNum >= gsl::narrow(_history.GetNumberOfCommands())) + if (NewCmdNum >= _history.GetNumberOfCommands()) { - NewCmdNum = gsl::narrow(_history.GetNumberOfCommands()) - 1; + NewCmdNum = _history.GetNumberOfCommands() - 1; } else if (NewCmdNum < 0) { @@ -484,16 +484,16 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) _bottomIndex += delta; if (_bottomIndex < Size - 1) { - _bottomIndex = gsl::narrow(Size - 1); + _bottomIndex = Size - 1; } Scroll = true; } else if (NewCmdNum > _bottomIndex) { _bottomIndex += delta; - if (_bottomIndex >= gsl::narrow(_history.GetNumberOfCommands())) + if (_bottomIndex >= _history.GetNumberOfCommands()) { - _bottomIndex = gsl::narrow(_history.GetNumberOfCommands()) - 1; + _bottomIndex = _history.GetNumberOfCommands() - 1; } Scroll = true; } @@ -516,7 +516,7 @@ void CommandListPopup::_update(const SHORT originalDelta, const bool wrap) // Arguments: // - OldCurrentCommand - The previous command highlighted // - NewCurrentCommand - The new command to be highlighted. -void CommandListPopup::_updateHighlight(const SHORT OldCurrentCommand, const SHORT NewCurrentCommand) +void CommandListPopup::_updateHighlight(const CommandHistory::Index OldCurrentCommand, const CommandHistory::Index NewCurrentCommand) { til::CoordType TopIndex; if (_bottomIndex < Height()) diff --git a/src/host/CommandListPopup.hpp b/src/host/CommandListPopup.hpp index a893b905554..59bef4c3946 100644 --- a/src/host/CommandListPopup.hpp +++ b/src/host/CommandListPopup.hpp @@ -29,8 +29,8 @@ class CommandListPopup : public Popup private: void _drawList(); - void _update(const SHORT delta, const bool wrap = false); - void _updateHighlight(const SHORT oldCommand, const SHORT newCommand); + void _update(const CommandHistory::Index delta, const bool wrap = false); + void _updateHighlight(const CommandHistory::Index oldCommand, const CommandHistory::Index newCommand); void _handleReturn(COOKED_READ_DATA& cookedReadData); void _cycleSelectionToMatchingCommands(COOKED_READ_DATA& cookedReadData, const wchar_t wch); @@ -40,8 +40,8 @@ class CommandListPopup : public Popup [[nodiscard]] NTSTATUS _swapUp(COOKED_READ_DATA& cookedReadData) noexcept; [[nodiscard]] NTSTATUS _swapDown(COOKED_READ_DATA& cookedReadData) noexcept; - SHORT _currentCommand; - SHORT _bottomIndex; // number of command displayed on last line of popup + CommandHistory::Index _currentCommand; + CommandHistory::Index _bottomIndex; // number of command displayed on last line of popup const CommandHistory& _history; #ifdef UNIT_TESTING diff --git a/src/host/CommandNumberPopup.cpp b/src/host/CommandNumberPopup.cpp index 4ccce4d6d75..6e990439d70 100644 --- a/src/host/CommandNumberPopup.cpp +++ b/src/host/CommandNumberPopup.cpp @@ -101,8 +101,7 @@ void CommandNumberPopup::_handleEscape(COOKED_READ_DATA& cookedReadData) noexcep // - cookedReadData - read data to operate on void CommandNumberPopup::_handleReturn(COOKED_READ_DATA& cookedReadData) noexcept { - const auto commandNumber = gsl::narrow(std::min(static_cast(_parse()), - cookedReadData.History().GetNumberOfCommands() - 1)); + const auto commandNumber = gsl::narrow(std::min(_parse(), cookedReadData.History().GetNumberOfCommands() - 1)); CommandLine::Instance().EndAllPopups(); SetCurrentCommandLine(cookedReadData, commandNumber); diff --git a/src/host/VtApiRoutines.cpp b/src/host/VtApiRoutines.cpp index ff71f8c3a5b..fe7d0b7060e 100644 --- a/src/host/VtApiRoutines.cpp +++ b/src/host/VtApiRoutines.cpp @@ -119,42 +119,19 @@ void VtApiRoutines::_SynchronizeCursor(std::unique_ptr& waiter) no return hr; } -[[nodiscard]] HRESULT VtApiRoutines::ReadConsoleAImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept -{ - const auto hr = m_pUsualRoutines->ReadConsoleAImpl(context, buffer, written, waiter, initialData, exeName, readHandleState, clientHandle, controlWakeupMask, controlKeyState); - // If we're about to tell the caller to wait, let's synchronize the cursor we have with what - // the terminal is presenting in case there's a cooked read going on. - // TODO GH10001: we only need to do this in cooked read mode. - if (clientHandle) - { - m_listeningForDSR = true; - (void)m_pVtEngine->_ListenForDSR(); - (void)m_pVtEngine->RequestCursor(); - } - return hr; -} - -[[nodiscard]] HRESULT VtApiRoutines::ReadConsoleWImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept -{ - const auto hr = m_pUsualRoutines->ReadConsoleWImpl(context, buffer, written, waiter, initialData, exeName, readHandleState, clientHandle, controlWakeupMask, controlKeyState); +[[nodiscard]] HRESULT VtApiRoutines::ReadConsoleImpl(IConsoleInputObject& context, + std::span buffer, + size_t& written, + std::unique_ptr& waiter, + const std::wstring_view initialData, + const std::wstring_view exeName, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool IsUnicode, + const HANDLE clientHandle, + const DWORD controlWakeupMask, + DWORD& controlKeyState) noexcept +{ + const auto hr = m_pUsualRoutines->ReadConsoleImpl(context, buffer, written, waiter, initialData, exeName, readHandleState, IsUnicode, clientHandle, controlWakeupMask, controlKeyState); // If we're about to tell the caller to wait, let's synchronize the cursor we have with what // the terminal is presenting in case there's a cooked read going on. // TODO GH10001: we only need to do this in cooked read mode. diff --git a/src/host/VtApiRoutines.h b/src/host/VtApiRoutines.h index 8c3cbf917ef..7dc77f78b09 100644 --- a/src/host/VtApiRoutines.h +++ b/src/host/VtApiRoutines.h @@ -60,27 +60,17 @@ class VtApiRoutines : public IApiRoutines const bool IsPeek, std::unique_ptr& waiter) noexcept override; - [[nodiscard]] HRESULT ReadConsoleAImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept override; - - [[nodiscard]] HRESULT ReadConsoleWImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept override; + [[nodiscard]] HRESULT ReadConsoleImpl(IConsoleInputObject& context, + std::span buffer, + size_t& written, + std::unique_ptr& waiter, + const std::wstring_view initialData, + const std::wstring_view exeName, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool IsUnicode, + const HANDLE clientHandle, + const DWORD controlWakeupMask, + DWORD& controlKeyState) noexcept override; [[nodiscard]] HRESULT WriteConsoleAImpl(IConsoleOutputObject& context, const std::string_view buffer, diff --git a/src/host/cmdline.cpp b/src/host/cmdline.cpp index 4b48f4bd24b..1e1587e37fc 100644 --- a/src/host/cmdline.cpp +++ b/src/host/cmdline.cpp @@ -27,51 +27,6 @@ #pragma hdrstop using Microsoft::Console::Interactivity::ServiceLocator; -// Routine Description: -// - This routine validates a string buffer and returns the pointers of where the strings start within the buffer. -// Arguments: -// - Unicode - Supplies a boolean that is TRUE if the buffer contains Unicode strings, FALSE otherwise. -// - Buffer - Supplies the buffer to be validated. -// - Size - Supplies the size, in bytes, of the buffer to be validated. -// - Count - Supplies the expected number of strings in the buffer. -// ... - Supplies a pair of arguments per expected string. The first one is the expected size, in bytes, of the string -// and the second one receives a pointer to where the string starts. -// Return Value: -// - TRUE if the buffer is valid, FALSE otherwise. -bool IsValidStringBuffer(_In_ bool Unicode, _In_reads_bytes_(Size) PVOID Buffer, _In_ ULONG Size, _In_ ULONG Count, ...) -{ - va_list Marker; - va_start(Marker, Count); - - while (Count > 0) - { - const auto StringSize = va_arg(Marker, ULONG); - const auto StringStart = va_arg(Marker, PVOID*); - - // Make sure the string fits in the supplied buffer and that it is properly aligned. - if (StringSize > Size) - { - break; - } - - if (Unicode && (StringSize % sizeof(WCHAR)) != 0) - { - break; - } - - *StringStart = Buffer; - - // Go to the next string. - Buffer = RtlOffsetToPointer(Buffer, StringSize); - Size -= StringSize; - Count -= 1; - } - - va_end(Marker); - - return Count == 0; -} - // Routine Description: // - Detects Word delimiters bool IsWordDelim(const wchar_t wch) @@ -274,7 +229,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData) // Routine Description: // - This routine copies the commandline specified by Index into the cooked read buffer -void SetCurrentCommandLine(COOKED_READ_DATA& cookedReadData, _In_ SHORT Index) // index, not command number +void SetCurrentCommandLine(COOKED_READ_DATA& cookedReadData, _In_ CommandHistory::Index Index) // index, not command number { DeleteCommandLine(cookedReadData, TRUE); FAIL_FAST_IF_FAILED(cookedReadData.History().RetrieveNth(Index, @@ -983,7 +938,7 @@ til::point CommandLine::_cycleMatchingCommandHistoryToPrompt(COOKED_READ_DATA& c auto cursorPosition = cookedReadData.ScreenInfo().GetTextBuffer().GetCursor().GetPosition(); if (cookedReadData.HasHistory()) { - SHORT index; + CommandHistory::Index index; if (cookedReadData.History().FindMatchingCommand({ cookedReadData.BufferStartPtr(), cookedReadData.InsertionPoint() }, cookedReadData.History().LastDisplayed, index, @@ -993,7 +948,7 @@ til::point CommandLine::_cycleMatchingCommandHistoryToPrompt(COOKED_READ_DATA& c const auto CurrentPos = cookedReadData.InsertionPoint(); DeleteCommandLine(cookedReadData, true); - THROW_IF_FAILED(cookedReadData.History().RetrieveNth((SHORT)index, + THROW_IF_FAILED(cookedReadData.History().RetrieveNth(index, cookedReadData.SpanWholeBuffer(), cookedReadData.BytesRead())); FAIL_FAST_IF(!(cookedReadData.BufferStartPtr() == cookedReadData.BufferCurrentPtr())); diff --git a/src/host/cmdline.h b/src/host/cmdline.h index a8669edadde..a8a5d19ca5b 100644 --- a/src/host/cmdline.h +++ b/src/host/cmdline.h @@ -1,56 +1,5 @@ -/*++ -Copyright (c) Microsoft Corporation -Licensed under the MIT license. - -Module Name: -- cmdline.h - -Abstract: -- This file contains the internal structures and definitions used by command line input and editing. - -Author: -- Therese Stowell (ThereseS) 15-Nov-1991 - -Revision History: -- Mike Griese (migrie) Jan 2018: - Refactored the history and alias functionality into their own files. -- Michael Niksa (miniksa) May 2018: - Split apart popup information. Started encapsulating command line things. Removed 0 length buffers. -Notes: - The input model for the command line editing popups is complex. - Here is the relevant pseudocode: - - CookedReadWaitRoutine - if (CookedRead->Popup) - Status = (*CookedRead->Popup->Callback)(); - if (Status == CONSOLE_STATUS_READ_COMPLETE) - return STATUS_SUCCESS; - return Status; - - CookedRead - if (Command Line Editing Key) - ProcessCommandLine - else - process regular key - - ProcessCommandLine - if F7 - return Popup - - Popup - draw popup - return ProcessCommandListInput - - ProcessCommandListInput - while (TRUE) - GetChar - if (wait) - return wait - switch (char) - . - . - . ---*/ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. #pragma once @@ -138,6 +87,4 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData); bool IsWordDelim(const wchar_t wch); bool IsWordDelim(const std::wstring_view charData); -bool IsValidStringBuffer(_In_ bool Unicode, _In_reads_bytes_(Size) PVOID Buffer, _In_ ULONG Size, _In_ ULONG Count, ...); - -void SetCurrentCommandLine(COOKED_READ_DATA& cookedReadData, _In_ SHORT Index); +void SetCurrentCommandLine(COOKED_READ_DATA& cookedReadData, _In_ CommandHistory::Index Index); diff --git a/src/host/conddkrefs.h b/src/host/conddkrefs.h index 486825ed8ab..854d03e8081 100644 --- a/src/host/conddkrefs.h +++ b/src/host/conddkrefs.h @@ -139,8 +139,6 @@ typedef struct _FILE_FS_DEVICE_INFORMATION #pragma region ntifs.h(public DDK) -#define RtlOffsetToPointer(B, O) ((PCHAR)(((PCHAR)(B)) + ((ULONG_PTR)(O)))) - __kernel_entry NTSYSCALLAPI NTSTATUS NTAPI diff --git a/src/host/ft_host/Message_KeyPressTests.cpp b/src/host/ft_host/Message_KeyPressTests.cpp index 53d8b9dade8..b11f53ed0bf 100644 --- a/src/host/ft_host/Message_KeyPressTests.cpp +++ b/src/host/ft_host/Message_KeyPressTests.cpp @@ -103,6 +103,100 @@ class KeyPressTests TEST_METHOD_PROPERTY(L"Ignore[default]", L"true") END_TEST_METHOD() + TEST_METHOD(TestInvalidKeyPressIsIgnored) + { + if (!OneCoreDelay::IsSendMessageWPresent()) + { + Log::Comment(L"Injecting keys to the window message queue cannot be done on systems without a classic window message queue. Skipping."); + Log::Result(WEX::Logging::TestResults::Skipped); + return; + } + + Log::Comment(L"Testing that key events with an invalid virtual keycode and an invalid scan code are properly ignored, and not put into the input buffer"); + BOOL successBool; + auto hwnd = GetConsoleWindow(); + VERIFY_IS_TRUE(!!IsWindow(hwnd)); + auto inputHandle = GetStdHandle(STD_INPUT_HANDLE); + DWORD events = 0; + + // flush input buffer + FlushConsoleInputBuffer(inputHandle); + successBool = GetNumberOfConsoleInputEvents(inputHandle, &events); + VERIFY_IS_TRUE(!!successBool); + VERIFY_ARE_EQUAL(events, 0u); + + WPARAM vKey = 0xFF; + BYTE scanCode = 0; + WORD repeatCount = 1; + + LPARAM lParam = (scanCode << 16) | repeatCount; + + // Send the keypress + SendMessage(hwnd, WM_KEYDOWN, vKey, lParam); + + // make sure the keypress got ignored + events = 0; + successBool = GetNumberOfConsoleInputEvents(inputHandle, &events); + VERIFY_IS_TRUE(!!successBool); + VERIFY_ARE_EQUAL(events, 0u); + auto inputBuffer = std::make_unique(1); + PeekConsoleInput(inputHandle, + inputBuffer.get(), + 1, + &events); + VERIFY_ARE_EQUAL(events, 0u); + } + + TEST_METHOD(TestKeyPressWithScanCodeZero) + { + if (!OneCoreDelay::IsSendMessageWPresent()) + { + Log::Comment(L"Injecting keys to the window message queue cannot be done on systems without a classic window message queue. Skipping."); + Log::Result(WEX::Logging::TestResults::Skipped); + return; + } + + Log::Comment(L"Testing that key events with a valid keycode and an invalid scancode (0) are properly processed."); + BOOL successBool; + auto hwnd = GetConsoleWindow(); + VERIFY_IS_TRUE(!!IsWindow(hwnd)); + auto inputHandle = GetStdHandle(STD_INPUT_HANDLE); + DWORD events = 0; + + // flush input buffer + FlushConsoleInputBuffer(inputHandle); + successBool = GetNumberOfConsoleInputEvents(inputHandle, &events); + VERIFY_IS_TRUE(!!successBool); + VERIFY_ARE_EQUAL(events, 0u); + + WPARAM vKey = VK_LWIN; + BYTE scanCode = 0; // Conhost should convert this to the correct scan code + WORD repeatCount = 1; + + LPARAM lParam = (scanCode << 16) | repeatCount; + + // Send the keypress + SendMessage(hwnd, WM_KEYDOWN, vKey, lParam); + + // Make sure the keypress got processed. + events = 0; + successBool = GetNumberOfConsoleInputEvents(inputHandle, &events); + VERIFY_IS_TRUE(!!successBool); + VERIFY_ARE_EQUAL(events, 1u); + auto inputBuffer = std::make_unique(1); + PeekConsoleInput(inputHandle, + inputBuffer.get(), + 1, + &events); + VERIFY_ARE_EQUAL(events, 1u); + VERIFY_ARE_EQUAL(inputBuffer[0].EventType, KEY_EVENT); + VERIFY_ARE_EQUAL(inputBuffer[0].Event.KeyEvent.wRepeatCount, 1, NoThrowString().Format(L"%d", inputBuffer[0].Event.KeyEvent.wRepeatCount)); + // Scan code should be set to the correct value. + VERIFY_ARE_EQUAL(inputBuffer[0].Event.KeyEvent.wVirtualScanCode, VK_LWIN); + // 'VK_LWIN' is an enhanced key, so the ENHANCED_KEY bit should be set. + VERIFY_IS_TRUE(inputBuffer[0].Event.KeyEvent.dwControlKeyState & ENHANCED_KEY); + } + TEST_METHOD(TestCoalesceSameKeyPress) { if (!OneCoreDelay::IsSendMessageWPresent()) @@ -125,7 +219,7 @@ class KeyPressTests VERIFY_IS_TRUE(!!successBool); VERIFY_ARE_EQUAL(events, 0u); - // send a bunch of 'a' keypresses to the console + // send a bunch of 'a' keypresses to the console. DWORD repeatCount = 1; const unsigned int messageSendCount = 1000; for (unsigned int i = 0; i < messageSendCount; ++i) @@ -133,7 +227,7 @@ class KeyPressTests SendMessage(hwnd, WM_CHAR, 0x41, - repeatCount); + repeatCount); // WM_CHAR doesn't use scan code } // make sure the keypresses got processed and coalesced diff --git a/src/host/history.cpp b/src/host/history.cpp index 501b33e0ee2..dfe1624987a 100644 --- a/src/host/history.cpp +++ b/src/host/history.cpp @@ -62,13 +62,14 @@ void CommandHistory::s_Free(const HANDLE processHandle) void CommandHistory::s_ResizeAll(const size_t commands) { + const auto size = gsl::narrow(commands); + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - FAIL_FAST_IF(commands > SHORT_MAX); gci.SetHistoryBufferSize(gsl::narrow(commands)); for (auto& historyList : s_historyLists) { - historyList.Realloc(commands); + historyList.Realloc(size); } } @@ -81,7 +82,7 @@ bool CommandHistory::IsAppNameMatch(const std::wstring_view other) const // - This routine is called when escape is entered or a command is added. void CommandHistory::_Reset() { - LastDisplayed = gsl::narrow(_commands.size()) - 1; + LastDisplayed = GetNumberOfCommands() - 1; WI_SetFlag(Flags, CLE_RESET); } @@ -109,7 +110,7 @@ void CommandHistory::_Reset() if (suppressDuplicates) { - SHORT index; + Index index; if (FindMatchingCommand(newCommand, LastDisplayed, index, CommandHistory::MatchOptions::ExactMatch)) { reuse = Remove(index); @@ -117,7 +118,7 @@ void CommandHistory::_Reset() } // find free record. if all records are used, free the lru one. - if ((SHORT)_commands.size() == _maxCommands) + if (GetNumberOfCommands() == _maxCommands) { _commands.erase(_commands.cbegin()); // move LastDisplayed back one in order to stay synced with the @@ -152,27 +153,28 @@ void CommandHistory::_Reset() return S_OK; } -std::wstring_view CommandHistory::GetNth(const SHORT index) const +std::wstring_view CommandHistory::GetNth(Index index) const { - try + if (index >= 0 && index < GetNumberOfCommands()) { return _commands.at(index); } - CATCH_LOG(); - return {}; } -[[nodiscard]] HRESULT CommandHistory::RetrieveNth(const SHORT index, - std::span buffer, - size_t& commandSize) +const std::vector& CommandHistory::GetCommands() const noexcept +{ + return _commands; +} + +[[nodiscard]] HRESULT CommandHistory::RetrieveNth(const Index index, std::span buffer, size_t& commandSize) { LastDisplayed = index; try { const auto& cmd = _commands.at(index); - if (cmd.size() > (size_t)buffer.size()) + if (cmd.size() > buffer.size()) { commandSize = buffer.size(); // room for CRLF? } @@ -229,16 +231,7 @@ std::wstring_view CommandHistory::GetNth(const SHORT index) const std::wstring_view CommandHistory::GetLastCommand() const { - if (_commands.size() != 0) - { - try - { - return _commands.at(LastDisplayed); - } - CATCH_LOG(); - } - - return {}; + return GetNth(LastDisplayed); } void CommandHistory::Empty() @@ -255,54 +248,44 @@ bool CommandHistory::AtFirstCommand() const return FALSE; } - auto i = (SHORT)(LastDisplayed - 1); + auto i = LastDisplayed - 1; if (i == -1) { - i = ((SHORT)_commands.size()) - 1i16; + i = GetNumberOfCommands() - 1; } - return (i == ((SHORT)_commands.size()) - 1i16); + return (i == GetNumberOfCommands() - 1); } bool CommandHistory::AtLastCommand() const { - return LastDisplayed == ((SHORT)_commands.size()) - 1i16; + return LastDisplayed == GetNumberOfCommands() - 1; } -void CommandHistory::Realloc(const size_t commands) +void CommandHistory::Realloc(const Index commands) { - // To protect ourselves from overflow and general arithmetic errors, a limit of SHORT_MAX is put on the size of the command history. - if (_maxCommands == (SHORT)commands || commands > SHORT_MAX) + if (_maxCommands == commands) { return; } - const auto oldCommands = _commands; - const auto newNumberOfCommands = gsl::narrow(std::min(_commands.size(), commands)); - - _commands.clear(); - for (SHORT i = 0; i < newNumberOfCommands; i++) - { - _commands.emplace_back(oldCommands[i]); - } + _commands.resize(std::min(_commands.size(), gsl::narrow_cast(std::max(0, commands)))); WI_SetFlag(Flags, CLE_RESET); - LastDisplayed = gsl::narrow(_commands.size()) - 1; - _maxCommands = (SHORT)commands; + LastDisplayed = GetNumberOfCommands() - 1; + _maxCommands = commands; } void CommandHistory::s_ReallocExeToFront(const std::wstring_view appName, const size_t commands) { - for (auto it = s_historyLists.begin(); it != s_historyLists.end(); it++) + const auto size = gsl::narrow(commands); + + for (auto it = s_historyLists.begin(), end = s_historyLists.end(); it != end; ++it) { if (WI_IsFlagSet(it->Flags, CLE_ALLOCATED) && it->IsAppNameMatch(appName)) { - auto backup = *it; - backup.Realloc(commands); - - s_historyLists.erase(it); - s_historyLists.push_front(backup); - + it->Realloc(size); + s_historyLists.splice(s_historyLists.begin(), s_historyLists, it); return; } } @@ -336,19 +319,20 @@ CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, cons auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // Reuse a history buffer. The buffer must be !CLE_ALLOCATED. // If possible, the buffer should have the same app name. - std::optional BestCandidate; + const auto beg = s_historyLists.begin(); + const auto end = s_historyLists.end(); + auto BestCandidate = end; auto SameApp = false; - for (auto it = s_historyLists.cbegin(); it != s_historyLists.cend(); it++) + for (auto it = beg; it != end; ++it) { if (WI_IsFlagClear(it->Flags, CLE_ALLOCATED)) { // use MRU history buffer with same app name if (it->IsAppNameMatch(appName)) { - BestCandidate = *it; + BestCandidate = it; SameApp = true; - s_historyLists.erase(it); break; } } @@ -363,7 +347,7 @@ CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, cons History._appName = appName; History.Flags = CLE_ALLOCATED; History.LastDisplayed = -1; - History._maxCommands = gsl::narrow(gci.GetHistoryBufferSize()); + History._maxCommands = gsl::narrow(gci.GetHistoryBufferSize()); History._processHandle = processHandle; return &s_historyLists.emplace_front(History); } @@ -371,28 +355,22 @@ CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, cons // If we have no candidate already and we need one, // take the LRU (which is the back/last one) which isn't allocated // and if possible the one with empty commands list. - if (!BestCandidate.has_value()) + if (BestCandidate == end) { - auto BestCandidateIt = s_historyLists.cend(); - for (auto it = s_historyLists.cbegin(); it != s_historyLists.cend(); it++) + for (auto it = beg; it != end; ++it) { if (WI_IsFlagClear(it->Flags, CLE_ALLOCATED)) { - if (it->_commands.empty() || BestCandidateIt == s_historyLists.cend() || !BestCandidateIt->_commands.empty()) + if (it->_commands.empty() || BestCandidate == end || !BestCandidate->_commands.empty()) { - BestCandidateIt = it; + BestCandidate = it; } } } - if (BestCandidateIt != s_historyLists.cend()) - { - BestCandidate = *BestCandidateIt; - s_historyLists.erase(BestCandidateIt); - } } // If the app name doesn't match, copy in the new app name and free the old commands. - if (BestCandidate.has_value()) + if (BestCandidate != end) { if (!SameApp) { @@ -404,36 +382,38 @@ CommandHistory* CommandHistory::s_Allocate(const std::wstring_view appName, cons BestCandidate->_processHandle = processHandle; WI_SetFlag(BestCandidate->Flags, CLE_ALLOCATED); - return &s_historyLists.emplace_front(BestCandidate.value()); + // move to the front of the list + s_historyLists.splice(beg, s_historyLists, BestCandidate); + return &*BestCandidate; } return nullptr; } -size_t CommandHistory::GetNumberOfCommands() const +CommandHistory::Index CommandHistory::GetNumberOfCommands() const { - return _commands.size(); + return gsl::narrow_cast(_commands.size()); } -void CommandHistory::_Prev(SHORT& ind) const +void CommandHistory::_Prev(Index& ind) const { if (ind <= 0) { - ind = gsl::narrow(_commands.size()); + ind = GetNumberOfCommands(); } ind--; } -void CommandHistory::_Next(SHORT& ind) const +void CommandHistory::_Next(Index& ind) const { ++ind; - if (ind >= (SHORT)_commands.size()) + if (ind >= GetNumberOfCommands()) { ind = 0; } } -void CommandHistory::_Dec(SHORT& ind) const +void CommandHistory::_Dec(Index& ind) const { if (ind <= 0) { @@ -442,7 +422,7 @@ void CommandHistory::_Dec(SHORT& ind) const ind--; } -void CommandHistory::_Inc(SHORT& ind) const +void CommandHistory::_Inc(Index& ind) const { ++ind; if (ind >= _maxCommands) @@ -451,64 +431,33 @@ void CommandHistory::_Inc(SHORT& ind) const } } -std::wstring CommandHistory::Remove(const SHORT iDel) +std::wstring CommandHistory::Remove(const Index iDel) { - SHORT iFirst = 0; - auto iLast = gsl::narrow(_commands.size() - 1); - auto iDisp = LastDisplayed; - - if (_commands.size() == 0) + if (iDel < 0 || iDel >= GetNumberOfCommands()) { return {}; } - const auto nDel = iDel; - if ((nDel < iFirst) || (nDel > iLast)) - { - return {}; - } + const auto str = std::move(_commands.at(iDel)); + _commands.erase(_commands.begin() + iDel); - if (iDisp == iDel) + if (LastDisplayed == iDel) { LastDisplayed = -1; } - - try + else if (LastDisplayed > iDel) { - const auto str = _commands.at(iDel); - - if (iDel < iLast) - { - _commands.erase(_commands.cbegin() + iDel); - if ((iDisp > iDel) && (iDisp <= iLast)) - { - _Dec(iDisp); - } - _Dec(iLast); - } - else if (iFirst <= iDel) - { - _commands.erase(_commands.cbegin() + iDel); - if ((iDisp >= iFirst) && (iDisp < iDel)) - { - _Inc(iDisp); - } - _Inc(iFirst); - } - - LastDisplayed = iDisp; - return str; + _Dec(LastDisplayed); } - CATCH_LOG(); - return {}; + return str; } // Routine Description: // - this routine finds the most recent command that starts with the letters already in the current command. it returns the array index (no mod needed). [[nodiscard]] bool CommandHistory::FindMatchingCommand(const std::wstring_view givenCommand, - const SHORT startingIndex, - SHORT& indexFound, + const Index startingIndex, + Index& indexFound, const MatchOptions options) { indexFound = startingIndex; @@ -565,9 +514,15 @@ void CommandHistory::s_ClearHistoryListStorage() // Arguments: // - indexA - index of one history item to swap // - indexB - index of one history item to swap -void CommandHistory::Swap(const short indexA, const short indexB) +void CommandHistory::Swap(const Index indexA, const Index indexB) { - std::swap(_commands.at(indexA), _commands.at(indexB)); + const auto num = GetNumberOfCommands(); + if (indexA != indexB && + indexA >= 0 && indexA < num && + indexB >= 0 && indexB < num) + { + std::swap(_commands.at(indexA), _commands.at(indexB)); + } } // Routine Description: @@ -689,9 +644,8 @@ HRESULT GetConsoleCommandHistoryLengthImplHelper(const std::wstring_view exeName // Every command history item is made of a string length followed by 1 null character. const size_t cchNull = 1; - for (SHORT i = 0; i < gsl::narrow(pCommandHistory->GetNumberOfCommands()); i++) + for (const auto& command : pCommandHistory->GetCommands()) { - const auto command = pCommandHistory->GetNth(i); auto cchCommand = command.size(); // If we're counting how much multibyte space will be needed, trial convert the command string before we add. @@ -796,10 +750,8 @@ HRESULT GetConsoleCommandHistoryWImplHelper(const std::wstring_view exeName, const size_t cchNull = 1; - for (SHORT i = 0; i < gsl::narrow(CommandHistory->GetNumberOfCommands()); i++) + for (const auto& command : CommandHistory->GetCommands()) { - const auto command = CommandHistory->GetNth(i); - const auto cchCommand = command.size(); size_t cchNeeded; diff --git a/src/host/history.h b/src/host/history.h index 9abe6841a95..943f9475e21 100644 --- a/src/host/history.h +++ b/src/host/history.h @@ -1,20 +1,14 @@ -/*++ -Copyright (c) Microsoft Corporation -Licensed under the MIT license. - -Module Name: -- history.h - -Abstract: -- Encapsulates the cmdline functions and structures specifically related to - command history functionality. ---*/ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. #pragma once class CommandHistory { public: + using Index = int32_t; + static constexpr Index IndexMax = INT32_MAX; + // CommandHistory Flags static constexpr int CLE_ALLOCATED = 0x00000001; static constexpr int CLE_RESET = 0x00000002; @@ -41,8 +35,8 @@ class CommandHistory }; bool FindMatchingCommand(const std::wstring_view command, - const SHORT startingIndex, - SHORT& indexFound, + const Index startingIndex, + Index& indexFound, const MatchOptions options); bool IsAppNameMatch(const std::wstring_view other) const; @@ -53,24 +47,25 @@ class CommandHistory const std::span buffer, size_t& commandSize); - [[nodiscard]] HRESULT RetrieveNth(const SHORT index, + [[nodiscard]] HRESULT RetrieveNth(const Index index, const std::span buffer, size_t& commandSize); - size_t GetNumberOfCommands() const; - std::wstring_view GetNth(const SHORT index) const; + Index GetNumberOfCommands() const; + std::wstring_view GetNth(Index index) const; + const std::vector& GetCommands() const noexcept; - void Realloc(const size_t commands); + void Realloc(Index commands); void Empty(); - std::wstring Remove(const SHORT iDel); + std::wstring Remove(const Index iDel); bool AtFirstCommand() const; bool AtLastCommand() const; std::wstring_view GetLastCommand() const; - void Swap(const short indexA, const short indexB); + void Swap(const Index indexA, const Index indexB); private: void _Reset(); @@ -78,22 +73,24 @@ class CommandHistory // _Next and _Prev go to the next and prev command // _Inc and _Dec go to the next and prev slots // Don't get the two confused - it matters when the cmd history is not full! - void _Prev(SHORT& ind) const; - void _Next(SHORT& ind) const; - void _Dec(SHORT& ind) const; - void _Inc(SHORT& ind) const; + void _Prev(Index& ind) const; + void _Next(Index& ind) const; + void _Dec(Index& ind) const; + void _Inc(Index& ind) const; + // NOTE: In conhost v1 this used to be a circular buffer because removal at the + // start is a very common operation. It seems this was lost in the C++ refactor. std::vector _commands; - SHORT _maxCommands; + Index _maxCommands = 0; std::wstring _appName; - HANDLE _processHandle; + HANDLE _processHandle = nullptr; static std::list s_historyLists; public: - DWORD Flags; - SHORT LastDisplayed; + DWORD Flags = 0; + Index LastDisplayed = 0; #ifdef UNIT_TESTING static void s_ClearHistoryListStorage(); diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index 6da7ed899ab..75e57b55963 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -50,7 +50,7 @@ COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, _In_ char* UserBuffer, _In_ ULONG CtrlWakeupMask, _In_ const std::wstring_view exeName, - _In_ const std::string_view initialData, + _In_ const std::wstring_view initialData, _In_ ConsoleProcessHandle* const pClientProcess) : ReadData(pInputBuffer, pInputReadHandleData), _screenInfo{ screenInfo }, @@ -97,11 +97,11 @@ COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, if (!initialData.empty()) { - memcpy_s(_bufPtr, _bufferSize, initialData.data(), initialData.size()); + memcpy_s(_bufPtr, _bufferSize, initialData.data(), initialData.size() * 2); - _bytesRead += initialData.size(); + _bytesRead += initialData.size() * 2; - const auto cchInitialData = initialData.size() / sizeof(wchar_t); + const auto cchInitialData = initialData.size(); VisibleCharCount() = cchInitialData; _bufPtr += cchInitialData; _currentPosition = cchInitialData; diff --git a/src/host/readDataCooked.hpp b/src/host/readDataCooked.hpp index a263303e45d..be2e8645bc1 100644 --- a/src/host/readDataCooked.hpp +++ b/src/host/readDataCooked.hpp @@ -40,7 +40,7 @@ class COOKED_READ_DATA final : public ReadData _In_ char* UserBuffer, _In_ ULONG CtrlWakeupMask, _In_ const std::wstring_view exeName, - _In_ const std::string_view initialData, + _In_ const std::wstring_view initialData, _In_ ConsoleProcessHandle* const pClientProcess); ~COOKED_READ_DATA() override; diff --git a/src/host/stream.cpp b/src/host/stream.cpp index 9ecccd2fadb..c98ef9eaf08 100644 --- a/src/host/stream.cpp +++ b/src/host/stream.cpp @@ -354,7 +354,7 @@ NT_CATCH_RETURN() std::span buffer, size_t& bytesRead, DWORD& controlKeyState, - const std::string_view initialData, + const std::wstring_view initialData, const DWORD ctrlWakeupMask, INPUT_READ_HANDLE_DATA& readHandleState, const std::wstring_view exeName, @@ -492,7 +492,7 @@ NT_CATCH_RETURN() std::span buffer, size_t& bytesRead, ULONG& controlKeyState, - const std::string_view initialData, + const std::wstring_view initialData, const DWORD ctrlWakeupMask, INPUT_READ_HANDLE_DATA& readHandleState, const std::wstring_view exeName, @@ -552,60 +552,29 @@ NT_CATCH_RETURN() CATCH_RETURN(); } -[[nodiscard]] HRESULT ApiRoutines::ReadConsoleAImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept +[[nodiscard]] HRESULT ApiRoutines::ReadConsoleImpl(IConsoleInputObject& context, + std::span buffer, + size_t& written, + std::unique_ptr& waiter, + const std::wstring_view initialData, + const std::wstring_view exeName, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool IsUnicode, + const HANDLE clientHandle, + const DWORD controlWakeupMask, + DWORD& controlKeyState) noexcept { - try - { - return HRESULT_FROM_NT(DoReadConsole(context, - clientHandle, - buffer, - written, - controlKeyState, - initialData, - controlWakeupMask, - readHandleState, - exeName, - false, - waiter)); - } - CATCH_RETURN(); -} - -[[nodiscard]] HRESULT ApiRoutines::ReadConsoleWImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept -{ - try - { - return HRESULT_FROM_NT(DoReadConsole(context, - clientHandle, - buffer, - written, - controlKeyState, - initialData, - controlWakeupMask, - readHandleState, - exeName, - true, - waiter)); - } - CATCH_RETURN(); + return HRESULT_FROM_NT(DoReadConsole(context, + clientHandle, + buffer, + written, + controlKeyState, + initialData, + controlWakeupMask, + readHandleState, + exeName, + IsUnicode, + waiter)); } void UnblockWriteConsole(const DWORD dwReason) diff --git a/src/host/tracing.cpp b/src/host/tracing.cpp index c0547075078..bf54edbed99 100644 --- a/src/host/tracing.cpp +++ b/src/host/tracing.cpp @@ -18,7 +18,7 @@ enum TraceKeywords Output = 0x008, // _DBGOUTPUT General = 0x100, Input = 0x200, - API = 0x400, + //API = 0x400, // No longer used UIA = 0x800, CookedRead = 0x1000, ConsoleAttachDetach = 0x2000, @@ -26,241 +26,8 @@ enum TraceKeywords }; DEFINE_ENUM_FLAG_OPERATORS(TraceKeywords); -// Routine Description: -// - Creates a tracing object to assist with automatically firing a stop event -// when this object goes out of scope. -// - Give it back to the caller and they will release it when the event period is over. -// Arguments: -// - onExit - Function to process when the object is destroyed (on exit) -Tracing::Tracing(std::function onExit) : - _onExit(onExit) -{ -} - -// Routine Description: -// - Destructs a tracing object, running any on exit routine, if necessary. -Tracing::~Tracing() -{ - if (_onExit) - { - _onExit(); - } -} - -// Routine Description: -// - Provides generic tracing for all API call types in the form of -// start/stop period events for timing and region-of-interest purposes -// while doing performance analysis. -// Arguments: -// - result - Reference to the area where the result code from the Api call -// will be stored for use in the stop event. -// - traceName - The name of the API call to list in the trace details -// Return Value: -// - An object for the caller to hold until the API call is complete. -// Then destroy it to signal that the call is over so the stop trace can be written. -Tracing Tracing::s_TraceApiCall(const NTSTATUS result, PCSTR traceName) -{ - // clang-format off - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "ApiCall", - TraceLoggingString(traceName, "ApiName"), - TraceLoggingOpcode(WINEVENT_OPCODE_START), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); - - return Tracing([traceName, &result] { - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "ApiCall", - TraceLoggingString(traceName, "ApiName"), - TraceLoggingHResult(result, "Result"), - TraceLoggingOpcode(WINEVENT_OPCODE_STOP), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); - }); - // clang-format on -} - ULONG Tracing::s_ulDebugFlag = 0x0; -void Tracing::s_TraceApi(const NTSTATUS status, const CONSOLE_GETLARGESTWINDOWSIZE_MSG* const a) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_GetLargestWindowSize", - TraceLoggingHexInt32(status, "ResultCode"), - TraceLoggingInt32(a->Size.X, "MaxWindowWidthInChars"), - TraceLoggingInt32(a->Size.Y, "MaxWindowHeightInChars"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); -} - -void Tracing::s_TraceApi(const NTSTATUS status, const CONSOLE_SCREENBUFFERINFO_MSG* const a, const bool fSet) -{ - // Duplicate copies required by TraceLogging documentation ("don't get cute" examples) - // Using logic inside these macros can make problems. Do all logic outside macros. - - if (fSet) - { - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_SetConsoleScreenBufferInfo", - TraceLoggingHexInt32(status, "ResultCode"), - TraceLoggingInt32(a->Size.X, "BufferWidthInChars"), - TraceLoggingInt32(a->Size.Y, "BufferHeightInChars"), - TraceLoggingInt32(a->CurrentWindowSize.X, "WindowWidthInChars"), - TraceLoggingInt32(a->CurrentWindowSize.Y, "WindowHeightInChars"), - TraceLoggingInt32(a->MaximumWindowSize.X, "MaxWindowWidthInChars"), - TraceLoggingInt32(a->MaximumWindowSize.Y, "MaxWindowHeightInChars"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); - } - else - { - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_GetConsoleScreenBufferInfo", - TraceLoggingHexInt32(status, "ResultCode"), - TraceLoggingInt32(a->Size.X, "BufferWidthInChars"), - TraceLoggingInt32(a->Size.Y, "BufferHeightInChars"), - TraceLoggingInt32(a->CurrentWindowSize.X, "WindowWidthInChars"), - TraceLoggingInt32(a->CurrentWindowSize.Y, "WindowHeightInChars"), - TraceLoggingInt32(a->MaximumWindowSize.X, "MaxWindowWidthInChars"), - TraceLoggingInt32(a->MaximumWindowSize.Y, "MaxWindowHeightInChars"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); - } -} - -void Tracing::s_TraceApi(const NTSTATUS status, const CONSOLE_SETSCREENBUFFERSIZE_MSG* const a) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_SetConsoleScreenBufferSize", - TraceLoggingHexInt32(status, "ResultCode"), - TraceLoggingInt32(a->Size.X, "BufferWidthInChars"), - TraceLoggingInt32(a->Size.Y, "BufferHeightInChars"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); -} - -void Tracing::s_TraceApi(const NTSTATUS status, const CONSOLE_SETWINDOWINFO_MSG* const a) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_SetConsoleWindowInfo", - TraceLoggingHexInt32(status, "ResultCode"), - TraceLoggingBool(a->Absolute, "IsWindowRectAbsolute"), - TraceLoggingInt32(a->Window.Left, "WindowRectLeft"), - TraceLoggingInt32(a->Window.Right, "WindowRectRight"), - TraceLoggingInt32(a->Window.Top, "WindowRectTop"), - TraceLoggingInt32(a->Window.Bottom, "WindowRectBottom"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); -} - -void Tracing::s_TraceApi(_In_ const void* const buffer, const CONSOLE_WRITECONSOLE_MSG* const a) -{ - // clang-format off - if (a->Unicode) - { - const auto buf = static_cast(buffer); - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_WriteConsole", - TraceLoggingBoolean(a->Unicode, "Unicode"), - TraceLoggingUInt32(a->NumBytes, "NumBytes"), - TraceLoggingCountedWideString(buf, static_cast(a->NumBytes / sizeof(wchar_t)), "input buffer"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); - } - else - { - const auto buf = static_cast(buffer); - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_WriteConsole", - TraceLoggingBoolean(a->Unicode, "Unicode"), - TraceLoggingUInt32(a->NumBytes, "NumBytes"), - TraceLoggingCountedString(buf, static_cast(a->NumBytes / sizeof(char)), "input buffer"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); - } - // clang-format on -} - -void Tracing::s_TraceApi(const CONSOLE_SCREENBUFFERINFO_MSG* const a) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_GetConsoleScreenBufferInfo", - TraceLoggingInt16(a->Size.X, "Size.X"), - TraceLoggingInt16(a->Size.Y, "Size.Y"), - TraceLoggingInt16(a->CursorPosition.X, "CursorPosition.X"), - TraceLoggingInt16(a->CursorPosition.Y, "CursorPosition.Y"), - TraceLoggingInt16(a->ScrollPosition.X, "ScrollPosition.X"), - TraceLoggingInt16(a->ScrollPosition.Y, "ScrollPosition.Y"), - TraceLoggingHexUInt16(a->Attributes, "Attributes"), - TraceLoggingInt16(a->CurrentWindowSize.X, "CurrentWindowSize.X"), - TraceLoggingInt16(a->CurrentWindowSize.Y, "CurrentWindowSize.Y"), - TraceLoggingInt16(a->MaximumWindowSize.X, "MaximumWindowSize.X"), - TraceLoggingInt16(a->MaximumWindowSize.Y, "MaximumWindowSize.Y"), - TraceLoggingHexUInt16(a->PopupAttributes, "PopupAttributes"), - TraceLoggingBoolean(a->FullscreenSupported, "FullscreenSupported"), - TraceLoggingHexUInt32FixedArray((UINT32 const*)a->ColorTable, _countof(a->ColorTable), "ColorTable"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); - static_assert(sizeof(UINT32) == sizeof(*a->ColorTable), "a->ColorTable"); -} - -void Tracing::s_TraceApi(const CONSOLE_MODE_MSG* const a, const std::wstring_view handleType) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_GetConsoleMode", - TraceLoggingHexUInt32(a->Mode, "Mode"), - TraceLoggingCountedWideString(handleType.data(), gsl::narrow_cast(handleType.size()), "Handle type"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); -} - -void Tracing::s_TraceApi(const CONSOLE_SETTEXTATTRIBUTE_MSG* const a) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_SetConsoleTextAttribute", - TraceLoggingHexUInt16(a->Attributes, "Attributes"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); -} - -void Tracing::s_TraceApi(const CONSOLE_WRITECONSOLEOUTPUTSTRING_MSG* const a) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "API_WriteConsoleOutput", - TraceLoggingInt16(a->WriteCoord.X, "WriteCoord.X"), - TraceLoggingInt16(a->WriteCoord.Y, "WriteCoord.Y"), - TraceLoggingHexUInt32(a->StringType, "StringType"), - TraceLoggingUInt32(a->NumRecords, "NumRecords"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::API)); -} - void Tracing::s_TraceWindowViewport(const Viewport& viewport) { TraceLoggingWrite( @@ -413,10 +180,10 @@ void Tracing::s_TraceCookedRead(_In_ ConsoleProcessHandle* const pConsoleProcess TraceLoggingWrite( g_hConhostV2EventTraceProvider, "CookedRead", + TraceLoggingPid(pConsoleProcessHandle->dwProcessId, "AttachedProcessId"), TraceLoggingCountedWideString(pwchCookedBuffer, cchCookedBufferLength, "ReadBuffer"), TraceLoggingULong(cchCookedBufferLength, "ReadBufferLength"), - TraceLoggingUInt32(pConsoleProcessHandle->dwProcessId, "AttachedProcessId"), - TraceLoggingUInt64(pConsoleProcessHandle->GetProcessCreationTime(), "AttachedProcessCreationTime"), + TraceLoggingFileTime(pConsoleProcessHandle->GetProcessCreationTime(), "AttachedProcessCreationTime"), TraceLoggingKeyword(TIL_KEYWORD_TRACE), TraceLoggingKeyword(TraceKeywords::CookedRead)); } @@ -431,8 +198,8 @@ void Tracing::s_TraceConsoleAttachDetach(_In_ ConsoleProcessHandle* const pConso TraceLoggingWrite( g_hConhostV2EventTraceProvider, "ConsoleAttachDetach", - TraceLoggingUInt32(pConsoleProcessHandle->dwProcessId, "AttachedProcessId"), - TraceLoggingUInt64(pConsoleProcessHandle->GetProcessCreationTime(), "AttachedProcessCreationTime"), + TraceLoggingPid(pConsoleProcessHandle->dwProcessId, "AttachedProcessId"), + TraceLoggingFileTime(pConsoleProcessHandle->GetProcessCreationTime(), "AttachedProcessCreationTime"), TraceLoggingBool(bIsAttach, "IsAttach"), TraceLoggingBool(bIsUserInteractive, "IsUserInteractive"), TraceLoggingKeyword(TIL_KEYWORD_TRACE), diff --git a/src/host/tracing.hpp b/src/host/tracing.hpp index f1d1e4126a1..8be4bf21b82 100644 --- a/src/host/tracing.hpp +++ b/src/host/tracing.hpp @@ -35,25 +35,14 @@ Author(s): #define DBGOUTPUT(_params_) #endif +#define TraceLoggingConsoleCoord(value, name) \ + TraceLoggingStruct(2, name), \ + TraceLoggingInt32(value.X, "X"), \ + TraceLoggingInt32(value.Y, "Y") + class Tracing { public: - ~Tracing(); - - static Tracing s_TraceApiCall(const NTSTATUS result, PCSTR traceName); - - static void s_TraceApi(const NTSTATUS status, const CONSOLE_GETLARGESTWINDOWSIZE_MSG* const a); - static void s_TraceApi(const NTSTATUS status, const CONSOLE_SCREENBUFFERINFO_MSG* const a, const bool fSet); - static void s_TraceApi(const NTSTATUS status, const CONSOLE_SETSCREENBUFFERSIZE_MSG* const a); - static void s_TraceApi(const NTSTATUS status, const CONSOLE_SETWINDOWINFO_MSG* const a); - - static void s_TraceApi(_In_ const void* const buffer, const CONSOLE_WRITECONSOLE_MSG* const a); - - static void s_TraceApi(const CONSOLE_SCREENBUFFERINFO_MSG* const a); - static void s_TraceApi(const CONSOLE_MODE_MSG* const a, const std::wstring_view handleType); - static void s_TraceApi(const CONSOLE_SETTEXTATTRIBUTE_MSG* const a); - static void s_TraceApi(const CONSOLE_WRITECONSOLEOUTPUTSTRING_MSG* const a); - static void s_TraceWindowViewport(const Microsoft::Console::Types::Viewport& viewport); static void s_TraceChars(_In_z_ const char* pszMessage, ...); @@ -69,8 +58,4 @@ class Tracing private: static ULONG s_ulDebugFlag; - - Tracing(std::function onExit); - - std::function _onExit; }; diff --git a/src/host/ut_host/CommandLineTests.cpp b/src/host/ut_host/CommandLineTests.cpp index b003c37b5f2..de09dd2e241 100644 --- a/src/host/ut_host/CommandLineTests.cpp +++ b/src/host/ut_host/CommandLineTests.cpp @@ -420,7 +420,7 @@ class CommandLineTests auto& commandLine = CommandLine::Instance(); commandLine._deleteCommandHistory(cookedReadData); - VERIFY_ARE_EQUAL(m_pHistory->GetNumberOfCommands(), 0u); + VERIFY_ARE_EQUAL(m_pHistory->GetNumberOfCommands(), 0); } TEST_METHOD(CanFillPromptWithPreviousCommandFragment) diff --git a/src/host/ut_host/CommandListPopupTests.cpp b/src/host/ut_host/CommandListPopupTests.cpp index d47f3bbfa31..342248702db 100644 --- a/src/host/ut_host/CommandListPopupTests.cpp +++ b/src/host/ut_host/CommandListPopupTests.cpp @@ -237,7 +237,7 @@ class CommandListPopupTests VERIFY_ARE_EQUAL(popup.Process(cookedReadData), static_cast(CONSOLE_STATUS_WAIT_NO_BLOCK)); // selection should have moved to the bottom line - VERIFY_ARE_EQUAL(m_pHistory->GetNumberOfCommands() - 1, gsl::narrow(popup._currentCommand)); + VERIFY_ARE_EQUAL(m_pHistory->GetNumberOfCommands() - 1, popup._currentCommand); } TEST_METHOD(HomeMovesSelectionToStart) diff --git a/src/host/ut_host/CommandNumberPopupTests.cpp b/src/host/ut_host/CommandNumberPopupTests.cpp index 8c4460ec8a0..006c36207bd 100644 --- a/src/host/ut_host/CommandNumberPopupTests.cpp +++ b/src/host/ut_host/CommandNumberPopupTests.cpp @@ -174,7 +174,7 @@ class CommandNumberPopupTests TEST_METHOD(CanSelectHistoryItem) { PopupTestHelper::InitHistory(*m_pHistory); - for (unsigned int historyIndex = 0; historyIndex < m_pHistory->GetNumberOfCommands(); ++historyIndex) + for (CommandHistory::Index historyIndex = 0; historyIndex < m_pHistory->GetNumberOfCommands(); ++historyIndex) { Popup::UserInputFunction fn = [historyIndex](COOKED_READ_DATA& /*cookedReadData*/, bool& popupKey, diff --git a/src/host/ut_host/HistoryTests.cpp b/src/host/ut_host/HistoryTests.cpp index 4861fb5dd6c..dbbe6f1c82d 100644 --- a/src/host/ut_host/HistoryTests.cpp +++ b/src/host/ut_host/HistoryTests.cpp @@ -145,15 +145,15 @@ class HistoryTests Log::Comment(L"Retrieve items/order."); std::vector commandsStored; - for (SHORT i = 0; i < (SHORT)history->GetNumberOfCommands(); i++) + for (CommandHistory::Index i = 0; i < history->GetNumberOfCommands(); i++) { commandsStored.emplace_back(history->GetNth(i)); } Log::Comment(L"Reallocate larger and ensure items and order are preserved."); - history->Realloc(_manyHistoryItems.size()); + history->Realloc((CommandHistory::Index)_manyHistoryItems.size()); VERIFY_ARE_EQUAL(s_BufferSize, history->GetNumberOfCommands()); - for (SHORT i = 0; i < (SHORT)commandsStored.size(); i++) + for (CommandHistory::Index i = 0; i < (CommandHistory::Index)commandsStored.size(); i++) { VERIFY_ARE_EQUAL(String(commandsStored[i].data()), String(history->GetNth(i).data())); } @@ -163,7 +163,7 @@ class HistoryTests { VERIFY_SUCCEEDED(history->Add(_manyHistoryItems[j], false)); } - VERIFY_ARE_EQUAL(_manyHistoryItems.size(), history->GetNumberOfCommands()); + VERIFY_ARE_EQUAL((CommandHistory::Index)_manyHistoryItems.size(), history->GetNumberOfCommands()); } TEST_METHOD(ReallocDown) @@ -179,14 +179,14 @@ class HistoryTests Log::Comment(L"Retrieve items/order."); std::vector commandsStored; - for (SHORT i = 0; i < (SHORT)history->GetNumberOfCommands(); i++) + for (CommandHistory::Index i = 0; i < history->GetNumberOfCommands(); i++) { commandsStored.emplace_back(history->GetNth(i)); } Log::Comment(L"Reallocate smaller and ensure items and order are preserved. Items at end of list should be trimmed."); history->Realloc(5); - for (SHORT i = 0; i < 5; i++) + for (CommandHistory::Index i = 0; i < 5; i++) { VERIFY_ARE_EQUAL(String(commandsStored[i].data()), String(history->GetNth(i).data())); } @@ -201,7 +201,7 @@ class HistoryTests VERIFY_SUCCEEDED(history->Add(L"dir", false)); VERIFY_SUCCEEDED(history->Add(L"dir", false)); - VERIFY_ARE_EQUAL(1ul, history->GetNumberOfCommands()); + VERIFY_ARE_EQUAL(1, history->GetNumberOfCommands()); } TEST_METHOD(AddSequentialNoDuplicates) @@ -213,7 +213,7 @@ class HistoryTests VERIFY_SUCCEEDED(history->Add(L"dir", true)); VERIFY_SUCCEEDED(history->Add(L"dir", true)); - VERIFY_ARE_EQUAL(1ul, history->GetNumberOfCommands()); + VERIFY_ARE_EQUAL(1, history->GetNumberOfCommands()); } TEST_METHOD(AddNonsequentialDuplicates) @@ -226,7 +226,7 @@ class HistoryTests VERIFY_SUCCEEDED(history->Add(L"cd", false)); VERIFY_SUCCEEDED(history->Add(L"dir", false)); - VERIFY_ARE_EQUAL(3ul, history->GetNumberOfCommands()); + VERIFY_ARE_EQUAL(3, history->GetNumberOfCommands()); } TEST_METHOD(AddNonsequentialNoDuplicates) @@ -239,7 +239,7 @@ class HistoryTests VERIFY_SUCCEEDED(history->Add(L"cd", false)); VERIFY_SUCCEEDED(history->Add(L"dir", true)); - VERIFY_ARE_EQUAL(2ul, history->GetNumberOfCommands()); + VERIFY_ARE_EQUAL(2, history->GetNumberOfCommands()); } private: @@ -267,7 +267,7 @@ class HistoryTests }; static constexpr UINT s_NumberOfBuffers = 4; - static constexpr UINT s_BufferSize = 10; + static constexpr CommandHistory::Index s_BufferSize = 10; HANDLE _MakeHandle(size_t index) { diff --git a/src/host/ut_host/PopupTestHelper.hpp b/src/host/ut_host/PopupTestHelper.hpp index 146949781e7..e427cd9624b 100644 --- a/src/host/ut_host/PopupTestHelper.hpp +++ b/src/host/ut_host/PopupTestHelper.hpp @@ -44,7 +44,7 @@ class PopupTestHelper final VERIFY_SUCCEEDED(history.Add(L"hear me shout", false)); VERIFY_SUCCEEDED(history.Add(L"here is my handle", false)); VERIFY_SUCCEEDED(history.Add(L"here is my spout", false)); - VERIFY_ARE_EQUAL(history.GetNumberOfCommands(), 4u); + VERIFY_ARE_EQUAL(history.GetNumberOfCommands(), 4); } static void InitLongHistory(CommandHistory& history) noexcept @@ -79,6 +79,6 @@ class PopupTestHelper final VERIFY_SUCCEEDED(history.Add(L"Since then - 'tis Centuries - and yet", false)); VERIFY_SUCCEEDED(history.Add(L"Feels shorter than the Day", false)); VERIFY_SUCCEEDED(history.Add(L"~ Emily Dickinson", false)); - VERIFY_ARE_EQUAL(history.GetNumberOfCommands(), 28u); + VERIFY_ARE_EQUAL(history.GetNumberOfCommands(), 28); } }; diff --git a/src/inc/test/CommonState.hpp b/src/inc/test/CommonState.hpp index 942d0c6007e..9a0e1272b7d 100644 --- a/src/inc/test/CommonState.hpp +++ b/src/inc/test/CommonState.hpp @@ -144,7 +144,7 @@ class CommonState delete gci.pInputBuffer; } - void PrepareCookedReadData(const std::string_view initialData = {}) + void PrepareCookedReadData(const std::wstring_view initialData = {}) { CONSOLE_INFORMATION& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation(); auto* readData = new COOKED_READ_DATA(gci.pInputBuffer, diff --git a/src/inc/til/unicode.h b/src/inc/til/unicode.h index 5191e236c31..9f703dcb813 100644 --- a/src/inc/til/unicode.h +++ b/src/inc/til/unicode.h @@ -59,28 +59,30 @@ namespace til return { ptr, len }; } - // Removes the first code point off of `wstr` and returns the rest. - constexpr std::wstring_view utf16_pop(std::wstring_view wstr) noexcept + // Returns the index of the next codepoint in the given wstr (i.e. after the codepoint that idx points at). + constexpr size_t utf16_iterate_next(const std::wstring_view& wstr, size_t idx) noexcept { - auto it = wstr.begin(); - const auto end = wstr.end(); - - if (it != end) + if (idx < wstr.size() && is_leading_surrogate(til::at(wstr, idx++))) { - const auto wch = *it; - ++it; - - if (is_surrogate(wch)) + if (idx < wstr.size() && is_trailing_surrogate(til::at(wstr, idx))) { - const auto wch2 = it != end ? *it : wchar_t{}; - if (is_leading_surrogate(wch) && is_trailing_surrogate(wch2)) - { - ++it; - } + ++idx; } } + return idx; + } - return { it, end }; + // Returns the index of the preceding codepoint in the given wstr (i.e. in front of the codepoint that idx points at). + constexpr size_t utf16_iterate_prev(const std::wstring_view& wstr, size_t idx) noexcept + { + if (idx > 0 && is_trailing_surrogate(til::at(wstr, --idx))) + { + if (idx > 0 && is_leading_surrogate(til::at(wstr, idx - 1))) + { + --idx; + } + } + return idx; } // Splits a UTF16 string into codepoints, yielding `wstring_view`s of UTF16 text. Use it as: diff --git a/src/interactivity/win32/windowio.cpp b/src/interactivity/win32/windowio.cpp index 5b882feeeef..c5a77b0e095 100644 --- a/src/interactivity/win32/windowio.cpp +++ b/src/interactivity/win32/windowio.cpp @@ -127,12 +127,13 @@ void HandleKeyEvent(const HWND hWnd, { auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - // BOGUS for WM_CHAR/WM_DEADCHAR, in which LOWORD(lParam) is a character + // BOGUS for WM_CHAR/WM_DEADCHAR, in which LOWORD(wParam) is a character auto VirtualKeyCode = LOWORD(wParam); WORD VirtualScanCode = LOBYTE(HIWORD(lParam)); const auto RepeatCount = LOWORD(lParam); - const auto ControlKeyState = GetControlKeyState(lParam); + auto ControlKeyState = GetControlKeyState(lParam); const BOOL bKeyDown = WI_IsFlagClear(lParam, KEY_TRANSITION_UP); + const bool IsCharacterMessage = (Message == WM_CHAR || Message == WM_SYSCHAR || Message == WM_DEADCHAR || Message == WM_SYSDEADCHAR); if (bKeyDown) { @@ -146,7 +147,7 @@ void HandleKeyEvent(const HWND hWnd, // Make sure we retrieve the key info first, or we could chew up // unneeded space in the key info table if we bail out early. - if (Message == WM_CHAR || Message == WM_SYSCHAR || Message == WM_DEADCHAR || Message == WM_SYSDEADCHAR) + if (IsCharacterMessage) { // --- START LOAD BEARING CODE --- // NOTE: We MUST match up the original data from the WM_KEYDOWN stroke (handled at some inexact moment in the @@ -167,9 +168,27 @@ void HandleKeyEvent(const HWND hWnd, // --- END LOAD BEARING CODE --- } + // Simulated key events (using `SendInput` or `SendMessage`) can have invalid + // virtual key code, and invalid scan code. We need to filter such events out, + // as some applications (e.g. WSL) treat those events as valid key events and + // translate them to an ascii NULL character. GH#15753 + if (VirtualScanCode == 0 && !IsCharacterMessage) // `WM_[SYS][DEAD]CHAR` messages don't have this issue + { + // We try to infer the correct scan code from the virtual key code. If the + // virtual key code is invalid or we couldn't map it to a scan code, + // MapVirtualKeyEx will return 0. + auto FullVirtualScanCode = gsl::narrow_cast(OneCoreSafeMapVirtualKeyW(VirtualKeyCode, MAPVK_VK_TO_VSC_EX)); + VirtualScanCode = LOBYTE(FullVirtualScanCode); + ControlKeyState |= (HIBYTE(FullVirtualScanCode) == 0xE0) ? ENHANCED_KEY : 0; + if (VirtualScanCode == 0) + { + return; + } + } + KeyEvent keyEvent{ !!bKeyDown, RepeatCount, VirtualKeyCode, VirtualScanCode, UNICODE_NULL, 0 }; - if (Message == WM_CHAR || Message == WM_SYSCHAR || Message == WM_DEADCHAR || Message == WM_SYSDEADCHAR) + if (IsCharacterMessage) { // If this is a fake character, zero the scancode. if (lParam & 0x02000000) @@ -409,7 +428,7 @@ void HandleKeyEvent(const HWND hWnd, // ignore key strokes that will generate CHAR messages. this is only necessary while a dialog box is up. if (ServiceLocator::LocateGlobals().uiDialogBoxCount != 0) { - if (Message != WM_CHAR && Message != WM_SYSCHAR && Message != WM_DEADCHAR && Message != WM_SYSDEADCHAR) + if (!IsCharacterMessage) { WCHAR awch[MAX_CHARS_FROM_1_KEYSTROKE]; BYTE KeyState[256]; diff --git a/src/server/ApiDispatchers.cpp b/src/server/ApiDispatchers.cpp index 22f7449a7b8..63e321d6c8a 100644 --- a/src/server/ApiDispatchers.cpp +++ b/src/server/ApiDispatchers.cpp @@ -12,6 +12,29 @@ #include "../host/telemetry.hpp" #include "../host/cmdline.h" +// Assumes that it will find in the calling environment. +#define TraceConsoleAPICallWithOrigin(ApiName, ...) \ + TraceLoggingWrite( \ + g_hConhostV2EventTraceProvider, \ + "API_" ApiName, \ + TraceLoggingPid(TraceGetProcessId(m), "OriginatingProcess"), \ + TraceLoggingTid(TraceGetThreadId(m), "OriginatingThread"), \ + __VA_ARGS__ __VA_OPT__(, ) \ + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), \ + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + +static DWORD TraceGetProcessId(CONSOLE_API_MSG* const m) +{ + const auto p = m->GetProcessHandle(); + return p ? p->dwProcessId : 0; +} + +static DWORD TraceGetThreadId(CONSOLE_API_MSG* const m) +{ + const auto p = m->GetProcessHandle(); + return p ? p->dwThreadId : 0; +} + [[nodiscard]] HRESULT ApiDispatchers::ServerGetConsoleCP(_Inout_ CONSOLE_API_MSG* const m, _Inout_ BOOL* const /*pbReplyPending*/) { @@ -35,35 +58,22 @@ { Telemetry::Instance().LogApiCall(Telemetry::ApiCall::GetConsoleMode); const auto a = &m->u.consoleMsgL1.GetConsoleMode; - std::wstring_view handleType = L"unknown"; - - TraceLoggingWrite(g_hConhostV2EventTraceProvider, - "API_GetConsoleMode", - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingOpcode(WINEVENT_OPCODE_START)); - - auto tracing = wil::scope_exit([&]() { - Tracing::s_TraceApi(a, handleType); - TraceLoggingWrite(g_hConhostV2EventTraceProvider, - "API_GetConsoleMode", - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingOpcode(WINEVENT_OPCODE_STOP)); - }); const auto pObjectHandle = m->GetObjectHandle(); RETURN_HR_IF_NULL(E_HANDLE, pObjectHandle); + + TraceConsoleAPICallWithOrigin( + "GetConsoleMode", + TraceLoggingBool(pObjectHandle->IsInputHandle(), "InputHandle")); + if (pObjectHandle->IsInputHandle()) { - handleType = L"input"; InputBuffer* pObj; RETURN_IF_FAILED(pObjectHandle->GetInputBuffer(GENERIC_READ, &pObj)); m->_pApiRoutines->GetConsoleInputModeImpl(*pObj, a->Mode); } else { - handleType = L"output"; SCREEN_INFORMATION* pObj; RETURN_IF_FAILED(pObjectHandle->GetScreenBuffer(GENERIC_READ, &pObj)); m->_pApiRoutines->GetConsoleOutputModeImpl(*pObj, a->Mode); @@ -79,6 +89,12 @@ const auto pObjectHandle = m->GetObjectHandle(); RETURN_HR_IF_NULL(E_HANDLE, pObjectHandle); + + TraceConsoleAPICallWithOrigin( + "SetConsoleMode", + TraceLoggingBool(pObjectHandle->IsInputHandle(), "InputHandle"), + TraceLoggingHexULong(a->Mode, "Mode")); + if (pObjectHandle->IsInputHandle()) { InputBuffer* pObj; @@ -260,17 +276,23 @@ const std::wstring_view exeView(pwsExeName.get(), cchExeName); // 2. Existing data in the buffer that was passed in. - const auto cbInitialData = a->InitialNumBytes; std::unique_ptr pbInitialData; + std::wstring_view initialData; try { + const auto cbInitialData = a->InitialNumBytes; if (cbInitialData > 0) { + // InitialNumBytes is only supported for ReadConsoleW (via CONSOLE_READCONSOLE_CONTROL::nInitialChars). + RETURN_HR_IF(E_INVALIDARG, !a->Unicode); + pbInitialData = std::make_unique(cbInitialData); // This parameter starts immediately after the exe name so skip by that many bytes. RETURN_IF_FAILED(m->ReadMessageInput(cbExeName, pbInitialData.get(), cbInitialData)); + + initialData = { reinterpret_cast(pbInitialData.get()), cbInitialData / sizeof(wchar_t) }; } } CATCH_RETURN(); @@ -285,37 +307,18 @@ std::unique_ptr waiter; size_t cbWritten; - HRESULT hr; - if (a->Unicode) - { - const std::string_view initialData(pbInitialData.get(), cbInitialData); - const std::span outputBuffer(reinterpret_cast(pvBuffer), cbBufferSize); - hr = m->_pApiRoutines->ReadConsoleWImpl(*pInputBuffer, - outputBuffer, - cbWritten, // We must set the reply length in bytes. - waiter, - initialData, - exeView, - *pInputReadHandleData, - hConsoleClient, - a->CtrlWakeupMask, - a->ControlKeyState); - } - else - { - const std::string_view initialData(pbInitialData.get(), cbInitialData); - const std::span outputBuffer(reinterpret_cast(pvBuffer), cbBufferSize); - hr = m->_pApiRoutines->ReadConsoleAImpl(*pInputBuffer, + const std::span outputBuffer(reinterpret_cast(pvBuffer), cbBufferSize); + auto hr = m->_pApiRoutines->ReadConsoleImpl(*pInputBuffer, outputBuffer, cbWritten, // We must set the reply length in bytes. waiter, initialData, exeView, *pInputReadHandleData, + a->Unicode, hConsoleClient, a->CtrlWakeupMask, a->ControlKeyState); - } LOG_IF_FAILED(SizeTToULong(cbWritten, &a->NumBytes)); @@ -365,9 +368,6 @@ // Get input parameter buffer PVOID pvBuffer; ULONG cbBufferSize; - auto tracing = wil::scope_exit([&]() { - Tracing::s_TraceApi(pvBuffer, a); - }); RETURN_IF_FAILED(m->GetInputBuffer(&pvBuffer, &cbBufferSize)); std::unique_ptr waiter; @@ -385,6 +385,11 @@ const std::wstring_view buffer(reinterpret_cast(pvBuffer), cbBufferSize / sizeof(wchar_t)); size_t cchInputRead; + TraceConsoleAPICallWithOrigin( + "WriteConsoleW", + TraceLoggingUInt32(a->NumBytes, "NumBytes"), + TraceLoggingCountedWideString(buffer.data(), static_cast(buffer.size()), "Buffer")); + hr = m->_pApiRoutines->WriteConsoleWImpl(*pScreenInfo, buffer, cchInputRead, requiresVtQuirk, waiter); // We must set the reply length in bytes. Convert back from characters. @@ -395,6 +400,11 @@ const std::string_view buffer(reinterpret_cast(pvBuffer), cbBufferSize); size_t cchInputRead; + TraceConsoleAPICallWithOrigin( + "WriteConsoleA", + TraceLoggingUInt32(a->NumBytes, "NumBytes"), + TraceLoggingCountedString(buffer.data(), static_cast(buffer.size()), "Buffer")); + hr = m->_pApiRoutines->WriteConsoleAImpl(*pScreenInfo, buffer, cchInputRead, requiresVtQuirk, waiter); // Reply length is already in bytes (chars), don't need to convert. @@ -581,10 +591,6 @@ Telemetry::Instance().LogApiCall(Telemetry::ApiCall::GetConsoleScreenBufferInfoEx); const auto a = &m->u.consoleMsgL2.GetConsoleScreenBufferInfo; - auto tracing = wil::scope_exit([&]() { - Tracing::s_TraceApi(a); - }); - CONSOLE_SCREEN_BUFFER_INFOEX ex = { 0 }; ex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX); @@ -640,6 +646,12 @@ ex.wAttributes = a->Attributes; ex.wPopupAttributes = a->PopupAttributes; + TraceConsoleAPICallWithOrigin( + "SetConsoleScreenBufferInfoEx", + TraceLoggingConsoleCoord(a->Size, "BufferSize"), + TraceLoggingConsoleCoord(a->CurrentWindowSize, "WindowSize"), + TraceLoggingConsoleCoord(a->MaximumWindowSize, "MaxWindowSize")); + return m->_pApiRoutines->SetConsoleScreenBufferInfoExImpl(*pObj, ex); } @@ -655,6 +667,10 @@ SCREEN_INFORMATION* pObj; RETURN_IF_FAILED(pObjectHandle->GetScreenBuffer(GENERIC_WRITE, &pObj)); + TraceConsoleAPICallWithOrigin( + "SetConsoleScreenBufferSize", + TraceLoggingConsoleCoord(a->Size, "BufferSize")); + return m->_pApiRoutines->SetConsoleScreenBufferSizeImpl(*pObj, til::wrap_coord_size(a->Size)); } @@ -731,15 +747,16 @@ Telemetry::Instance().LogApiCall(Telemetry::ApiCall::SetConsoleTextAttribute); const auto a = &m->u.consoleMsgL2.SetConsoleTextAttribute; - auto tracing = wil::scope_exit([&]() { - Tracing::s_TraceApi(a); - }); - const auto pObjectHandle = m->GetObjectHandle(); RETURN_HR_IF_NULL(E_HANDLE, pObjectHandle); SCREEN_INFORMATION* pObj; RETURN_IF_FAILED(pObjectHandle->GetScreenBuffer(GENERIC_WRITE, &pObj)); + + TraceConsoleAPICallWithOrigin( + "SetConsoleTextAttribute", + TraceLoggingHexUInt16(a->Attributes, "Attributes")); + RETURN_HR(m->_pApiRoutines->SetConsoleTextAttributeImpl(*pObj, a->Attributes)); } @@ -755,6 +772,14 @@ SCREEN_INFORMATION* pObj; RETURN_IF_FAILED(pObjectHandle->GetScreenBuffer(GENERIC_WRITE, &pObj)); + TraceConsoleAPICallWithOrigin( + "SetConsoleWindowInfo", + TraceLoggingBool(a->Absolute, "IsWindowRectAbsolute"), + TraceLoggingInt32(a->Window.Left, "WindowRectLeft"), + TraceLoggingInt32(a->Window.Right, "WindowRectRight"), + TraceLoggingInt32(a->Window.Top, "WindowRectTop"), + TraceLoggingInt32(a->Window.Bottom, "WindowRectBottom")); + return m->_pApiRoutines->SetConsoleWindowInfoImpl(*pObj, a->Absolute, til::wrap_small_rect(a->Window)); } @@ -911,10 +936,6 @@ { const auto a = &m->u.consoleMsgL2.WriteConsoleOutputString; - auto tracing = wil::scope_exit([&]() { - Tracing::s_TraceApi(a); - }); - switch (a->StringType) { case CONSOLE_ATTRIBUTE: @@ -951,6 +972,11 @@ { const std::string_view text(reinterpret_cast(pvBuffer), cbBufferSize); + TraceConsoleAPICallWithOrigin( + "WriteConsoleOutputCharacterA", + TraceLoggingConsoleCoord(a->WriteCoord, "WriteCoord"), + TraceLoggingUInt32(a->NumRecords, "NumRecords")); + hr = m->_pApiRoutines->WriteConsoleOutputCharacterAImpl(*pScreenInfo, text, til::wrap_coord(a->WriteCoord), @@ -963,6 +989,11 @@ { const std::wstring_view text(reinterpret_cast(pvBuffer), cbBufferSize / sizeof(wchar_t)); + TraceConsoleAPICallWithOrigin( + "WriteConsoleOutputCharacterW", + TraceLoggingConsoleCoord(a->WriteCoord, "WriteCoord"), + TraceLoggingUInt32(a->NumRecords, "NumRecords")); + hr = m->_pApiRoutines->WriteConsoleOutputCharacterWImpl(*pScreenInfo, text, til::wrap_coord(a->WriteCoord), @@ -974,6 +1005,11 @@ { const std::span text(reinterpret_cast(pvBuffer), cbBufferSize / sizeof(WORD)); + TraceConsoleAPICallWithOrigin( + "WriteConsoleOutputAttribute", + TraceLoggingConsoleCoord(a->WriteCoord, "WriteCoord"), + TraceLoggingUInt32(a->NumRecords, "NumRecords")); + hr = m->_pApiRoutines->WriteConsoleOutputAttributeImpl(*pScreenInfo, text, til::wrap_coord(a->WriteCoord), @@ -1221,38 +1257,38 @@ ULONG cbBufferSize; RETURN_IF_FAILED(m->GetInputBuffer(&pvBuffer, &cbBufferSize)); - PVOID pvInputTarget; - const auto cbInputTarget = a->TargetLength; - PVOID pvInputExeName; - const auto cbInputExeName = a->ExeLength; - PVOID pvInputSource; - const auto cbInputSource = a->SourceLength; - // clang-format off - RETURN_HR_IF(E_INVALIDARG, !IsValidStringBuffer(a->Unicode, - pvBuffer, - cbBufferSize, - 3, - cbInputExeName, - &pvInputExeName, - cbInputSource, - &pvInputSource, - cbInputTarget, - &pvInputTarget)); - // clang-format on + // There are 3 strings stored back-to-back within the message payload. + // First we verify that their size and alignment are alright and then we extract them. + const ULONG cbInputExeName = a->ExeLength; + const ULONG cbInputSource = a->SourceLength; + const ULONG cbInputTarget = a->TargetLength; + + const auto alignment = a->Unicode ? alignof(wchar_t) : alignof(char); + // ExeLength, SourceLength and TargetLength are USHORT and summing them up will not overflow a ULONG. + const auto badLength = cbInputTarget + cbInputExeName + cbInputSource > cbBufferSize; + // Since (any) alignment is a power of 2, we can use bit tricks to test if the alignment is right: + // a) Combining the values with OR works, because we're only interested whether the lowest bits are 0 (= aligned). + // b) x % y can be replaced with x & (y - 1) if y is a power of 2. + const auto badAlignment = ((cbInputExeName | cbInputSource | cbInputTarget) & (alignment - 1)) != 0; + RETURN_HR_IF(E_INVALIDARG, badLength || badAlignment); + + const auto pvInputExeName = static_cast(pvBuffer); + const auto pvInputSource = pvInputExeName + cbInputExeName; + const auto pvInputTarget = pvInputSource + cbInputSource; if (a->Unicode) { + const std::wstring_view inputExeName(reinterpret_cast(pvInputExeName), cbInputExeName / sizeof(wchar_t)); const std::wstring_view inputSource(reinterpret_cast(pvInputSource), cbInputSource / sizeof(wchar_t)); const std::wstring_view inputTarget(reinterpret_cast(pvInputTarget), cbInputTarget / sizeof(wchar_t)); - const std::wstring_view inputExeName(reinterpret_cast(pvInputExeName), cbInputExeName / sizeof(wchar_t)); return m->_pApiRoutines->AddConsoleAliasWImpl(inputSource, inputTarget, inputExeName); } else { - const std::string_view inputSource(reinterpret_cast(pvInputSource), cbInputSource); - const std::string_view inputTarget(reinterpret_cast(pvInputTarget), cbInputTarget); - const std::string_view inputExeName(reinterpret_cast(pvInputExeName), cbInputExeName); + const std::string_view inputExeName(pvInputExeName, cbInputExeName); + const std::string_view inputSource(pvInputSource, cbInputSource); + const std::string_view inputTarget(pvInputTarget, cbInputTarget); return m->_pApiRoutines->AddConsoleAliasAImpl(inputSource, inputTarget, inputExeName); } @@ -1268,20 +1304,22 @@ ULONG cbInputBufferSize; RETURN_IF_FAILED(m->GetInputBuffer(&pvInputBuffer, &cbInputBufferSize)); - PVOID pvInputExe; - const auto cbInputExe = a->ExeLength; - PVOID pvInputSource; - const auto cbInputSource = a->SourceLength; - // clang-format off - RETURN_HR_IF(E_INVALIDARG, !IsValidStringBuffer(a->Unicode, - pvInputBuffer, - cbInputBufferSize, - 2, - cbInputExe, - &pvInputExe, - cbInputSource, - &pvInputSource)); - // clang-format on + // There are 2 strings stored back-to-back within the message payload. + // First we verify that their size and alignment are alright and then we extract them. + const ULONG cbInputExeName = a->ExeLength; + const ULONG cbInputSource = a->SourceLength; + + const auto alignment = a->Unicode ? alignof(wchar_t) : alignof(char); + // ExeLength and SourceLength are USHORT and summing them up will not overflow a ULONG. + const auto badLength = cbInputExeName + cbInputSource > cbInputBufferSize; + // Since (any) alignment is a power of 2, we can use bit tricks to test if the alignment is right: + // a) Combining the values with OR works, because we're only interested whether the lowest bits are 0 (= aligned). + // b) x % y can be replaced with x & (y - 1) if y is a power of 2. + const auto badAlignment = ((cbInputExeName | cbInputSource) & (alignment - 1)) != 0; + RETURN_HR_IF(E_INVALIDARG, badLength || badAlignment); + + const auto pvInputExeName = static_cast(pvInputBuffer); + const auto pvInputSource = pvInputExeName + cbInputExeName; PVOID pvOutputBuffer; ULONG cbOutputBufferSize; @@ -1291,9 +1329,9 @@ size_t cbWritten; if (a->Unicode) { - const std::wstring_view inputSource(reinterpret_cast(pvInputSource), cbInputSource / sizeof(wchar_t)); - const std::wstring_view inputExeName(reinterpret_cast(pvInputExe), cbInputExe / sizeof(wchar_t)); - std::span outputBuffer(reinterpret_cast(pvOutputBuffer), cbOutputBufferSize / sizeof(wchar_t)); + const std::wstring_view inputExeName{ reinterpret_cast(pvInputExeName), cbInputExeName / sizeof(wchar_t) }; + const std::wstring_view inputSource{ reinterpret_cast(pvInputSource), cbInputSource / sizeof(wchar_t) }; + const std::span outputBuffer{ static_cast(pvOutputBuffer), cbOutputBufferSize / sizeof(wchar_t) }; size_t cchWritten; hr = m->_pApiRoutines->GetConsoleAliasWImpl(inputSource, outputBuffer, cchWritten, inputExeName); @@ -1303,9 +1341,9 @@ } else { - const std::string_view inputSource(reinterpret_cast(pvInputSource), cbInputSource); - const std::string_view inputExeName(reinterpret_cast(pvInputExe), cbInputExe); - std::span outputBuffer(reinterpret_cast(pvOutputBuffer), cbOutputBufferSize); + const std::string_view inputExeName{ pvInputExeName, cbInputExeName }; + const std::string_view inputSource{ pvInputSource, cbInputSource }; + const std::span outputBuffer{ static_cast(pvOutputBuffer), cbOutputBufferSize }; size_t cchWritten; hr = m->_pApiRoutines->GetConsoleAliasAImpl(inputSource, outputBuffer, cchWritten, inputExeName); diff --git a/src/server/ApiSorter.cpp b/src/server/ApiSorter.cpp index 2593eb5806e..39cf676e497 100644 --- a/src/server/ApiSorter.cpp +++ b/src/server/ApiSorter.cpp @@ -174,7 +174,6 @@ PCONSOLE_API_MSG ApiSorter::ConsoleDispatchRequest(_Inout_ PCONSOLE_API_MSG Mess // alias API. NTSTATUS Status = S_OK; { - const auto trace = Tracing::s_TraceApiCall(Status, Descriptor->TraceName); Status = (*Descriptor->Routine)(Message, &ReplyPending); } if (Status != STATUS_BUFFER_TOO_SMALL) diff --git a/src/server/IApiRoutines.h b/src/server/IApiRoutines.h index a5d691bf149..87830795c6c 100644 --- a/src/server/IApiRoutines.h +++ b/src/server/IApiRoutines.h @@ -74,27 +74,17 @@ class IApiRoutines const bool IsPeek, std::unique_ptr& waiter) noexcept = 0; - [[nodiscard]] virtual HRESULT ReadConsoleAImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept = 0; - - [[nodiscard]] virtual HRESULT ReadConsoleWImpl(IConsoleInputObject& context, - std::span buffer, - size_t& written, - std::unique_ptr& waiter, - const std::string_view initialData, - const std::wstring_view exeName, - INPUT_READ_HANDLE_DATA& readHandleState, - const HANDLE clientHandle, - const DWORD controlWakeupMask, - DWORD& controlKeyState) noexcept = 0; + [[nodiscard]] virtual HRESULT ReadConsoleImpl(IConsoleInputObject& context, + std::span buffer, + size_t& written, + std::unique_ptr& waiter, + const std::wstring_view initialData, + const std::wstring_view exeName, + INPUT_READ_HANDLE_DATA& readHandleState, + const bool IsUnicode, + const HANDLE clientHandle, + const DWORD controlWakeupMask, + DWORD& controlKeyState) noexcept = 0; [[nodiscard]] virtual HRESULT WriteConsoleAImpl(IConsoleOutputObject& context, const std::string_view buffer, diff --git a/src/server/ProcessHandle.cpp b/src/server/ProcessHandle.cpp index b9cf2f1b359..51b6f221c6b 100644 --- a/src/server/ProcessHandle.cpp +++ b/src/server/ProcessHandle.cpp @@ -28,7 +28,7 @@ ConsoleProcessHandle::ConsoleProcessHandle(const DWORD dwProcessId, dwProcessId))), _policy(ConsoleProcessPolicy::s_CreateInstance(_hProcess.get())), _shimPolicy(_hProcess.get()), - _processCreationTime(0) + _processCreationTime{} { if (nullptr != _hProcess.get()) { @@ -77,24 +77,17 @@ const HANDLE ConsoleProcessHandle::GetRawHandle() const // Routine Description: // - Retrieves the process creation time (currently used in telemetry traces) // - The creation time is lazily populated on first call -const ULONG64 ConsoleProcessHandle::GetProcessCreationTime() const +const FILETIME ConsoleProcessHandle::GetProcessCreationTime() const { - if (_processCreationTime == 0 && _hProcess != nullptr) + if (_processCreationTime.dwHighDateTime == 0 && _processCreationTime.dwLowDateTime == 0 && _hProcess != nullptr) { - FILETIME ftCreationTime, ftDummyTime = { 0 }; - ULARGE_INTEGER creationTime = { 0 }; + FILETIME ftDummyTime = { 0 }; - if (::GetProcessTimes(_hProcess.get(), - &ftCreationTime, - &ftDummyTime, - &ftDummyTime, - &ftDummyTime)) - { - creationTime.HighPart = ftCreationTime.dwHighDateTime; - creationTime.LowPart = ftCreationTime.dwLowDateTime; - } - - _processCreationTime = creationTime.QuadPart; + ::GetProcessTimes(_hProcess.get(), + &_processCreationTime, + &ftDummyTime, + &ftDummyTime, + &ftDummyTime); } return _processCreationTime; diff --git a/src/server/ProcessHandle.h b/src/server/ProcessHandle.h index 02f7ad9ba05..58f9f9c52d2 100644 --- a/src/server/ProcessHandle.h +++ b/src/server/ProcessHandle.h @@ -53,14 +53,14 @@ class ConsoleProcessHandle CD_CONNECTION_INFORMATION GetConnectionInformation(IDeviceComm* deviceComm) const; - const ULONG64 GetProcessCreationTime() const; + const FILETIME GetProcessCreationTime() const; private: ULONG _ulTerminateCount; ULONG const _ulProcessGroupId; wil::unique_handle const _hProcess; - mutable ULONG64 _processCreationTime; + mutable FILETIME _processCreationTime; const ConsoleProcessPolicy _policy; const ConsoleShimPolicy _shimPolicy; diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index 3673aa09d0a..feeb0413979 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -166,7 +166,8 @@ namespace Microsoft::Console::VirtualTerminal constexpr VTParameter at(const size_t index) const noexcept { - return til::at(_subParams, index); + // If the index is out of range, we return a sub parameter with no value. + return index < _subParams.size() ? til::at(_subParams, index) : defaultParameter; } VTSubParameters subspan(const size_t offset, const size_t count) const noexcept @@ -191,6 +192,8 @@ namespace Microsoft::Console::VirtualTerminal } private: + static constexpr VTParameter defaultParameter{}; + std::span _subParams; }; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 3f80931a3d7..04ce77e46cd 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -176,7 +176,7 @@ void AdaptDispatch::_WriteToBuffer(const std::wstring_view string) // we tried writing a wide glyph into the last column which can't work. if (textPositionBefore == textPositionAfter && (state.columnBegin == 0 || !wrapAtEOL)) { - textBuffer.ConsumeGrapheme(state.text); + state.text = state.text.substr(textBuffer.GraphemeNext(state.text, 0)); } if (wrapAtEOL) @@ -4117,14 +4117,14 @@ void AdaptDispatch::_ReportSGRSetting() const else if (color.IsIndex256()) { const auto index = color.GetIndex(); - fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{};5;{}"), base + 8, index); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{}:5:{}"), base + 8, index); } else if (color.IsRgb()) { const auto r = GetRValue(color.GetRGB()); const auto g = GetGValue(color.GetRGB()); const auto b = GetBValue(color.GetRGB()); - fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{};2;{};{};{}"), base + 8, r, g, b); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{}:2::{}:{}:{}"), base + 8, r, g, b); } }; addColor(30, attr.GetForeground()); diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index e0c346dc9d0..87f5f11ba4d 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -297,12 +297,15 @@ namespace Microsoft::Console::VirtualTerminal size_t _SetRgbColorsHelper(const VTParameters options, TextAttribute& attr, const bool isForeground) noexcept; + void _SetRgbColorsHelperFromSubParams(const VTParameter colorItem, + const VTSubParameters options, + TextAttribute& attr) noexcept; size_t _ApplyGraphicsOption(const VTParameters options, const size_t optionIndex, TextAttribute& attr) noexcept; - void _ApplyGraphicsOptionSubParam(const VTParameter option, - const VTSubParameters subParams, - TextAttribute& attr) noexcept; + void _ApplyGraphicsOptionWithSubParams(const VTParameter option, + const VTSubParameters subParams, + TextAttribute& attr) noexcept; void _ApplyGraphicsOptions(const VTParameters options, TextAttribute& attr) noexcept; diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp index 5259cf8ce64..a6567abd09e 100644 --- a/src/terminal/adapter/adaptDispatchGraphics.cpp +++ b/src/terminal/adapter/adaptDispatchGraphics.cpp @@ -35,7 +35,8 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options, const size_t red = options.at(1).value_or(0); const size_t green = options.at(2).value_or(0); const size_t blue = options.at(3).value_or(0); - // ensure that each value fits in a byte + // We only apply the color if the R, G, B values fit within a byte. + // This is to match XTerm's and VTE's behavior. if (red <= 255 && green <= 255 && blue <= 255) { const auto rgbColor = RGB(red, green, blue); @@ -46,6 +47,9 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options, { optionsConsumed = 2; const size_t tableIndex = options.at(1).value_or(0); + + // We only apply the color if the index value fit within a byte. + // This is to match XTerm's and VTE's behavior. if (tableIndex <= 255) { const auto adjustedIndex = gsl::narrow_cast(tableIndex); @@ -62,6 +66,77 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options, return optionsConsumed; } +// Routine Description: +// - Helper to parse extended graphics options, which start with 38 (FG) or 48 (BG) +// - These options are followed by either a 2 (RGB) or 5 (xterm index): +// - RGB sequences then take 4 MORE options to designate the ColorSpaceID, R, G, B parts +// of the color. +// - Xterm index will use the option that follows to use a color from the +// preset 256 color xterm color table. +// Arguments: +// - colorItem - One of FG(38) and BG(48), indicating which color we're setting. +// - options - An array of options that will be used to generate the RGB color +// - attr - The attribute that will be updated with the parsed color. +// Return Value: +// - +void AdaptDispatch::_SetRgbColorsHelperFromSubParams(const VTParameter colorItem, + const VTSubParameters options, + TextAttribute& attr) noexcept +{ + // This should be called for applying FG and BG colors only. + assert(colorItem == GraphicsOptions::ForegroundExtended || + colorItem == GraphicsOptions::BackgroundExtended); + + const bool isForeground = (colorItem == GraphicsOptions::ForegroundExtended); + const DispatchTypes::GraphicsOptions typeOpt = options.at(0); + + if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint) + { + // sub params are in the order: + // :2:::: + + // We treat a color as invalid, if it has a color space ID, as some + // applications that support non-standard ODA color sequence may send + // the red value in its place. + const bool hasColorSpaceId = options.at(1).has_value(); + + // Skip color-space-id. + const size_t red = options.at(2).value_or(0); + const size_t green = options.at(3).value_or(0); + const size_t blue = options.at(4).value_or(0); + + // We only apply the color if the R, G, B values fit within a byte. + // This is to match XTerm's and VTE's behavior. + if (!hasColorSpaceId && red <= 255 && green <= 255 && blue <= 255) + { + const auto rgbColor = RGB(red, green, blue); + attr.SetColor(rgbColor, isForeground); + } + } + else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index) + { + // sub params are in the order: + // :5: + // where 'n' is the index into the xterm color table. + const size_t tableIndex = options.at(1).value_or(0); + + // We only apply the color if the index value fit within a byte. + // This is to match XTerm's and VTE's behavior. + if (tableIndex <= 255) + { + const auto adjustedIndex = gsl::narrow_cast(tableIndex); + if (isForeground) + { + attr.SetIndexedForeground256(adjustedIndex); + } + else + { + attr.SetIndexedBackground256(adjustedIndex); + } + } + } +} + // Routine Description: // - Helper to apply a single graphic rendition option to an attribute. // - Calls appropriate helper to apply the option with sub parameters when necessary. @@ -80,7 +155,7 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options, if (options.hasSubParamsFor(optionIndex)) { const auto subParams = options.subParamsFor(optionIndex); - _ApplyGraphicsOptionSubParam(opt, subParams, attr); + _ApplyGraphicsOptionWithSubParams(opt, subParams, attr); return 1; } @@ -260,20 +335,30 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options, } // Routine Description: -// - This is a no-op until we have something meaningful to do with sub parameters. // - Helper to apply a single graphic rendition option with sub parameters to an attribute. // Arguments: // - option - An option to apply. +// - subParams - Sub parameters associated with the option. // - attr - The attribute that will be updated with the applied option. // Return Value: // - -void AdaptDispatch::_ApplyGraphicsOptionSubParam(const VTParameter /* option */, - const VTSubParameters /* subParam */, - TextAttribute& /* attr */) noexcept +void AdaptDispatch::_ApplyGraphicsOptionWithSubParams(const VTParameter option, + const VTSubParameters subParams, + TextAttribute& attr) noexcept { // here, we apply our "best effort" rule, while handling sub params if we don't // recognise the parameter substring (parameter and it's sub parameters) then // we should just skip over them. + switch (option) + { + case ForegroundExtended: + case BackgroundExtended: + _SetRgbColorsHelperFromSubParams(option, subParams, attr); + break; + default: + /* do nothing */ + break; + } } // Routine Description: diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index c0be3a74e0f..162142ae3e4 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -1699,7 +1699,7 @@ class AdapterTest attribute.SetIndexedBackground256(45); _testGetSet->_textBuffer->SetCurrentAttributes(attribute); requestSetting(L"m"); - _testGetSet->ValidateInputEvent(L"\033P1$r0;38;5;123;48;5;45m\033\\"); + _testGetSet->ValidateInputEvent(L"\033P1$r0;38:5:123;48:5:45m\033\\"); Log::Comment(L"Requesting SGR attributes (ITU RGB colors)."); _testGetSet->PrepData(); @@ -1708,7 +1708,7 @@ class AdapterTest attribute.SetBackground(RGB(65, 43, 21)); _testGetSet->_textBuffer->SetCurrentAttributes(attribute); requestSetting(L"m"); - _testGetSet->ValidateInputEvent(L"\033P1$r0;38;2;12;34;56;48;2;65;43;21m\033\\"); + _testGetSet->ValidateInputEvent(L"\033P1$r0;38:2::12:34:56;48:2::65:43:21m\033\\"); Log::Comment(L"Requesting DECSCA attributes (unprotected)."); _testGetSet->PrepData(); @@ -2464,6 +2464,119 @@ class AdapterTest rgOptions[4] = 123; _testGetSet->_expectedAttribute.SetForeground(RGB(0, 0, 123)); VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, 5 })); + + Log::Comment(L"Test 6: Ignore Rgb color when R, G or B is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgOptions[1] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgOptions[2] = 283; + rgOptions[3] = 182; + rgOptions[4] = 123; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, 5 })); + + Log::Comment(L"Test 7: Ignore indexed color when index is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + rgOptions[2] = 283; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, 3 })); + } + + TEST_METHOD(XtermExtendedSubParameterColorTest) + { + Log::Comment(L"Starting test..."); + + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + + VTParameter rgOptions[1]; + VTParameter rgSubParamOpts[16]; + std::pair subParamRanges[1]; + + _testGetSet->_expectedAttribute = _testGetSet->_textBuffer->GetCurrentAttributes(); + + Log::Comment(L"Test 1: Change Indexed Foreground with missing index sub parameter"); + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + subParamRanges[0] = { (BYTE)0, (BYTE)1 }; + _testGetSet->_expectedAttribute.SetIndexedForeground256(TextColor::DARK_BLACK); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 2: Change Indexed Background with default index sub parameter"); + rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + rgSubParamOpts[1] = {}; + subParamRanges[0] = { (BYTE)0, (BYTE)2 }; + _testGetSet->_expectedAttribute.SetIndexedBackground256(TextColor::DARK_BLACK); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 3: Change RGB Foreground with all RGB sub parameters missing"); + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + subParamRanges[0] = { (BYTE)0, (BYTE)1 }; + _testGetSet->_expectedAttribute.SetForeground(RGB(0, 0, 0)); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 4: Change RGB Background with some missing RGB sub parameters"); + rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = {}; // color-space-id + rgSubParamOpts[2] = 123; + subParamRanges[0] = { (BYTE)0, (BYTE)3 }; + _testGetSet->_expectedAttribute.SetBackground(RGB(123, 0, 0)); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 5: Change RGB Foreground with some default RGB sub parameters"); + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = {}; // color-space-id + rgSubParamOpts[2] = {}; + rgSubParamOpts[3] = {}; + rgSubParamOpts[4] = 123; + subParamRanges[0] = { (BYTE)0, (BYTE)5 }; + _testGetSet->_expectedAttribute.SetForeground(RGB(0, 0, 123)); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 6: Ignore color when ColorSpaceID is not empty"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = 7; // color-space-id + rgSubParamOpts[2] = 182; + rgSubParamOpts[3] = 182; + rgSubParamOpts[4] = 123; + subParamRanges[0] = { (BYTE)0, (BYTE)5 }; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 7: Ignore Rgb color when R, G or B is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = {}; // color-space-id + // Ensure r, g and b set a color that is different from current color. + // Otherwise, the test will pass even if the color is not ignored. + rgSubParamOpts[2] = 128; + rgSubParamOpts[3] = 283; + rgSubParamOpts[4] = 155; + subParamRanges[0] = { (BYTE)0, (BYTE)5 }; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 8: Ignore indexed color when index is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + rgSubParamOpts[1] = 283; + subParamRanges[0] = { (BYTE)0, (BYTE)2 }; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); } TEST_METHOD(SetColorTableValue)