## Summary of the Pull Request
Adds support to navigate to clickable hyperlinks using only the keyboard. When in mark mode, the user can press [shift+]tab to go the previous/next hyperlink in the text buffer. Once a hyperlink is selected, the user can press <kbd>Ctrl+Enter</kbd> to open the hyperlink.
## References
#4993
## PR Checklist
* [x] Closes#6649
* [x] Documentation updated at https://github.com/MicrosoftDocs/terminal/pull/558
## Detailed Description of the Pull Request / Additional comments
- Main change
- The `OpenHyperlink` event needs to be piped down to `ControlCore` now so that we can open a hyperlink at that layer.
- `SelectHyperlink(dir)` searches the buffer in a given direction and finds the first hyperlink, then selects it.
- "finding the hyperlink" is the tough part because the pattern tree takes in buffer coordinates, searches through the buffer in that given space, then stores everything relative to the defined space. Normally, the given buffer coordinates would align with the viewport's start and end. However, we're now trying to search outside of the viewport (sometimes), so we need to manage two coordinate systems at the same time.
- `convertToSearchArea()` lambda was used to convert a given coordinate into the search area coordinate system. So if the search area is the visible viewport, we spit out a viewport position. If the search area is the _next_ viewport, we spit out a position relative to that.
- `extractResultFromList()` lambda takes the list of patterns from the pattern tree and spits out the hyperlink we want. If we're searching forwards, we get the next one. Otherwise, we get the previous one. We explicitly ignore the one we're already on. If we don't find any, we return `nullopt`.
- Now that we have all these cool tools, we use them to progressively search through the buffer to find the next/previous hyperlink. Start by searching the visible viewport _after_ (or _before_) the current selection. If we can't find anything, go to the next "page" (viewport scrolled up/down). Repeat this process until something comes up.
- If we can't find anything, nothing happens. We don't wrap around.
- Other relevant changes
- the `copy` action is no longer bound to `Enter`. Instead, we manually handle it in `ControlCore.cpp`. This also lets us handle <kbd>Shift+Enter</kbd> appropriately without having to take another key binding.
- `_ScrollToPoint` was added. It's a simple function that just scrolls the viewport such that the provided buffer position is in view. This was used to de-duplicate code.
- `_ScrollToPoint` was added into the `ToggleMarkMode()` function. Turns out, we don't scroll to the new selection when we enter mark mode (whoops!). We _should_ do that and we should backport this part of the change too. I'll handle that.
- add some clarity when some functions are using the viewport position vs the buffer position. This is important because the pattern interval tree works in the viewport space.
## Validation Steps Performed
- case: all hyperlinks are in the view
- ✅ get next link
- ✅ get prev link
- ✅ case: need to scroll down for next link
- ✅ case: need to scroll up for next link
Does what it says on the tin. When we get focused, temporarily turn off readonly mode, as to not pop the dialog when the focus sequence is eventually sent to the connection.
* closes#13461
## Summary of the Pull Request
In #13370, we should be notifying the renderer that the selection changed. Minor oversight and simple fix.
## References
#4993#13370Closes#13413
## Summary of the Pull Request
Introduces the `switchSelectionEndpoint` action which switches whichever selection endpoint is targeted when a selection is present. For example, if you are targeting "start", `switchSelectionEndpoint` makes it so that now you are targeting "end". This also updates the selection markers appropriately.
## References
Spec - #5804#13358Closes#3663
## Detailed Description of the Pull Request / Additional comments
Most of the code is just standard code of adding a new action. Other than that, we have...
- if there is no selection, the action fails and the keybinding is passed through (similar to `copy()`)
- when we update the selection endpoint, we need to also update the "pivot". This ensures that future calls of `UpdateSelection()` respect this swap.
- [Corner Case] if the cursor is being moved, we make it so that you basically "anchored" an endpoint and you don't have to hold shift anymore.
## Summary of the Pull Request
1. [copy on select] when manually copying text (i.e. kbd or right-click) while in mark/quick-edit mode, we now dismiss the selection.
2. `Enter` is now bound to copy by default.
- This works very well with mark mode and provides a more consistent behavior with conhost's selection experience overall.
- "why not hardcode `Enter` as a way to copy when in mark mode?"
- In an effort to make this as configurable as possible, I decided to make it a configurable keybinding, but am open to suggestions.
3. selection markers
a. we now hide the selection markers when multi-clicking the terminal.
b. selection markers are now properly shown when a single cell selection exists
- Prior to this PR, any single cell selection would display both markers. Now, we actually track which endpoint we're moving and display the appropriate one.
4. ensures that when you use keyboard selection to move past the top/bottom of the scroll area, we clamp it to the origin/bottom-right respectively. The fix is also better here in that it provides consistent behavior across all of the `_MoveByX` functions.
5. adds `toggleBlockSelection` to the schema
## References
#13053
## Validation Steps Performed
Did a whole flowchart of expected behavior with copy on select:
- enable `copyOnSelect`
- make a selection with the mouse
- ✅ right-click should copy the text --> clear the selection --> paste
- use keyboard selection to quick-edit the existing selection
- ✅ `copy` action should clear the selection
- ✅ right-click should copy the text --> clear the selection --> paste
Played with selection markers a bit in mark mode and quick edit mode. Markers are updating appropriately.
## Summary of the Pull Request
This introduces a selection marker overlay that tells the user which endpoint is currently being moved by the keyboard. The selection markers are respect font size changes and `cursor` color.
## References
#715 - Keyboard Selection
#2840 - Keyboard Selection Spec
#5804 - Mark Mode Spec
## Detailed Description of the Pull Request / Additional comments
- `TermControl` layer:
- Use a canvas (similar to the one used for hyperlinks) to be able to draw the selection markers.
- If we are notified that the selection changed, update the selection markers appropriately.
- `UpdateSelectionMarkersEventArgs` lets us distinguish between mouse and keyboard selections. `ClearMarkers` is set to true in the following cases...
1. Mouse selection, via SetEndSelectionPoint
2. `LeftClickOnTerminal`, pretty self-explanatory
3. a selection created from searching for text
- `ControlCore` layer:
- Responsible for notifying `TermControl` to update the selection markers when a selection has changed.
- Transfers info (the selection endpoint positions and which endpoint we're moving) from the terminal core to the term control.
- `TerminalCore` layer:
- Provides the viewport position of the selection endpoints.
## Validation Steps Performed
- mouse selection (w/ and w/out shift) --> no markers
- keyboard selection --> markers
- markers update appropriately when we pivot the selection
- markers scroll when you hit a boundary
- markers take the color of the cursor color setting
- markers are resized when the font size changes
When I moved this into ControlCore, I forgot that UserScrollViewport is usually triggered by the scrollbar updating, so it doesn't ask the UI to update. Since this logic is in ControlCore, it's sorta in a weird place where it needs to communicate both up and down:
* update the `Terminal`'s viewport position
* update the `TermControl`'s scrollbar position
Checklist:
* [x] Closes a bug bash bug
* [x] Missed in #12948
* See also #11000
See also: #12799, the origin of much of this.
This change evolved over multiple phases.
### Part the first
When we create a defterm connection in `TerminalPage::_OnNewConnection`,
we don't have the hosting HWND yet, so the tab gets created without one.
We'll later get called with the owner, in `Initialize`.
To remedy this, we need to:
* In `Initialize`, make sure to update any existing controls with the
new owner.
* In `ControlCore`, actually propogate the new owner down to the
connection
### Part the second
DefTerm launches don't actually request focus mode, so the Terminal
never sends them focus events. We need those focus events so that the
console can request foreground rights.
To remedy this, we need to:
* pass `--win32input` to the commandline used to initialize OpenConsole
in ConPTY mode. We request focus events at the same time we request
win32-input-mode.
* I also added `--resizeQuirk`, because _by all accounts that should be
there_. Resizing in defterm windows should be _wacky_ without it, and
I'm a little surprised we haven't seen any bugs due to this yet.
### Part the third
`ConsoleSetForeground` expects a `HANDLE` to the process we want to give
foreground rights to. The problem is, the wire format we used _also_
decided that a HANDLE value was a good idea. It's not. If we pass the
literal value of the HANDLE to the process from OpenConsole to conhost,
so conhost can call that API, the value that conhost uses there will
most likely be an invalid handle. The HANDLE's value is its value in
_OpenConsole_, not in conhost.
To remedy this, we need to:
* Just not forward `ConsoleSetForeground`. Turns out, we _can_ just call
that in OpenConsole safely. There's no validation. So just instantiate
a static version of the Win32 version of ConsoleControl, just to use
for SetForeground. (thanks Dustin)
* [x] Tested manually - Win+R `powershell`, `notepad` spawns on top.
Closes#13211
Adds support for marks in the scrollbar. These marks can be added in 3
ways:
* Via the iterm2 `OSC 1337 ; SetMark` sequence
* Via the `addMark` action
* Automatically when the `experimental.autoMarkPrompts` per-profile
setting is enabled.
#11000 has more tracking for the big-picture for this feature, as well
as additional follow-ups. This set of functionality seemed complete
enough to send a review for now. That issue describes these how I wish
these actions to look in the fullness of time. This is simply the v0.1
from the hackathon last month.
#### Actions
* `addMark`: add a mark to the buffer. If there's a selection, use
place the mark covering at the selection. Otherwise, place the mark
on the cursor row.
- `color`: a color for the scrollbar mark. This is optional - defaults
to the `foreground` color of the current scheme if omitted.
* `scrollToMark`
- `direction`: `["first", "previous", "next", "last"]`
* `clearMark`: Clears marks at the current postition (either the
selection if there is one, or the cursor position.
* `clearAllMarks`: Don't think this needs explanation.
#### Per-profile settings
* `experimental.autoMarkPrompts`: `bool`, default `false`.
* `experimental.showMarksOnScrollbar`: `bool`
## PR Checklist
* [x] Closes#1527
* [x] Closes#6232
## Detailed Description of the Pull Request / Additional comments
This is basically hackathon code. It's experimental! That's okay! We'll
figure the rest of the design in post.
Theoretically, I should make these actions `experimental.` as well, but
it seemed like since the only way to see these guys was via the
`experimental.showMarksOnScrollbar` setting, you've already broken
yourself into experimental jail, and you know what you're doing.
Things that won't work as expected:
* resizing, ESPECIALLY reflowing
* Clearing the buffer with ED sequences / Clear Buffer
I could theoretically add velocity around this in the `TermControl`
layer. Always prevent marks from being visible, ignore all the actions.
Marks could still be set by VT and automark, but they'd be useless.
Next up priorities:
* Making this work with the FinalTerm sequences
* properly speccing
* adding support for `showMarksOnScrollbar: flags(categories)`, so you
can only display errors on the scrollbar
* adding the `category` flag to the `addMark` action
## Validation Steps Performed
I like using it quite a bit. The marks can get noisy if you have them
emitted on every prompt and the buffer has 9000 lines. But that's the
beautiful thing, the actions work even if the marks aren't visible, so
you can still scroll between prompts.
<details>
<summary>Settings blob</summary>
```jsonc
// actions
{ "keys": "ctrl+up", "command": { "action": "scrollToMark", "direction": "previous" }, "name": "Previous mark" },
{ "keys": "ctrl+down", "command": { "action": "scrollToMark", "direction": "next" }, "name": "Next mark" },
{ "keys": "ctrl+pgup", "command": { "action": "scrollToMark", "direction": "first" }, "name": "First mark" },
{ "keys": "ctrl+pgdn", "command": { "action": "scrollToMark", "direction": "last" }, "name": "Last mark" },
{ "command": { "action": "addMark" } },
{ "command": { "action": "addMark", "color": "#ff00ff" } },
{ "command": { "action": "addMark", "color": "#0000ff" } },
{ "command": { "action": "clearAllMarks" } },
// profiles.defaults
"experimental.autoMarkPrompts": true,
"experimental.showMarksOnScrollbar": true,
```
</details>
## Summary of the Pull Request
This introduced the `toggleBlockSelection` action to allow users to create a block selection using only the keyboard. This is not bound to any keys by default, however it is added to the command palette.
## References
#4993 - Epic
#5804 - Spec
## Validation Steps Performed
- [X] Mark mode always starts in line selection mode
- [X] Mouse selections are always in line selection mode by default
- [X] Can toggle block selection for an existing selection (regardless of how it was created)
- [X] The selection is copied properly (aka, no rendering issues)
Previously this project used a great variety of types to present text buffer
coordinates: `short`, `unsigned short`, `int`, `unsigned int`, `size_t`,
`ptrdiff_t`, `COORD`/`SMALL_RECT` (aka `short`), and more.
This massive commit migrates almost all use of those types over to the
centralized types `til::point`/`size`/`rect`/`inclusive_rect` and their
underlying type `til::CoordType` (aka `int32_t`).
Due to the size of the changeset and statistics I expect it to contain bugs.
The biggest risk I see is that some code potentially, maybe implicitly, expected
arithmetic to be mod 2^16 and that this code now allows it to be mod 2^32.
Any narrowing into `short` later on would then throw exceptions.
## PR Checklist
* [x] Closes#4015
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
Casual usage of OpenConsole and Windows Terminal. ✅
## Summary of the Pull Request
The `DECPS` (Play Sound) escape sequence provides applications with a
way to play a basic sequence of musical notes. This emulates
functionality that was originally supported on the DEC VT520 and VT525
hardware terminals.
## PR Checklist
* [x] Closes#8687
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #8687
## Detailed Description of the Pull Request / Additional comments
When a `DECPS` control is executed, any further output is blocked until
all the notes have finished playing. So to prevent the UI from hanging
during this period, we have to temporarily release the console/terminal
lock, and then reacquire it before returning.
The problem we then have is how to deal with the terminal being closed
during that unlocked interval. The way I've dealt with that is with a
promise that is set to indicate a shutdown. This immediately aborts any
sound that is in progress, but also signals the thread that it needs to
exit as soon as possible.
The thread exit is achieved by throwing a custom exception which is
recognised by the state machine and rethrown instead of being logged.
This gets it all the way up to the root of the write operation, so it
won't attempt to process anything further output that might still be
buffered.
## Validation Steps Performed
Thanks to the testing done by @jerch on a real VT525 terminal, we have a
good idea of how this sequence is supposed to work, and I'm fairly
confident that our implementation is reasonably compatible.
The only significant difference I'm aware of is that we support multiple
notes in a sequence. That was a feature that was documented in the
VT520/VT525 manual, but didn't appear to be supported on the actual
device.
## Summary of the Pull Request
Introduces a non-configurable version of mark mode to Windows Terminal. It has the following interactions defined:
- <kbd>ctrl+shift+m</kbd> --> Enter Mark Mode
- when in Mark Mode...
- <kbd>ESC</kbd> --> Exit Mark Mode
- arrow keys --> move "start"
- <kbd>shift</kbd> + arrow keys --> anchor "start", move "end"
- <kbd>ctrl+a</kbd> --> select all
- when a selection is active...
When in mark mode, the cursor does not blink.
## References
#4993 - [Epic] Keyboard Selection
## PR Checklist
* [X] Closes#715
* [X] Provides a resolution for #11985
## Detailed Description of the Pull Request / Additional comments
- `TermControl`:
- `TermControl.cpp` just adds logic to prevent the cursor from blinking when in mark mode
- `ControlCore`
- in the same place we handle quick edit, we add an entry point to mark mode
- `TerminalCore`
- this leverages `UpdateSelection()` and other quick edit functions to make mark mode happen
## Validation Steps Performed
- [x] Make selection, split pane, close pane
- NOTE: A similar scenario caused a crash at one point. Really weird. Keep an eye on it.
- [x] Cursor is off when in mark mode
- [x] general movement/selection
- [x] general movement/selection that forces the viewport to move
- [x] In mark mode, selectAll...
- [x] arrow keys --> move start
- [x] shift + arrow keys --> move end
- [x] (regardless of mark mode) if selection active, enter --> copy to clipboard
A bad merge, that actually revealed a horrible bug.
There was a secret conflict between the code in #12526 and #12515. 69b77ca was a bad merge that hid just how bad the issue was. Fixing the one line `nullptr`->`this` in `InteractivityFactory` resulted in a window that would flash uncontrollably, as it minimized and restored itself in a loop. Great.
This can seemingly be fixed by making sure that the conpty window is initially created with the owner already set, rather than relying on a `SetParent` call in post. This does pose some complications for the #1256 future we're approaching. However, this is a blocking bug _now_, and we can figure out the tearout/`SetParent` thing in post.
* fixes#13066.
* Tested with the script in that issue.
* Window doesn't flash uncontrollably.
* `gci | ogv` still works right
* I work here.
* Opening a new tab doesn't spontaneously cause the window to minimize
* Restoring from minimized doesn't yeet focus to an invisible window
* Opening a new tab doesn't yeet focus to an invisible window
* There _is_ a viable way to call `GetAncestor` s.t. it returns the Terminal's hwnd in Terminal, and the console's in Conhost
The `SW_SHOWNOACTIVATE` change is also quite load bearing. With just `SW_NORMAL`, the pseudo window (which is invisible!) gets activated whenever the terminal window is restored from minimized. That's BAD.
There's actually more to this as well.
Calling `SetParent` on a window that is `WS_VISIBLE` will cause the OS to hide the window, make it a _child_ window, then call `SW_SHOW` on the window to re-show it. `SW_SHOW`, however, will cause the OS to also set that window as the _foreground_ window, which would result in the pty's hwnd stealing the foreground away from the owning terminal window. That's bad.
`SetWindowLongPtr` seems to do the job of changing who the window owner is, without all the other side effects of reparenting the window.
Without `SetParent`, however, the pty HWND is no longer a descendant of the Terminal HWND, so that means `GA_ROOT` can no longer be used to find the owner's hwnd. For even more insanity, without `WS_POPUP`, none of the values of `GetAncestor` will actually get the terminal HWND. So, now we also need `WS_POPUP` on the pty hwnd. To get at the Terminal hwnd, you'll need
```c++
GetAncestor(GetConsoleWindow(), GA_ROOTOWNER)
```
The `DECAC` (Assign Colors) escape sequence controls which color table
entries are associated with the default foreground and background
colors. This is how you would change the default colors on the the
original DEC VT525 terminals.
But `DECAC` also allows you to assign the color table entries for the
"window frame", which in our case is mapped to the tab color (just the
background for now). So this now gives us a way to control the tab color
via an escape sequence as well.
DETAILS
-------
The way this works is there are now two new entries in the color table
for the frame colors, and two new aliases in the color alias table that
are mapped to those color table entries. As previously mentioned, only
the background is used for now.
By default, the colors are set to `INVALID_COLOR`, which indicates that
the system colors should be used. But if the user has set a `tabColor`
property in their profile, the frame background will be initialized with
that value instead.
And note that some of the existing color table entries are now
renumbered for compatibility with XTerm, which uses entries 256 to 260
for special colors which we don't yet support. Our default colors are
now at 261 and 262, the frame colors are 263 and 264, and the cursor
color is 265.
So at runtime, you can change the tab color programmatically by setting
the color table entry at index 262 using `OSC 4` (assuming you need a
specific RGB value). Otherwise if you just want to set the tab color to
an existing color index, you can use `DECAC 2`.
You can even make the tab color automatically match background color by
mapping the frame background alias to the color table entry for the
default background, using `DECAC 2;261;262` (technically this is mapping
both the the foreground and background).
This PR doesn't include support for querying the color mapping with
`DECRQSS`, and doesn't support resetting the colors with `RIS`, but
hopefully those can be figured out in a future PR - there are some
complications that'll need to be resolved first.
## Validation Steps Performed
I've added a basic unit test that confirms the `DECAC` escape sequence
updates the color aliases in the render settings as expected. I've also
manually confirmed that the tab color in Windows Terminal is updated by
`DECAC 2`, and the default colors are updated in both conhost and WT
using `DECAC 1`.
Closes#6574
Adds the `selectAll` action which can be used to select all text in the buffer (regardless of whether a selection is present).
## References
#3663 - Mark Mode
#4993 - [Scenario] Keyboard selection
## PR Checklist
* [x] Closes#1469
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
I've made it such that selecting the "entire buffer" really just selects up to the mutable viewport. This seems like a nice QOL improvement since there's generally nothing past that.
When the user selects all, the viewport does not move. This is consistent with CMD behavior and is intended to allow the user to not lose context when selecting everything.
A minor change had to be made to the DxRenderer because this uncovered an underflow issue. Basically, the selection rects were handed to the DxEngine relative to the viewport (which means that some had a negative y-value). At some point, those rects were stored into `size_t`s, resulting in an underflow issue. This caused the renderer to behave strangely when rendering the selection. Generally, these kinds of issues weren't really noticed because selection would always modify a portion of the viewport.
Funny enough, in a way, this satisfies the "mark mode" scenario because the user now has a way to initiate a selection using only the keyboard. Though this isn't ideal, just a fun thing to point out (that's why I'm not closing the mark mode issue).
## Validation Steps Performed
- Verified using DxEngine and AtlasEngine
- select all --> keyboard selection --> start moving the top-left endpoint (and scroll to there)
- select all --> do not scroll automatically
## Summary of the Pull Request
This PR replaces the `TerminalDispatch` class with the `AdaptDispatch` class from conhost, so we're no longer duplicating the VT functionality in two places. It also gives us a more complete VT implementation on the Terminal side, so it should work better in pass-through mode.
## References
This is essentially part two of PR #12703.
## PR Checklist
* [x] Closes#3849
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number where discussion took place: #12662
## Detailed Description of the Pull Request / Additional comments
The first thing was to give the `ConGetSet` interface a new name, since it's now no longer specific to conhost. I went with `ITerminalApi`, since that was the equivalent interface on the terminal side, and it still seemed like a generic enough name. I also changed the way the api is managed by the `AdaptDispatch` class, so it's now stored as a reference rather than a `unique_ptr`, which more closely matches the way the `TerminalDispatch` class worked.
I then had to make sure that `AdaptDispatch` actually included all of the functionality currently in `TerminalDispatch`. That meant copying across the code for bracketed paste mode, the copy to clipboard operation, and the various ConEmu OSC operations. This also required a few new methods to the `ConGetSet`/`ITerminalApi` interface, but for now these are just stubs in conhost.
Then there were a few thing in the api interface that needed cleaning up. The `ReparentWindow` method doesn't belong there, so I've moved that into `PtySignalInputThread` class. And the `WriteInput` method was too low-level for the Terminal requirements, so I've replaced that with a `ReturnResponse` method which takes a `wstring_view`.
It was then a matter of getting the `Terminal` class to implement all the methods in the new `ITerminalApi` interface that it didn't already have. This was mostly mapping to existing functionality, but there are still a number of methods that I've had to leave as stubs for now. However, what we have is still good enough that I could then nuke the `TerminalDispatch` class from the Terminal code and replace it with `AdaptDispatch`.
One oddity that came up in testing, though, was the `AdaptDispatch` implementation of `EraseAll` would push a blank line into the scrollback when called on an empty buffer, whereas the previous terminal implementation did not. That caused problems for the conpty connection, because one of the first things it does on startup is send an `ED 2` sequence. I've now updated the `AdaptDispatch` implementation to match the behavior of the terminal implementation in that regard.
Another problem was that the terminal implementation of the color table commands had special handling for the background color to notify the application window that it needed to repaint the background. I didn't want to have to push the color table operations through the `ITerminalApi` interface, so I've instead moved the handling of the background update into the renderer, initiated by a flag on the `TriggerRefreshAll` method.
## Validation Steps Performed
Surprisingly this PR didn't require a lot of changes to get the unit tests working again. There were just a few methods used from the original `ITerminalApi` that have now been removed, and which needed an equivalent replacement. Also the updated behavior of the `EraseAll` method in conhost resulted in a change to the expected cursor position in one of the screen buffer tests.
In terms of manual testing, I've tried out all the different shells in Windows Terminal to make sure there wasn't anything obviously wrong. And I've run a bunch of the tests from _vttest_ to try and get a wider coverage of the VT functionality, and confirmed everything still works at least as well as it used to. I've also run some of my own tests to verify the operations that had to be copied from `TerminalDispatch` to `AdaptDispatch`.
Propagate show/hide window calls against the ConPTY pseudo window to the Terminal
## PR Checklist
* [x] Closes#12570
* [x] I work here
* [x] Manual Tests passed
* [x] Spec Link: →[Doc Link](https://github.com/microsoft/terminal/blob/dev/miniksa/msgs/doc/specs/%2312570%20-%20Show%20Hide%20operations%20on%20GetConsoleWindow%20via%20PTY.md)←
## Detailed Description of the Pull Request / Additional comments
- See the spec. It's pretty much everything I went through deciding on this.
## Validation Steps Performed
- [x] Manual validation against scratch application calling all of the `::ShowWindow` commands against the pseudo console "fake window" and observing the real terminal window state
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Fixes#6028
Setting is "experimental.useBackgroundImageForWindow"
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
https://github.com/microsoft/terminal/issues/6028
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] Closes#6028
* [X] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated. I read CONTRIBUTING.md, but I'm not sure if a spec is needed for an experimental feature such as this one.
* [ ] Schema updated. I added a JSON key, not sure where I need to update it.
* [X] I've discussed this with core contributors already. Somewhat discussed in https://github.com/microsoft/terminal/issues/6028
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here
## Detailed Description of the Pull Request / Additional comments -->
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Set ` "experimental.useBackgroundImageForWindow": true` and a bg image for one profile, then make splits and tabs and make sure the bg updates accordingly:
I also did the same with the setting off to make sure it still works correctly and didn't break. And I made sure opening the settings tab does not crash or show the bg image.
#4015 requires sweeping changes in order to allow a migration of our buffer
coordinates from `int16_t` to `int32_t`. This commit reduces the size of
future commits by using type inference wherever possible, dropping the
need to manually adjust types throughout the project later.
As an added bonus this commit standardizes the alignment of cv qualifiers
to be always left of the type (e.g. `const T&` instead of `T const&`).
The migration to type inference with `auto` was mostly done
using JetBrains Resharper with some manual intervention and the
standardization of cv qualifier alignment using clang-format 14.
## References
This is preparation work for #4015.
## Validation Steps Performed
* Tests pass ✅
Further builds on #12799. #12799 assumes that the connection is prepared to receive FocusIn/FocusOut events as input. For ConPTY we can be relatively sure of that, but that's not _technically_ correct. In the hypothetical world where the connection is not a ConPTY connection, then the other side might not be expecting those sequences.
This remedies the issue by
* ConPTY will always request focus event mode (from the terminal) when it starts up
* when a client tries to disable focus events in conpty, conpty is gonna note that internally, but never transmit that to the hosting terminal, to leave the terminal in focus event mode.
* `TerminalDispatch` and `ControlCore` are hooked up now to only send focus events when the Terminal is in focus event mode (which will be always for conpty)
* At this point, it was like, 4LOC in `terminalInput.cpp` to add support for focus events to conhost as well.
## checklist
* [x] closes#11682
* This combined with #12515 will finally close out #2988 as well, but we can do that manually.
* [x] I work here
* [ ] There aren't tests for this. There probably should be.
## Window shenanigans, part the third:
Hooks the Terminal's focus state up to the underlying ConPTY. This is LOAD BEARING for allowing windows created by console applications to bring themselves to the foreground.
We're using the [FocusIn/FocusOut](https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-FocusIn_FocusOut) sequences to communicate to ConPTY when a control gains/loses focus. Theoretically, other terminals could do this as well.
## References
#11682 tracks _real_ support for this sequence in Console & conpty. When we do that, we should consider even if a client application disables this mode, the Terminal & conpty should always request this from the hosting terminal (and just ignore internally to ConPTY).
See also #12515, #12526, which are the other two parts of this effort. This was tested with all three merged together, and they worked beautifully for all our scenarios. They are kept separate for ease of review.
## PR Checklist
* [x] This is prototype 3 for #2988
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
This allows windows spawned by console processes to bring themselves to the foreground _when the console is focused_. (Historically, this is also called in the WndProc, when focus changes).
Notably, before this, ConPTY was _never_ focused, so windows could never bring themselves to the foreground when run from a ConPTY console. We're not blanket granting the SetForeground right to all console apps when run in ConPTY. It's the responsibility of the hosting terminal emulator to always tell ConPTY when a particular instance is focused.
## Validation Steps Performed
(gif below)
"Alternate scroll mode" is a neat little mode where the app wants mouse wheel events to come through as arrow keypresses instead, when in the alternate buffer. Now that we've got support for the alt buffer in the Terminal, we can support this as well.
* [x] Closes https://github.com/microsoft/terminal/issues/3321
* [x] I work here
* [ ] Tests would be nice
Tested manually with
```bash
printf "\e[?1007h" ; man ps
```
## Window shenanigans, part the first:
This PR enables terminals to tell ConPTY what the owning window for the
pseudo window should be. This allows thigs like MessageBoxes created by
console applications to work. It also enables console apps to use
`GetAncestor(GetConsoleWindow(), GA_ROOT)` to get directly at the HWND
of the Terminal (but _don't please_).
This is tested with our internal partners and seems to work for their
scenario.
See #2988, #12799, #12515, #12570.
## PR Checklist
This is 1/3 of #2988.
Make a VTApiRoutines servicer that does minimal translations instead of
environmental simulation for some output methods. Remaining methods are
backed on the existing console host infrastructure (primarily input
related methods).
## PR Checklist
* [x] I work here
* [x] It's Fix-Hack-Learn quality so it's behind a feature gate so we
can keep refining it. But it's a start!
To turn this on, you will have to be in the Dev or Preview rings
(feature staged). Then add `experimental.connection.passthroughMode:
true` to a profile and on the next launch, the flags will propagate down
through the `ConptyConnection` into the underlying `Openconsole.exe`
startup and tell it to use the passthrough mode instead of the full
simulation mode.
## Validation Steps Performed
- Played with it manually in CMD.exe, it seems to work mostly.
- Played with it manually in Ubuntu WSL, it seems to work.
- Played with it manually in Powershell and it's mostly sad. It'll get
there.
Starts #1173
Previously we would only call `SetWindowSize` and `TriggerRedrawAll` if the
viewport size in characters changed. This commit removes the limitation.
Since the if-condition limiting full redraws is now gone, this commit
moves the responsibility of limiting the calls up the call chain.
With `_refreshSizeUnderLock` now being a heavier function call
than before, some surrounding code was thus refactored.
## PR Checklist
* [x] Closes#11317
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
Test relevant for #11317:
* Print text, filling the entire window
* Move the window from a 150% scale to a 300% scale monitor
* The application works as expected ✅
Regression tests:
* Text zoom with Ctrl+Plus/Minus/0 works as before ✅
* Resizing a window works as before ✅
* No deadlocks, etc. during settings updates ✅
## Summary of the Pull Request
The purpose of the `IRenderTarget` interface was to support the concept
of multiple buffers in conhost. When a text buffer needed to trigger a
redraw, the render target implementation would be responsible for
deciding whether to forward that redraw to the renderer, depending on
whether the buffer was active or not.
This PR instead introduces a flag in the `TextBuffer` class to track
whether it is active or not, and it can then simply check the flag to
decide whether it needs to trigger a redraw or not. That way it can work
with the `Renderer` directly, and we can avoid a bunch of virtual calls
for the various redraw methods.
## PR Checklist
* [x] Closes#12551
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #12551
## Detailed Description of the Pull Request / Additional comments
Anywhere that had previously been getting an `IRenderTarget` from the
`TextBuffer` class in order to trigger a redraw, will now just call the
`TriggerRedraw` method on the `TextBuffer` class itself, since that will
handle the active check which used to be the responsibility of the
render target. All the redraw methods that were in `IRenderTarget` are
now exposed in `TextBuffer` instead.
For this to work, though, the `Renderer` needed to be available before
the `TextBuffer` could be constructed, which required a change in the
conhost initialization order. In the `ConsoleInitializeConnectInfo`
function, I had to move the `Renderer` initialization up so it could be
constructed before the call to `SetUpConsole` (which initializes the
buffer). Once both are ready, the `Renderer::EnablePainting` method is
called to start the render thread.
The other catch is that the renderer used to setup its initial viewport
in the constructor, but with the new initialization order, the viewport
size would not be known at that point. I've addressed that problem by
moving the viewport initialization into the `EnablePainting` method,
since that will only be called after the buffer is setup.
## Validation Steps Performed
The changes in architecture required a number of tweaks to the unit
tests. Some were using a dummy `IRenderTarget` implementation which now
needed to be replaced with a full `Renderer` (albeit with mostly null
parameters). In the case of the scroll test, a mock `IRenderTarget` was
used to track the scroll delta, which now had to be replaced with a mock
`RenderEngine` instead.
Some tests previously relied on having just a `TextBuffer` but without a
`Renderer`, and now they require both. And tests that were constructing
the `TextBuffer` and `Renderer` themselves had to be updated to use the
new initialization order, i.e. with the renderer constructed first.
Semantically, though, the tests still essentially work the same way as
they did before, and they all still pass.
When the dpi is changed, call `updateFont()` instead of `TriggerFontChange`, this
means that we continue to use the existing font features/axes
Closes#11287
Basically, this is the same as #12266, but for the `SearchBoxControl`. Trickily, the ControlCore is the one that knows if there were search results, but the TermControl has to be the one to announce it.
* [x] Will take care of #11973 once a11y team confirms
* [x] Tested manually with Narrator
* [x] Resolves a part of #6319, which I'm repurposing just to displaying the number of results in general.
* See also #3920
Since `AtlasEngine` prefers drawing without alpha for performance reasons,
calls to `EnableTransparentBackground` require a draw cycle.
This PR makes `Renderer::_NotifyPaintFrame` public and calls it.
Related to #9999.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Open Windows Terminal command palette
* Choose "Set background opacity..."
* Cycling through the items without selecting one
changes background opacity immediately ✅
This commit correctly restores the previous opacity when the command palette
preview is cancelled. It includes an additional change in order to make the
`AdjustOpacity` setter consistent and symmetric with the `Opacity` getter.
## PR Checklist
* [x] Closes#12228
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Open Windows Terminal command palette
* Choose "Set background opacity..."
* Cycle through the items without selecting one
* Press Escape
* Previous opacity is restored ✅
## Summary of the Pull Request
This PR moves the color table and related render settings, which are common to both conhost and Windows Terminal, into a shared class that can be accessed directly from the renderer. This avoids the overhead of having to look up these properties via the `IRenderData` interface, which relies on inefficient virtual function calls.
This also introduces the concept of color aliases, which determine the position in the color table that colors like the default foreground and background are stored. This allows the option of mapping them to one of the standard 16 colors, or to have their own separate table entries.
## References
This is a continuation of the color table refactoring started in #11602 and #11784. The color alias functionality is a prerequisite for supporting a default bold color as proposed in #11939. The color aliases could also be a way for us to replace the PowerShell color quirk for #6807.
## PR Checklist
* [x] Closes#12002
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
In addition to the color table, this new `RenderSettings` class manages the blinking state, the code for adjusting indistinguishable colors, and various boolean properties used in the color calculations. These boolean properties are now stored in a `til::enumset` so they can all be managed through a single `SetRenderMode` API, and easily extended with additional modes that we're likely to need in the future.
In Windows Terminal we have an instance of `RenderSettings` stored in the `Terminal` class, and in conhost it's stored in the `Settings` class. In both cases, a reference to this class is passed to the `Renderer` constructor, so it now has direct access to that data. The renderer can then pass that reference to the render engines where it's needed in the `UpdateDrawingBrushes` method.
This means the renderer no longer needs the `IRenderData` interface to access the `GetAttributeColors`, `GetCursorColor`, or `IsScreenReversed` methods, so those have now been removed. We still need access to `GetAttributeColors` in certain accessibility code, though, so I've kept that method in the `IUIAData` interface, but the implementation just forwards to the `RenderSettings` class.
The implementation of the `RenderSettings::GetAttributeColors` method is loosely based on the original `Terminal` code, only the `CalculateRgbColors` call has now been incorporated directly into the code. This let us deduplicate some bits that were previously repeated in the section for adjusting indistinguishable colors. The last steps, where we calculate the alpha components, have now been split in to a separate `GetAttributeColorsWithAlpha` method, since that's typically not needed.
## Validation Steps Performed
There were quite a lot changes needed in the unit tests, but they're mostly straightforward replacements of one method call with another.
In the `TextAttributeTests`, where we were previously testing the `CalculateRgbColors` method, we're now running those tests though `RenderSettings::GetAttributeColors`, which incorporates the same functionality. The only complication is when testing the `IntenseIsBright` option, that needs to be set with an additional `SetRenderMode` call where previously it was just a parameter on `CalculateRgbColors`.
In the `ScreenBufferTests` and `TextBufferTests`, calls to `LookupAttributeColors` have again been replaced by the `RenderSettings::GetAttributeColors` method, which serves the same purpose, and calls to `IsScreenReversed` have been replaced with an appropriate `GetRenderMode` call. In the `VtRendererTests`, all the calls to `UpdateDrawingBrushes` now just need to be passed a reference to a `RenderSettings` instance.
This commit makes the following changes to `til::point/size/rectangle`
for the following reasons:
* Rename `rectangle` into `rect`
This will make the naming consistent with a later `small_rect` struct
as well as the existing Win32 POINT/SIZE/RECT structs.
* Standardizes til wrappers on `int32_t` instead of `ptrdiff_t`
Provides a consistent behavior between x86 and x64, preventing accidental
errors on x86, as it's less rigorously tested than x64. Additionally it
improves interop with MIDL3 which only supports fixed width integer types.
* Standardizes til wrappers on throwing `gsl::narrow_error`
Makes the behavior of our code more consistent.
* Makes all eligible functions `constexpr`
Because why not.
* Removes implicit constructors and conversion operators
This is a complex and controversial topic. My reasons are: You can't Ctrl+F
for an implicit conversion. This breaks most non-IDE engines, like the one on
GitHub or those we have internally at MS. This is important for me as these
implicit conversion operators aren't cost free. Narrowing integers itself,
as well as the boundary checks that need to be done have a certain,
fixed overhead each time. Additionally the lack of noexcept prevents
many advanced compiler optimizations. Removing their use entirely
drops conhost's code segment size by around ~6.5%.
## References
Preliminary work for #4015.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
I'm mostly relying on our unit tests here. Both OpenConsole and WT appear to work fine.
Adds an action which can be used to change the opacity at runtime. This is a follow up to some of the other work I had been doing around opacity and settings previewing at the end of the year.
> edit: pseudo-spec
>
> * `adjustOpacity`:
> * `"opacity"`: (**required**) an integer number to set the opacity to.
> * `"relative"`: (defaults to `true`)
> * If false, set the opacity to the given value, which should be between [0, 100].
> * If true, then use `opacity` as a relative adjustment to the current opacity. So the implementation would get the current opacity for this pane, then add this action's opacity to that value. Should be between [-100, 100]
>
> This would allow both setting exactly 25% opacity with an action, and binding an action that increases/decreases the opacity one {amount}
It's preview-able too, which is neat.
* [x] Closes#11205
* [x] I work here
* [x] Docs updated: https://github.com/MicrosoftDocs/terminal/pull/477
When copying content from the terminal to the clipboard (with
formatting), a default background color needs to be set to fill the
unused area of the pasted block. Prior to this PR, that color was not
correctly set, so the pasted content did not match what was seen on
screen.
Windows Terminal previously used the default background from the initial
color scheme, so it didn't take palette changes into account.
OpenConsole did use the active default color, but didn't take the
reverse screen mode into account, so could end up using the foreground
rather than the background color.
In both case I've changed the code to lookup the runtime colors in the
same way that renderer does, so they should now match what is seen on
screen.
## Validation Steps Performed
I've manually confirmed that the background color is now correctly set
when copying from both Windows Terminal and OpenConsole.
Closes#11988
