This pull request brings us up to fmt 11.1.4 and enables `FMT_PEDANTIC`.
`FMT_PEDANTIC` turns on `/W3`, which is required by our local feudal
lords who will automatically file bugs on us if we don't build with
enough warnings enabled.
PR adds a WinGet configuration file to install the necessary
dependencies in order to build terminal locally. The configuration file
enables developer mode, installs PowerShell 7, Visual Studio 2022 & all
the required workloads from the .vsconfig file (in accordance with
the dependencies listed in the README).
## Validation Steps Performed
Tested the configuration file by spinning up a clean Win11 Pro VM in
azure and then doing the following:
1. Install latest WinGet on the VM using WinGet sandbox script.
Install git and clone the repo
2. Run `winget configure .config/configuration.winget` (this should work
by just double-clicking the file in explorer too)
3. After the configuration is completed, open the solution in the now
installed Visual Studio and build. The build is successful and I could
start terminal with F5
Co-authored-by: Demitrius Nelon <denelon@microsoft.com>
After taking in 1.22, our CodeQL process caught a few locations where we
weren't following the right guidance:
- Performing integer comparisons of different sizes which could lead to
an infinite loop if the larger integer goes out of range of the smaller
integer
- Not checking HResult of a called method
Co-authored-by: aphistra <102989060+aphistra@users.noreply.github.com>
Allow users to preview color schemes by hovering over them with the
mouse pointer in the Command Palette.
- This PR handles issue #18238.
- This extends the previously closed issue #6689, which allowed the `UP`
and `DOWN` arrows to trigger a preview of color schemes in the Command
Palette.
This works by attaching event handlers for `PointerEntered` and
`PointerExited` to `ListViewItem` containers. When the mouse pointer
moves into the item's bounding area, the `PreviewAction` handler is
triggered to showcase the hovered color scheme. Conversely, when the
mouse pointer leaves the item's area, the `PreviewAction` is executed on
the selected item (generally from the `UP` and `DOWN` arrows).
**Important note:**
- This also provides previews for the other features that the
`ActionPreviewHandler` handles, such as the background opacity of the
terminal.
## Validation Steps Performed
- Hover a color scheme, and it becomes the active one.
- Pressing `ESC` at any point to dismiss the command palette, and the
scheme returns to the previous one.
- I did not add any additional test, though all existing ColorScheme
tests passed.
Closes#18238
I found multiple issues while investigating this:
* Render thread shutdown is racy, because it doesn't actually stop the
render thread.
* Lifetime management in `ControlCore` failed to account for the
circular dependency of render thread --> renderer --> render data -->
terminal --> renderer --> render thread. Fixed by reordering the
`ControlCore` members to ensure their correct destruction.
* Ensured that the connection setter calls close on the previous
connection.
(Hopefully) Closes#18598
## Validation Steps Performed
* Can't repro the original failure ❌
* Opening and closing tabs as fast as possible doesn't crash anymore ✅
* Detaching and reattaching a tab producing continuous output ✅
It turns out that we *can* support language overrides--fairly easily, in
fact!--by simply changing the default Language qualifier.
I elected not to change how packaged language override works until we
are certain this works properly everywhere. Consider it a healthy
distrust of the Windows App Platform.
Closes#18419Closes#18336Closes#17619
Since `WaitForDA1` would wait until `_deviceAttributes` is non-zero,
we must ensure it's actually non-zero at the end of this handler,
even if there are no parameters.
## Validation Steps Performed
* Mod the Terminal DA1 to be `\x1b[?6c`. No hang ✅
* Mod the Terminal DA1 to be `\x1b[?61c`. No hang ✅
I've received a dump from an affected user, and it showed that the
layout event in TerminalPage was raised synchronously. This meant that
during page initialization, the handoff listener was started while still
being stuck inside the handoff listener. This resulted in a deadlock.
This PR fixes the issue by not holding the lock across handoff callback
calls.
Closes#18634
## Validation Steps Performed
* Can't repro ❌
This can be considered "part 1" of fixing #18599: It prevents crashes
(due to unhandled exceptions) by ensuring we only create 1 content
dialog across all windows at a time. Sounds bad, but I tried it and it's
not actually _that_ bad in practice (it's still really gross though).
The bad news is that I don't have a "part 2", because I can't figure out
what's going on:
* Create 2 windows
* Open the About dialog in window 1
and right click the text
* Close the About dialog
* Open the About dialog in window 2
and right click the text
* WinUI will simply toss the focus to window 1
It appears as if context menus are permanently associated with the first
window that uses them. It has nothing to do with whether a ContentDialog
instance is reused (I tested that).
## Validation Steps Performed
* Open 2 windows with 2 tabs each
* Attempt to close window 1, dialog appears ✅
* Attempt to close window 2, dialog moves to window 2 ✅
The logic didn't work when persistence was enabled and you had 2 windows
and closed the 2nd one, or when dragging the last tab out of the only
window.
## Validation Steps Performed
* 2 windows, close the 2nd one, app doesn't exit ✅
* 1 window, 1 tab, drag the tab out of the window, app doesn't exit ✅
I don't actually know why this is happening, because it doesn't
happen with startup actions specified in the settings file.
In any case, it's fixed with more delays.
Closes#18572
## Validation Steps Performed
* Create a tab with 2 panes
* Tear it off into a new window
* New window has 1 tab with 2 panes ✅
Missed a few `_getTerminalPosition()` on the first run. Disabled
rounding for pointer movements and mouse wheel events (which are used
for hyperlink hover detection and vt mouse mode). The only time we round
now is...
- `SetEndSelectionPoint()` --> because we're updating a selection
- `ControlCore->LeftClickOnTerminal()` --> where all paths are used for
selection*
*the only path that doesn't is `RepositionCursorWithMouse` being
enabled, which also makes sense based on clicking around Notepad with a
large font size.
## References and Relevant Issues
Follow-up for #18486Closes#18595
## Validation Steps Performed
In large font size, play around with midnight commander and hover over
hyperlink edges.
* `_ApplyLanguageSettingChange` calls `PrimaryLanguageOverride`
(the WinRT API function) and we would call it every time a new
window is created. Now it's only called on settings load.
* `_RegisterTabEvents` would listen for "Content" changes which can
be null. `IVector::Append` throws if a null object is given.
In our case, it's null if the content got erased with nothing.
Additionally, this fixes a bug where we wouldn't call
`_ProcessLazySettingsChanges` on startup. This is important if the
settings file was changed while Windows Terminal wasn't running.
Lastly, there's a lifetime fix in this PR, which is a one-line change
and I didn't want to make a separate PR for that.
When the server handle gets closed on conhost (= terminal is gone),
and e.g. PowerShell is used, we would previously log 6 error messages.
This PR reduces it to zero, by removing the 3 biggest offenders.
## Summary of the Pull Request
There's already logic to tab to a hyperlink when we're in mark mode. We
do this by looking at the automatically detected hyperlinks and finding
the next one of interest. This adds an extra step afterwards to find any
embedded hyperlinks and tab to them too.
Since embedded hyperlinks are stored as text attributes, we need to
iterate through the buffer to find the hyperlink and it's buffer
boundaries. This PR tries to reduce the workload of that by first
finding the automatically detected hyperlinks (since that's a fairly
quick process), then using the reduced search area to find the embedded
hyperlink (if one exists).
## Validation Steps Performed
In PowerShell, add an embedded hyperlink as such:
```powershell
${ESC}=[char]27
Write-Host "${ESC}]8;;https://github.com/microsoft/terminal${ESC}\This is a link!${ESC}]8;;${ESC}\"
```
Enter mark mode (ctrl+shift+m) then shift+tab to it.
✅ The "This is a link!" is selected
✅ Verified that this works when searching forwards and backwards
Closes#18310Closes#15194
Follow-up from #13405
OSC 8 support added in #7251
WinUI asynchronously updates its tab view items, so it may happen that
we're given a `TabViewItem` that still contains a `TabBase` which has
actually already been removed. Regressed in #15924.
Closes#18581
## Validation Steps Performed
* Close tabs rapidly with middle click
* No crash ✅
This is a theoretical fix for #18584 as I cannot reproduce the issue
anymore. It did happen briefly on one of my devices though, and at the
time I observed that it would persist a window with no startup actions.
Found this one completely randomly.
## Validation Steps Performed
* Open 2 windows with 1 tab each
* Click the X button on the tab in the 1st window
* OpenConsole/etc. is cleaned up ✅
---------
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
During startup we relinquish ownership of the console lock to wait for
the DA1 response of the hosting terminal. The problem occurs if the
hosting terminal disconnects during that time. The broken pipe will
cause `VtIo` to send out `CTRL_CLOSE_EVENT` messages, but those won't
achieve anything, because the first and only client hasn't even finished
connecting yet. What we need to do instead is to return an error code.
In order to not use a bunch of booleans to control this behavior, I gave
`VtIo` a state enum. This however required restructuring the calling
code in order to not have a dozen states.
## Validation Steps Performed
* Launch cmd.exe with ConPTY
* ...but leave the stdin pipe unbound (which will hang the DA1 request)
* Immediately kill the ConPTY session
* cmd.exe exits after clicking away the error message ✅
Before we had a Settings UI, we added support for a setting called
`startOnUserLogin`. It was a boolean, and on startup we would try to
yeet the value of that setting into the Windows API responsible for
registering us as a startup task.
Unfortunately, we failed to take into account a few things.
- Startup tasks can be independently controlled by the user in Windows
Settings or by an enterprise using enterprise policy
- This control is not limited to *disabling* the task; it also supports
enabling it!
Users could enable our startup task outside the settings file and we
would never know it. We would load up, see that `startOnUserLogin` was
`false`, and go disable the task again. 🤦
Conversely, if the user disables our task outside the app _we can never
enable it from inside the app._ If an enterprise has configured it
either direction, we can't change it either.
The best way forward is to remove it from our settings model and only
ever interact with the Windows API.
This pull request replaces `startOnUserLogin` with a rich settings
experience that will reflect the current and final state of the task as
configured through Windows. Terminal will enable it if it can and
display a message if it can't.
My first attempt at this PR (which you can read in the commit history)
made us try harder to sync the state between the settings model and the
OS; we would propagate the disabled state back to the user setting when
the task was disabled in the OS or if we failed to enable it when the
user asked for it. That was fragile and didn't support reporting the
state in the settings UI, and it seems like it would be confusing for a
setting to silently turn itself back off anyway...
Closes#12564
Fixes an issue on Windows 10 where icon on selected color chips would be
missing in the NullableColorPicker.
Fixes (or at least significantly improves the experience) text being
truncated for the special colors in the NullableColorPicker. This was
done by removing the word "Use" from the labels and adding a visual
state trigger to change the layout of the chips and buttons when the
window becomes narrow.
Related to #18318
Campbell has been the default color scheme for a long time now,
but it has quite some issues with hue and chroma.
This PR introduces a new scheme which was created using the Oklab
color space to find colors with maximal distance to each other
and well distributed and consistent hue and chroma.
Because of this, I've named the scheme after the creator of Oklab.
Closes#17818
When Sixel images are rendered, they're automatically scaled to match
the 10x20 cell size of the original hardware terminals. If this requires
the image to be scaled down, the default GDI stretching mode can produce
ugly visual artefacts, particularly for color images. This PR changes
the stretching mode to `COLORONCOLOR`, which looks considerably better,
but without impacting performance.
The initial Sixel implementation was added in PR #17421.
## Validation Steps Performed
I've tested with a number of different images using a small font size to
trigger the downscaling, and I think the results are generally better,
although simple black on white images are still better with the default
mode (i.e. `BLACKONWHITE`), which is understandable.
I've also checked the performance with a variation of the [sixel-bench]
test, and confirmed that the new mode is no worse than the default.
[sixel-bench]: https://github.com/jerch/sixel-bench
Fixes an issue where pressing `CTRL` + `Insert` does not copy text
selected in the Command Palette. Instead, it closes it, and any text
selected in the pane is copied to the clipboard.
Since `Insert` is a virtual key, I address the issue by adding a
conditional check for `CTRL` with either `Insert` or `C` (previously, it
only checked for `CTRL` with `C`) for the copy action in the Command
Palette.
## Validation Steps Performed
I followed the reproduction steps and verified that the actual behaviour
matched the expected behaviour. All existing tests passed, but no new
test was added.
Closes#9520
## Summary of the Pull Request
Fixes a bug where VT mouse mode would round to the nearest cell when
clicking the mouse button.
The fix is to round to the nearest cell only when we're selecting text.
The other scenarios affected are:
- clicking on a hyperlink
- vt mouse mode
- where the context menu is anchored
Really the most notable ones were the first two. So now, we use the
position of the cell we clicked on. We only round for selection.
## References and Relevant Issues
Follow-up to #18106
## Detailed Description of the Pull Request / Additional comments
## Validation Steps Performed
Opened Midnight Commander in Ubuntu and clicked between the two panes.
- Before: threshold was too early to switch between panes
- After: threshold is clearly separated between the outline of the two
panes
---------
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
This PR allows users to enable the tab bar in fullscreen mode.
A new setting; "showTabsFullscreen"; has been added which accepts a
boolean value. When `true`, then the tab bar will remain visible when
the terminal app is fullscreen. If the value is `false` (default), then
the tab bar is hidden in fullscreen.
When the tab bar is visible in fullscreen, the min/max/close controls
are hidden to maintain the expected behaviour of a fullscreen app.
## Validation Steps Performed
All unit tests are passing.
Manually verified that when the "launchMode" setting is "fullscreen" and
the "showTabsFullscreen" setting is `true`, the tab bar is visible on
launch.
Manually verified that changing the setting at runtime causes the tab
bar to be shown/hidden immediately (if the terminal is currently
fullscreen).
Manually verified that the new "showTabsFullscreen" setting is honoured
regardless of whether "showTabsInTitlebar" is set to `true` or `false`.
Closes#11130
Selection is generally stored as an inclusive start and end. This PR
makes the end exclusive which now allows degenerate selections, namely
in mark mode. This also modifies mouse selection to round to the nearest
cell boundary (see #5099) and improves word boundaries to be a bit more
modern and make sense for degenerate selections (similar to #15787).
Closes#5099Closes#13447Closes#17892
## Detailed Description of the Pull Request / Additional comments
- Buffer, Viewport, and Point
- Introduced a few new functions here to find word boundaries, delimiter
class runs, and glyph boundaries.
- 📝These new functions should be able to replace a few other functions
(i.e. `GetWordStart` --> `GetWordStart2`). That migration is going to be
a part of #4423 to reduce the risk of breaking UIA.
- Viewport: added a few functions to handle navigating the _exclusive_
bounds (namely allowing RightExclusive as a position for buffer
coordinates). This is important for selection to be able to highlight
the entire line.
- 📝`BottomInclusiveRightExclusive()` will replace `EndExclusive` in the
UIA code
- Point: `iterate_rows_exclusive` is similar to `iterate_rows`, except
it has handling for RightExclusive
- Renderer
- Use `iterate_rows_exclusive` for proper handling (this actually fixed
a lot of our issues)
- Remove some workarounds in `_drawHighlighted` (this is a boundary
where we got inclusive coords and made them exclusive, but now we don't
need that!)
- Terminal
- fix selection marker rendering
- `_ConvertToBufferCell()`: add a param to allow for RightExclusive or
clamp it to RightInclusive (original behavior). Both are useful!
- Use new `GetWordStart2` and `GetWordEnd2` to improve word boundaries
and make them feel right now that the selection an exclusive range.
- Convert a few `IsInBounds` --> `IsInExclusiveBounds` for safety and
correctness
- Add `TriggerSelection` to `SelectNewRegion`
- 📝 We normally called `TriggerSelection` in a different layer, but it
turns out, UIA's `Select` function wouldn't actually update the
renderer. Whoops! This fixes that.
- TermControl
- `_getTerminalPosition` now has a new param to round to the nearest
cell (see #5099)
- UIA
- `TermControlUIAProvider::GetSelectionRange` no need to convert from
inclusive range to exclusive range anymore!
- `TextBuffer::GetPlainText` now works on an exclusive range, so no need
to convert the range anymore!
## Validation Steps Performed
This fundamental change impacts a lot of scenarios:
- ✅Rendering selections
- ✅Selection markers
- ✅Copy text
- ✅Session restore
- ✅Mark mode navigation (i.e. character, word, line, buffer)
- ✅Mouse selection (i.e. click+drag, shift+click, multi-click,
alt+click)
- ✅Hyperlinks (interaction and rendering)
- ✅Accessibility (i.e. get selection, movement, text extraction,
selecting text)
- [ ] Prev/Next Command/Output (untested)
- ✅Unit tests
## Follow-ups
- Refs #4423
- Now that selection and UIA are both exclusive ranges, it should be a
lot easier to deduplicate code between selection and UIA. We should be
able to remove `EndExclusive` as well when we do that. This'll also be
an opportunity to modernize that code and use more `til` classes.
Apparently, we were using the package containing the CRT _source code_
to determine the version of the tools.
Also apparently, VS does not guarantee that that package has the same
version as the tools package.
We should use the version of the tools package instead.
As explained in the comment on `_getViewportCursorPosition`, printing
to stdout after initiating a cooked stdin reads is a race condition
between the application and the terminal. But we can significantly
reduce the likelihood of this being obvious with this change.
Related to #18265
Possibly related to #18081
## Validation Steps Performed
Execute the following Go code and start typing:
```go
package main
import (
"fmt"
"time"
)
func main() {
go func() {
time.Sleep(50 * time.Millisecond)
fmt.Printf("Here is a prompt! >")
}()
var text string
fmt.Scanln(&text)
}
```
Without this change the prompt will disappear,
and with this change in place, it'll work as expected. ✅
The code in #17909 was not completely right for padding values with
fewer than four components, and it was doing some fragile string math
(that is: if you wanted to change the third element in the padding it
would parse out the whole thing, edit the third value, and then format
it again).
This pull request moves the control's padding parser into cppwinrt_utils
(for lack of a better place) and makes the settings UI use it to parse
the padding out into a `Thickness` as early as possible. Then, the
controls operate directly on the Thickness' members rather than parsing
the padding string again.
To handle two-way serialization properly, we also required a function
that converts a thickness back into a reduced string representation
(i.e. when all four values are N, it will return "N").
As a bonus, this pull request also:
- removes another use of `std::getline`
- fixes an issue where resetting the padding would change it
(infinitesimally) and cause it to be set again
- adds a readout of the current padding value in the expander itself
- removes `MaxValueFromPaddingString`, which was apparently unused
This pull request introduces a new profile setting,
`compatibility.allowOSC52`, which defaults to `true`. When disabled, it
will not allow applications to write to the clipboard.
Security-minded folks may choose to disable it.
Reroutes the `closeWindow` action to use the `CloseWindow()` method like
the window's X button does. This includes logic to display the
confirmation dialog.
Also removes `CloseRequested` as it was only used by this action
handler. We already have `CloseWindowRequested` so we're just using that
instead.
## Validation Steps Performed
✅ `closeWindow` action while multiple tabs opened brings up the
confirmation dialog
Closes#17613
