## Summary of the Pull Request
This PR aims to move the command palette spec out of the draft state and into a finalized state for inclusion in the 2.0 version of the Windows Terminal.
Notably, I've added sections regarding the ability to run `wt` commandlines using the Command Palette UI, something we hadn't considered in the original draft, because `wt` commandlines didn't land for like _4 months_ after this first draft.
## References
* #2046 - the original command palette thread
* #2193 - the original draft PR
* #5400 - the new command palette megathread for WT 2.0, which I'll be updating with follow-up tasks as we work on implementing this.
## PR Checklist
* [x] Specs #2046
* [x] I work here
* [x] Is documentation
## Detailed Description of the Pull Request / Additional comments
_read the spec_
Does what it says on the label. Pure modifier keys weren't making it
this far at all prior to #6309. This PR changes these methods to make
sure that we only dismiss a selection or snap on input when the key
pressed isn't a modifier key.
## References
* regressed in #6309
## PR Checklist
* [x] Closes#6423
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Tried to repro this in the Terminal, couldn't anymore.
<!-- 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
Upload the roadmap for Windows Terminal 2.0 and link to it on the README.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- 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
Wildcards are not allowed in toplevel ItemGroups in vcxproj; they must
be generated by targets.
We mostly use wildcards for pulling in PRI files that are dumped on disk
by the translation tool. We don't want to check those in, so we can't
expand references to them.
To that end, I've introduced a new target that will take a list of
folders containing resw files and expand wildcards under them.
All[1] other wildcards have been moved into their respective targets
_or_ simply expanded.
[1]: Nothing has complained about the resource wildcards in
CascadiaResources.build.items, so I haven't exploded it yet.
Fixes#6214.
This PR provides a faster algorithm for converting 8-bit and 24-bit
colors into the 4-bit legacy values that are required by the Win32
console APIs. It also fixes areas of the code that were incorrectly
using a simple 16-color conversion that didn't handle 8-bit and 24-bit
values.
The faster conversion algorithm should be an improvement for issues #783
and #3950.
One of the main points of this PR was to fix the
`ReadConsoleOutputAttribute` API, which was using a simplified legacy
color conversion (the original `TextAttribute:GetLegacyAttributes`
method), which could only handle values from the 16-color table. RGB
values, and colors from the 256-color table, would be mapped to
completely nonsensical values. This API has now been updated to use the
more correct `Settings::GenerateLegacyAttributes` method.
But there were also a couple of other places in the code that were using
`GetLegacyAttributes` when they really had no reason to be working with
legacy attributes at all. This could result in colors being downgraded
to 4-bit values (often badly, as explained above), when the code was
already perfectly capable of displaying the full 24-bits.
This included the fill colors in the IME composer (in `ConsoleImeInfo`),
and the construction of the highlighting colors in the color
search/selection handler (`Selection::_HandleColorSelection`). I also
got rid of some legacy attribute code in the `Popup` class, which was
originally intended to update colors below the popup when the settings
changed, but actually caused more problems than it solved.
The other major goal of this PR was to improve the performance of the
`GenerateLegacyAttributes` method, since the existing implementation
could be quite slow when dealing with RGB values.
The simple cases are handled much the same as they were before. For an
`IsDefault` color, we get the default index from the
`Settings::_wFillAttribute` field. For an `IsIndex16` color, the index
can just be returned as is.
For an `IsRgb` color, the RGB components are compressed down to 8 bits
(3 red, 3 green, 2 blue), simply by dropping the least significant bits.
This 8-bit value is then used to lookup a representative 16-color value
from a hard-coded table. An `IsIndex256` color is also converted with a
lookup table, just using the existing 8-bit index.
The RGB mapping table was calculated by taking each compressed 8-bit
color, and picking a entry from the _Campbell_ palette that best
approximated that color. This was done by looking at a range of 24-bit
colors that mapped to the 8-bit value, finding the best _Campbell_ match
for each of them (using a [CIEDE2000] color difference calculation), and
then the most common match became the index that the 8-bit value would
map to.
The 256-color table was just a simpler version of this process. For each
entry in the table, we take the default RGB palette value, and find it's
closest match in the _Campbell_ palette.
Because these tables are hard-coded, the results won't adjust to changes
in the palette. However, they should still produce reasonable results
for palettes that follow the standard ANSI color range. And since
they're only a very loose approximation of the colors anyway, the exact
value really isn't that important.
That said, I have tried to make sure that if you take an RGB value for a
particular index in a reasonable color scheme, then the legacy color
mapped from that value should ideally match the same index. This will
never be possible for all color schemes, but I have tweaked a few of the
table entries to improve the results for some of the common schemes.
One other point worth making regarding the hard-coded tables: even if we
wanted to take the active palette into account, that wouldn't actually
be possible over a conpty connection, because we can't easily know what
color scheme the client application is using. At least this way the
results in conhost are guaranteed to be the same as in the Windows
Terminal.
[CIEDE2000]: https://en.wikipedia.org/wiki/Color_difference#CIEDE2000
## Validation Steps Performed
This code still passes the `TextAttributeTests` that check the basic
`GetLegacyAttribute` behaviour and verify the all legacy attributes
roundtrip correctly. However, some of the values in the `RgbColorTests`
had to be updated, since we're now intentionally returning different
values as a result of the changes to the RGB conversion algorithm.
I haven't added additional unit tests, but I have done a lot of manual
testing to see how well the new algorithm works with a range of colors
and a variety of different color schemes. It's not perfect in every
situation, but I think it works well enough for the purpose it serves.
I've also confirmed that the issues reported in #5940 and #6247 are now
fixed by these changes.
Closes#5940Closes#6247
This PR improves our VT character set support, enabling the [`SCS`]
escape sequences to designate into all four G-sets with both 94- and
96-character sets, and supports invoking those G-sets into both the GL
and GR areas of the code table, with [locking shifts] and [single
shifts]. It also adds [`DOCS`] sequences to switch between UTF-8 and the
ISO-2022 coding system (which is what the VT character sets require),
and adds support for a lot more characters sets, up to around the level
of a VT510.
[`SCS`]: https://vt100.net/docs/vt510-rm/SCS.html
[locking shifts]: https://vt100.net/docs/vt510-rm/LS.html
[single shifts]: https://vt100.net/docs/vt510-rm/SS.html
[`DOCS`]: https://en.wikipedia.org/wiki/ISO/IEC_2022#Interaction_with_other_coding_systems
## Detailed Description of the Pull Request / Additional comments
To make it easier for us to declare a bunch of character sets, I've made
a little `constexpr` class that can build up a mapping table from a base
character set (ASCII or Latin1), along with a collection of mappings for
the characters the deviate from the base set. Many of the character sets
are simple variations of ASCII, so they're easy to define this way.
This class then casts directly to a `wstring_view` which is how the
translation tables are represented in most of the code. We have an array
of four of these tables representing the four G-sets, two instances for
the active left and right tables, and one instance for the single shift
table.
Initially we had just one `DesignateCharset` method, which could select
the active character set. We now have two designate methods (for 94- and
96- character sets), and each takes a G-set number specifying the target
of the designation, and a pair of characters identifying the character
set that will be designated (at the higher VT levels, character sets are
often identified by more than one character).
There are then two new `LockingShift` methods to invoke these G-sets
into either the GL or GR area of the code table, and a `SingleShift`
method which invokes a G-set temporarily (for just the next character
that is output).
I should mention here that I had to make some changes to the state
machine to make these single shift sequences work. The problem is that
the input state machine treats `SS3` as the start of a control sequence,
while the output state machine needs it to be dispatched immediately
(it's literally the _Single Shift 3_ escape sequence). To make that
work, I've added a `ParseControlSequenceAfterSs3` callback in the
`IStateMachineEngine` interface to decide which behavior is appropriate.
When it comes to mapping a character, it's simply an array reference
into the appropriate `wstring_view` table. If the single shift table is
set, that takes preference. Otherwise the GL table is used for
characters in the range 0x20 to 0x7F, and the GR table for characters
0xA0 to 0xFF (technically some character sets will only map up to 0x7E
and 0xFE, but that's easily controlled by the length of the
`wstring_view`).
The `DEL` character is a bit of a special case. By default it's meant to
be ignored like the `NUL` character (it's essentially a time-fill
character). However, it's possible that it could be remapped to a
printable character in a 96-character set, so we need to check for that
after the translation. This is handled in the `AdaptDispatch::Print`
method, so it doesn't interfere with the primary `PrintString` code
path.
The biggest problem with this whole process, though, is that the GR
mappings only really make sense if you have access to the raw output,
but by the time the output gets to us, it would already have been
translated to Unicode by the active code page. And in the case of UTF-8,
the characters we eventually receive may originally have been composed
from two or more code points.
The way I've dealt with this was to disable the GR translations by
default, and then added support for a pair of ISO-2022 `DOCS` sequences,
which can switch the code page between UTF-8 and ISO-8859-1. When the
code page is ISO-8859-1, we're essentially receiving the raw output
bytes, so it's safe to enable the GR translations. This is not strictly
correct ISO-2022 behavior, and there are edge cases where it's not going
to work, but it's the best solution I could come up with.
## Validation Steps Performed
As a result of the `SS3` changes in the state machine engine, I've had
to move the existing `SS3` tests from the `OutputEngineTest` to the
`InputEngineTest`, otherwise they would now fail (technically they
should never have been output tests).
I've added no additional unit tests, but I have done a lot of manual
testing, and made sure we passed all the character set tests in Vttest
(at least for the character sets we currently support). Note that this
required a slightly hacked version of the app, since by default it
doesn't expose a lot of the test to low-level terminals, and we
currently identify as a VT100.
Closes#3377Closes#3487
## Summary of the Pull Request
We have a number of bugs in the Terminal that all have the same singular root cause - VT input does not carry the same fidelity that Win32 input does. For Win32 applications there are certain keystrokes that simply cannot be represented with VT sequences.
This is my proposal for how we'll handle all these cases. I'm proposing a _new VT sequence_, which will enable the Terminal to send input with all of the information that an `INPUT_RECORD` might have, to conpty. There, conpty will be able to send input to the client application with the same fidelity they're used to, enabling these keys to work for those applications once again.
## PR Checklist
* [x] Specs #4999
* [x] I work here
* [x] is a spec
## Detailed Description of the Pull Request / Additional comments
_read the spec_
This commit introduces Generate-CodepointWidthsFromUCD, a powershell
(7+) script that will parse a UCD XML database in the UAX 42 format from
https://www.unicode.org/Public/UCD/latest/ucdxml/ and generate
CodepointWidthDetector's giant width array.
By default, it will emit one UnicodeRange for every range of non-narrow
glyphs with a different Width + Emoji + Emoji Presentation class;
however, it can be run in "packing" and "full" mode.
* Packing mode: ignore the width/emoji/pres class and combine adjacent
runs that CPWD will treat the same.
* This is for optimizing the number of individual ranges emitted
into code.
* Full mode: include narrow codepoints (helpful for visualization)
It also supports overrides, provided in an XML document of the same format
as the UCD itself. Entries in the overrides files are applied after the
entire UCD is read and will replace any impacted ranges.
The output (when packing) looks like this:
```c++
// Generated by Generate-CodepointWidthsFromUCD -Pack:True -Full:False
// on 05/17/2020 02:47:55 (UTC) from Unicode 13.0.0.
// 66182 (0x10286) codepoints covered.
static constexpr std::array<UnicodeRange, 23> s_wideAndAmbiguousTable{
UnicodeRange{ 0xa1, 0xa1, CodepointWidth::Ambiguous },
UnicodeRange{ 0xa4, 0xa4, CodepointWidth::Ambiguous },
UnicodeRange{ 0xa7, 0xa8, CodepointWidth::Ambiguous },
.
.
.
UnicodeRange{ 0x1f210, 0x1f23b, CodepointWidth::Wide },
UnicodeRange{ 0x1f37e, 0x1f393, CodepointWidth::Wide },
UnicodeRange{ 0x100000, 0x10fffd, CodepointWidth::Ambiguous },
};
```
The output (when overriding) looks like this:
```c++
// Generated by Generate-CodepointWidthsFromUCD.ps1 -Pack:True -Full:False -NoOverrides:False
// on 5/22/2020 11:17:39 PM (UTC) from Unicode 13.0.0.
// 321205 (0x4E6B5) codepoints covered.
// 240 (0xF0) codepoints overridden.
static constexpr std::array<UnicodeRange, 23> s_wideAndAmbiguousTable{
UnicodeRange{ 0xa1, 0xa1, CodepointWidth::Ambiguous },
...
UnicodeRange{ 0xfe20, 0xfe2f, CodepointWidth::Narrow }, // narrow combining ligatures (split into left/right halves, which take 2 columns together)
...
UnicodeRange{ 0x100000, 0x10fffd, CodepointWidth::Ambiguous },
};
```
## Summary of the Pull Request
When resizing the window title, a GDI object would be leaked. This has to do with our island message handler using `wil` to track these objects and `wil` having a bug.
## References
microsoft/wil#100
## PR Checklist
* [x] Closes#5949
* [x] I work here.
* [x] Tested manually
* [x] Doc not required.
* [x] Am core contributor.
## Validation Steps Performed
* [x] Added the GDI Objects column to Task Manager, set the Terminal to use the `titleWidth` size tabs, then changed the title a bunch with PowerShell. Confirmed repro before (increasing GDI count). Confirmed it's gone after (no change to object count).
<!-- 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
Updates the check spelling action to [0.0.16-a](https://github.com/check-spelling/check-spelling/releases/tag/0.0.16-alpha)
* update advice -- [sample](57fc13f6c6 (commitcomment-39489723)) -- I really do encourage others to adjust it as desired
* rename `expect` (there are consumers who were not a fan of the `whitelist` nomenclature)
* prune stale items
* some `patterns` improvements to reduce the number of items in `expect`
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
⚠️ Anyone with an inflight addition of a new file to the `whitelist` directory will be moderately unhappy as the action would only use items from there if it didn't find `expect` (and this PR includes the rename).
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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
<!-- 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
Runs should be ~30s faster.
I was hoping to be able to offer the ability to talk to the bot, but sadly that feature is still not quite ready -- and I suspect that I may want to let projects opt in/out of that feature.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
* I added a commit with misspellings: 57fc13f6c6❌ and ran the command it suggested (in bash).
* The commit [itself passes its own testing](78df00dcf6) ✔️
The commands were never `cmd`/`psh` friendly. This iteration is designed to make it easier for a bot to parse and eventually do the work in response to a GitHub request, sadly that feature is behind schedule.
This removes all glyphs from the emoji list that do not default to
"emoji presentation" (EPres). It removes all local overrides, but retains
the comments about the emoji we left out that are Microsoft-specific.
This brings us fully in line with the most popular Terminals on OS X,
except that we squash our emoji down to fit in one cell and they let
them hang over the edges and damage other characters. Oh well.
## Detailed Description of the Pull Request / Additional comments
Late Friday evening, I tested my emoji test file on iTerm2. In so doing, I realized
that @j4james and @leonMSFT were right the entire time in #5914: Emoji
that require `U+FE0F` must not be double-width by default.
I finally banged up a powershell script that parses the UCD and emits a codepoint
width table. Once checked in, this will be definitive.
Refs #900, #5914.
Fixes#5941.
A couple of codepoints, namely the card suites, male and female signs,
and white and black smiling faces were changed to have a two-column
width as part of #5795 since they were specified as emoji in Unicode's
emoji list v13.0[1].
These particular glyphs also show up in some of the most fundamental
code pages, such as CP437[2] and WGL4[3]. We should
not be touching the width of the glyphs in these codepages, as suddenly
changing a long-time-running narrow glyph to use two-columns all of a
sudden will surely break (and has already broken) things.
[1] https://www.unicode.org/Public/13.0.0/ucd/emoji/emoji-data.txt
[2] https://en.wikipedia.org/wiki/Code_page_437
[3] https://en.wikipedia.org/wiki/Windows_Glyph_List_4Closes#5822
This is an attempt to simplify the SGR (Select Graphic Rendition)
implementation in conhost, to cut down on the number of methods required
in the `ConGetSet` interface, and pave the way for future improvements
and bug fixes. It already fixes one bug that prevented SGR 0 from being
correctly applied when combined with meta attributes.
* This a first step towards fixing the conpty narrowing bugs in issue
#2661
* I'm hoping the simplification of `ConGetSet` will also help with
#3849.
* Some of the `TextAttribute` refactoring in this PR overlaps with
similar work in PR #1978.
## Detailed Description of the Pull Request / Additional comments
The main point of this PR was to simplify the
`AdaptDispatch::SetGraphicsRendition` implementation. So instead of
having it call a half a dozen methods in the `ConGetSet` API, depending
on what kinds of attributes needed to be set, there is now just one call
to get current attributes, and another call to set the new value. All
adjustments to the attributes are made in the `AdaptDispatch` class, in
a simple switch statement.
To help with this refactoring, I also made some change to the
`TextAttribute` class to make it easier to work with. This included
adding a set of methods for setting (and getting) the individual
attribute flags, instead of having the calling code being exposed to the
internal attribute structures and messing with bit manipulation. I've
tried to get rid of any methods that were directly setting legacy, meta,
and extended attributes.
Other than the fix to the `SGR 0` bug, the `AdaptDispatch` refactoring
mostly follows the behaviour of the original code. In particular, it
still maps the `SGR 38/48` indexed colors to RGB instead of retaining
the index, which is what we ultimately need it to do. Fixing that will
first require the color tables to be unified (issue #1223), which I'm
hoping to address in a followup PR.
But for now, mapping the indexed colors to RGB values required adding an
an additional `ConGetSet` API to lookup the color table entries. In the
future that won't be necessary, but the API will still be useful for
other color reporting operations that we may want to support. I've made
this API, and the existing setter, standardise on index values being in
the "Xterm" order, since that'll be essential for unifying the code with
the terminal adapter one day.
I should also point out one minor change to the `SGR 38/48` behavior,
which is that out-of-range RGB colors are now ignored rather than being
clamped, since that matches the way Xterm works.
## Validation Steps Performed
This refactoring has obviously required corresponding changes to the
unit tests, but most were just minor updates to use the new
`TextAttribute` methods without any real change in behavior. However,
the adapter tests did require significant changes to accommodate the new
`ConGetSet` API. The basic structure of the tests remain the same, but
the simpler API has meant fewer values needed to be checked in each test
case. I think they are all still covering the areas there were intended
to, though, and they are all still passing.
Other than getting the unit tests to work, I've also done a bunch of
manual testing of my own. I've made sure the color tests in Vttest all
still work as well as they used to. And I've confirmed that the test
case from issue #5341 is now working correctly.
Closes#5341
The table that we refer to in `CodepointWidthDetector.cpp` to determine
whether or not a codepoint should be rendered as Wide vs Narrow was
based off EastAsianWidth[1]. If a codepoint wasn't included in this
table, they're considered Narrow. Many emojis aren't specified in the
EAW list, so this PR supplements our table with emoji codepoints from
emoji-data[2] in order to render most, if not all, emojis as full-width.
There are certain codepoints I've added to the comments (in case we want
to add them officially to the table in the future) that Microsoft
decided to give an emoji presentation even if it's specified as
Narrow/Ambiguous in the EAW list and are _not_ specified in the Unicode
emoji list. These include all of the Mahjong Tiles block, different
direction pencils (✎✐), different pointing index fingers (☜, ☞) among
others. I have no idea if I've captured all of them, as I don't know of
an easy way to detect which are Microsoft specific emojis.
## Validation Steps Performed
I have looked at so many emojis that I dream emoji.
These screenshots aren't encompassing _all_ emoji but I've tried to grab
a couple from all across the codepoint ranges:
Before:
After:
[1] http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt
[2] https://www.unicode.org/Public/13.0.0/ucd/emoji/emoji-data.txtCloses#900
## Summary of the Pull Request
This PR resolves an issue with the Git for Windows (MSYS) version of `less`. It _doesn't_ use VT processing for emitting text tothe buffer, so when it hits `WriteCharsLegacy`, `WC_DELAY_EOL_WRAP` is NOT set.
When this happens, `less` is writing some text that's longer than the width of the buffer to the last line of the buffer. We're hitting the
```c++
Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);
```
call in `_stream.cpp:560`.
The cursor is _currently_ at `{40, 29}`, the _start_ of the run of text that wrapped. We're trying to adjust it to `{0, 30}`, which would be the start of the next line of the buffer. However, the buffer is only 30 lines tall, so we've got to `IncrementCircularBuffer` first, so we can move the cursor there.
When that happens, we're going to paint frame. At the end of that frame, we're going to try and paint the cursor position. The cursor is still at `{40, 29}` here, so unfortunately, the `cursorIsInDeferredWrap` check in `XtermEngine::PaintCursor` is `false`. That means, conpty is going to try to move the cursor to where the console thinks the cursor actually is at the end of this frame, which is `{40, 29}`.
If we're painting the frame because we circled the buffer, then the cursor might still be in the position it was before the text was written to the buffer to cause the buffer to circle. In that case, then we DON'T want to paint the cursor here either, because it'll cause us to manually break this line. That's okay though, the frame will be painted again, after the circling is complete.
## PR Checklist
* [x] Closes#5691
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I suppose that's the detailed description above
## Validation Steps Performed
* ran tests
* checked that the bug was actually fixed in the Terminal
## Summary of the Pull Request
Identifies and scales glyphs in the box and line drawing ranges U+2500-U+259F to fit their cells.
## PR Checklist
* [x] Closes#455
* [x] I work here.
* [x] Manual tests. This is all graphical.
* [x] Metric ton of comments
* [x] Math spreadsheet included in PR.
* [x] Double check RTL glyphs.
* [x] Why is there the extra pixel?
* [x] Scrolling the mouse wheel check is done.
* [x] Not drawing outline?
* [x] Am core contributor. Roar.
* [x] Try suppressing negative scale factors and see if that gets rid of weird shading.
## Detailed Description of the Pull Request / Additional comments
### Background
- We want the Terminal to be fast at drawing. To be fast at drawing, we perform differential drawing, or only drawing what is different from the previous frame. We use DXGI's `Present1` method to help us with this as it helps us compose only the deltas onto the previous frame at drawing time and assists us in scrolling regions from the previous frame without intervention. However, it only works on strictly integer pixel row heights.
- Most of the hit testing and size-calculation logic in both the `conhost` and the Terminal products are based on the size of an individual cell. Historically, a cell was always dictated in a `COORD` structure, or two `SHORT` values... which are integers. As such, when we specify the space for any individual glyph to be displayed inside our terminal drawing region, we want it to fall perfectly inside of an integer box to ensure all these other algorithms work correctly and continue to do so.
- Finally, we want the Terminal to have font fallback and locate glyphs that aren't in the primary selected font from any other font it can find on the system that contains the glyph, per DirectWrite's font fallback mechanisms. These glyphs won't necessarily have the same font or glyph metrics as the base font, but we need them to fit inside the same cell dimensions as if they did because the hit testing and other algorithms aren't aware of which particular font is sourcing each glyph, just the dimensions of the bounding box per cell.
### How does Terminal deal with this?
- When we select a font, we perform some calculations using the design metrics of the font and glyphs to determine how we could fit them inside a cell with integer dimensions. Our process here is that we take the requested font size (which is generally a proxy for height), find the matching glyph width for that height then round it to an integer. We back convert from that now integer width to a height value which is almost certainly now a floating point number. But because we need an integer box value, we add line padding above and below the glyphs to ensure that the height is an integer as well as the width. Finally, we don't add the padding strictly equally. We attempt to align the English baseline of the glyph box directly onto an integer pixel multiple so most characters sit crisply on a line when displayed.
- Note that fonts and their glyphs have a prescribed baseline, line gap, and advance values. We use those as guidelines to get us started, but then to meet our requirements, we pad out from those. This results in fonts that should be properly authored showing gaps. It also results in fonts that are improperly authored looking even worse than they normally would.
### Now how does block and line drawing come in?
- Block and Line drawing glyphs are generally authored so they will look fine when the font and glyph metrics are followed exactly as prescribed by the font. (For some fonts, this still isn't true and we want them to look fine anyway.)
- When we add additional padding or rounding to make glyphs fit inside of a cell, we can be adding more space than was prescribed around these glyphs. This can cause a gap to be visible.
- Additionally, when we move things like baselines to land on a perfect integer pixel, we may be drawing a glyph lower in the bounding box than was prescribed originally.
### And how do we solve it?
- We identify all glyphs in the line and block drawing ranges.
- We find the bounding boxes of both the cell and the glyph.
- We compare the height of the glyph to the height of the cell to see if we need to scale. We prescribe a scale transform if the glyph wouldn't be tall enough to fit the box. (We leave it alone otherwise as some glyphs intentionally overscan the box and scaling them can cause banding effects.)
- We inspect the overhang/underhang above and below the boxes and translate transform them (slide them) so they cover the entire cell area.
- We repeat the previous two steps but in the horizontal direction.
## Validation Steps Performed
- See these commments:
- https://github.com/microsoft/terminal/issues/455#issuecomment-620248375
- https://github.com/microsoft/terminal/issues/455#issuecomment-621533916
- https://github.com/microsoft/terminal/issues/455#issuecomment-622585453
Also see the below one with more screenshots:
- https://github.com/microsoft/terminal/pull/5743#issuecomment-624940567
I never got to fixing these in the original #3789 PR, but I messed up that branch way too many times already that I figured I'd just do it in post.
* [x] Fixes the typo bot in `master`
* [x] I work here
This commit introduces a context menu for Tab and a new item,
"Color...", which will display a color picker.
A flyout menu, containing a custom flyout, is attached to each tab. The
flyout displays a palette of 16 preset colors and includes a color
picker. When the user selects or clears color, an event is fired, which
is intercepted by the tab to which the flyout belongs.
The changing of the color is achieved by putting the selected color in
the resource dictionary of the tab, using well-defined dictionary keys
(e.g. TabViewItemHeaderBackground). Afterwards the visual state of the
tab is toggled, so that the color change is visible immediately.
Custom-colored tabs will be desaturated (somewhat) by alpha blending
them with the tab bar background.
The flyout menu also contains a 'Close' flyout item.
## Validation Steps Performed
I've validated the behavior manually: start the program via the start
menu. Right click on the tab -> Choose a tab color.
The color flyout is going to be shown. Click a color swatch or click
'Select a custom color' to use the color picker. Use the 'Clear the
current color' to remove the custom color.
Closes#2994. References #3327.
Turns out we're still being a bit too aggressive when removing spaces.
If there are spaces at the end of the first run painted to a bottom
line, _and the bottom line was a different color than the previous_,
then we can't trim those spaces off the string. We still need to emit
those to make sure the terminal has colored spaces in it as well.
## References
* there's like 80 PRs in the last month for this function
## PR Checklist
* [x] Closes#5502
* [x] I work here
* [x] Tests added/passed
## Validation Steps
* [x] ran the tests
* [x] checked that vtpipeterm still worked
* [x] Checked that the bug was fixed in the Terminal
For our release builds, we're just going to integrate the UWPDesktop CRT
into our package and delete the package dependencies. It's very
difficult for users who do not have access to the store to get our
dependency packages, and we want to be robust and deployable everywhere.
Since these libraries can be redistributed, it's easiest if we simply
redistribute them.
Our package grows by ~550kb per architecture (compressed) because of
this. I've added validation that we don't have both the libs _and_ the
dependencies in the same package.
Fixes#3097.
## Validation
The script does it!
<!-- 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
In tonight's episode of "I wanna my CPU back", we'll see a quite familiar face whose name is Mr.AttrRow.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#2937
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [ ] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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
<!-- 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
I knew this is possible a long time ago. Just didn't got the chance to actually implement this. I understand that you guys are busy preparing the v1.0 release. So if this is a bad time, this can wait.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Manually tested. If all goes well, nothing will be broken.
Also known as "Kill HRGN II: Kills Regions Dead (#5485)"
Copying the description from @greg904 in #4778.
--- 8< ---
My understanding is that the XAML framework uses another way of getting
mouse input that doesn't work with `WM_SYSCOMMAND` with `SC_MOVE`. It
looks like it "steals" our mouse messages like `WM_LBUTTONDOWN`.
Before, we were cutting (with `HRGN`s) the drag bar part of the XAML
islands window in order to catch mouse messages and be able to implement
the drag bar that can move the window. However this "cut" doesn't only
apply to input (mouse messages) but also to the graphics so we had to
paint behind with the same color as the drag bar using GDI to hide the
fact that we were cutting the window.
The main issue with this is that we have to replicate exactly the
rendering on the XAML drag bar using GDI and this is bad because:
1. it's hard to keep track of the right color: if a dialog is open, it
will cover the whole window including the drag bar with a transparent
white layer and it's hard to keep track of those things.
2. we can't do acrylic with GDI
So I found another method, which is to instead put a "drag window"
exactly where the drag bar is, but on top of the XAML islands window (in
Z order). I've found that this lets us receive the `WM_LBUTTONDOWN`
messages.
--- >8 ---
Dustin's notes: I've based this on the implementation of the input sink
window in the UWP application frame host.
Tested manually in all configurations (debug, release) with snap,
drag, move, double-click and double-click on the resize handle. Tested
at 200% scale.
Closes#4744Closes#2100Closes#4778 (superseded.)
This change is necessary as the dep/ folder is not synced into the
Windows source tree.
I've also added a build rule producing a lib for {fmt}.
This will be required for our next OS ingestion.
* Cleaning up the whitelist a bit.
* The magic to exclude repeated characters worked 👍
* Every successful run on master now logs its suggested cleanup, e.g. for 5740e197c2 has https://github.com/microsoft/terminal/runs/596271627#step:4:37
* ⚠️ This check-spelling 0.0.15a+ tolerates Windows line endings in the `whitelist.txt` file (another project I touched had some `.gitconfig` magic which required supporting them).
This means that if someone edits the file w/ something that likes Windows line endings, the file will successfully convert (instead of it being ignored and check-spelling complaining about everything). Most likely anyone else who then edits the file will use something that will maintain the line endings.
This commit adds a `WT_PROFILE_ID` environment variable, which contains
the guid of the active profile.
It also teaches ConptyConnection to take an environment map on creation.
We had to do a little manual jiggery with the WSLENV environment
variable as passed by the creator.
* [x] CLA signed
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already.
Ran terminal, validated vars and translated paths under windows and WSL.
References #4566 (this PR originally introduced WT_SETTINGS/DEFAULTS)
Closes#3589
## Summary of the Pull Request
When WSL vim prints the initial empty buffer (the one that's just a bunch of '\~'s), it prints this by doing the following:
* Print '\~' followed by enough spaces to clear the line
* Use CUP (`^[[H`) to move the cursor to the start of the next line
* repeat until the buffer is full
When we'd get the line of "\~ "... in conhost, we'd mark that line as wrapped.
Logically, it doesn't really make any sense that when we follow that up by moving the cursor, the line is wrapped. However, this is just how conhost is right now.
This wasn't ever a problem in just conhost before, because we really didn't care if lines in the alt buffer were "wrapped" or not. Plus, when vim would get resized, it would just reprint it's own buffer anyways. Nor was this a problem in conpty before this year (2020). We've only just recently added logic to conpty to try and preserve wrapped lines.
Initially, I tried fixing this by breaking the line manually when the cursor was moved. This seemed to work great, except for the win32 vim.exe. Vim.exe doesn't emit a newline or a CUP to get to the next line. It just _goes for it_ and keeps printing. So there's _no way_ for us to know the line broke, because they're essentially just printing one long line, assuming we'll automatically move the cursor.
So instead, I'm making sure to emit the proper number of spaces at the end of a line when the line is wrapped. We won't do any funny business in that scenario and try to optimize for them, we'll _just print the spaces_.
## References
* #5181 - This change regressed this
* #4415 - Actually implemented wrapped lines in conpty
## PR Checklist
* [x] Closes#5291
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Wrote a unittest first and foremost
* Checked vtpipeterm to make sure vim still works
* checked Terminal to make sure vim still works
_This is literally just #1357, but moved to the `drafts/` folder_. Since
@dsafa doesn't have the time to finish this on their own, we'll take it
from here for 2.0 ☺️
## Summary of the Pull Request
Adds a spec describing jumplist integration and adding profiles to the jumplist. Includes details about previous investigations into adding the jumplist.
## PR Checklist
* [x] Specs #576
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [n/a] Tests added/passed
* [x] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #576
## Detailed Description of the Pull Request / Additional comments
Details in the spec.
## Validation Steps Performed
N/A
Co-authored-by: Brandon Chong <brndnchong@gmail.com>
This pull request makes sure we still get a usable (for troubleshooting purposes) version number in the about dialog and settings file when the user is running unpackaged.
This introduces a magic LCID constant (0x0409).B y default, Package ES emits
version resource information that says we're localized to ... language zero.
It also emits a language-coded version block for 0x0409 (en-US).
These two things cannot both be true. Collapse the wave function by hardcoding
0x0409.
Now that the Terminal is doing a better job of actually marking which
lines were and were not wrapped, we're not always copying lines as
"wrapped" when they should be. We're more correctly marking lines as not
wrapped, when previously we'd leave them marked wrapped.
The real problem is here in the `ScrollFrame` method - we'd manually
newline the cursor to make the terminal's viewport shift down to a new
line. If we had to scroll the viewport for a _wrapped_ line, this would
cause the Terminal to mark that line as broken, because conpty would
emit an extra `\n` that didn't actually exist.
This more correctly implements `ScrollFrame`. Now, well move where we
"thought" the cursor was, so when we get to the next `PaintBufferLine`,
if the cursor needs to newline for the next line, it'll newline, but if
we're in the middle of a wrapped line, we'll just keep printing the
wrapped line.
A couple follow up bugs were found to be caused by the same bad logic.
See #5039 and #5161 for more details on the investigations there.
## References
* #4741 RwR, which probably made this worse
* #5122, which I branched off of
* #1245, #357 - a pair of other conpty wrapped lines bugs
* #5228 - A followup issue for this PR
## PR Checklist
* [x] Closes#5113
* [x] Closes#5180 (by fixing DECRST 25)
* [x] Closes#5039
* [x] Closes#5161 (by ensuring we only `removeSpaces` on the actual
bottom line)
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Checked the cases from #1245, #357 to validate that they still work
* Added more and more tests for these scenarios, and then I added MORE
tests
* The entire team played with this in selfhost builds
This pull request introduces unexpanded variables (`%DEFAULT_PROFILE%`,
`%VERSION%` and `%PRODUCT%`) to the user settings template and code to
expand them.
While doing this, I ran into a couple things that needed to widen from
accepting strings to accepting string views. I also had to move
application name and version detection up to AppLogic and expose the
AppLogic singleton.
The dynamic profile generation logic had to be moved to before we inject
the templated variables, as the new default profile depends on the
generated dynamic profiles.
References #5189, #5217 (because it has a dependency on `VERSION` and
`PRODUCT`).
## PR Checklist
* [x] Closes#2721
* [x] CLA signed
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already
## Validation Steps Performed
Deleted my settings and watched them regenerate.
_Technically_, "wether" is a word but I'd be shocked if there's a scenario for
us to use it properly in this repo, so I'm pulling it from the dictionary.
Also, in #5200, we added "VTE", which is totally a valid acronym, to the codebase,
but not the whitelist. I'm not sure why the bot let me merge it anyways, but I'm
fixing it now.
## Summary of the Pull Request
As we've learned in #979, not all touchpads are created equal. Some of them have bad drivers that makes scrolling inactive windows not work. For whatever reason, these devices think the Terminal is all one giant inactive window, so we don't get the mouse wheel events through the XAML stack. We do however get the event as a `WM_MOUSEWHEEL` on those devices (a message we don't get on devices with normally functioning trackpads).
This PR attempts to take that `WM_MOUSEWHEEL` and manually dispatch it to the `TermControl`, so we can at least scroll the terminal content.
Unfortunately, this solution is not very general purpose. This only works to scroll controls that manually implement our own `IMouseWheelListener` interface. As we add more controls, we'll need to continue manually implementing this interface, until the underlying XAML Islands bug is fixed. **I don't love this**. I'd rather have a better solution, but it seems that we can't synthesize a more general-purpose `PointerWheeled` event that could get routed through the XAML tree as normal.
## References
* #2606 and microsoft/microsoft-ui-xaml#2101 - these bugs are also tracking a similar "inactive windows" / "scaled mouse events" issue in XAML
## PR Checklist
* [x] Closes#979
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I've also added a `til::point` conversion _to_ `winrt::Windows::Foundation::Point`, and some scaling operators for `point`
## Validation Steps Performed
* It works on my HP Spectre 2017 with a synaptics trackpad
- I also made sure to test that `tmux` works in panes on this laptop
* It works on my slaptop, and DOESN'T follow this hack codepath on this machine.
This pull request introduces the `til::math` namespace, which provides some casting functions to be used in support of `til::point` and `til::size`. When point/size want to ingest a floating-point structure, they _must_ be instructed on how to convert those floating-point values into integers.
This enables:
```
Windows::Foundation::Point wfPoint = /* ... */;
til::point tp{ til::math::rounding, wfPoint };
```
Future thoughts: should the TilMath types be stackable? Right now, you cannot get "checked + rounding" behavior (where it throws if it doesn't fit) so everything is saturating.
## PR Checklist
* [x] Closes a request by Michael
* [x] I've discussed this with core contributors already
Correct scrolling invalidation region for tmux in pty w/ bitmap
Add tracing for circling and scrolling operations. Fix improper
invalidation within AdjustCursorPosition routine in the subsection about
scrolling down at the bottom with a set of margins enabled.
## References
- Introduced with #5024
## Detailed Description of the Pull Request / Additional comments
- This occurs when there is a scroll region restriction applied and a
newline operation is performed to attempt to spin the contents of just
the scroll region. This is a frequent behavior of tmux.
- Right now, the Terminal doesn't support any sort of "scroll content"
operation, so what happens here generally speaking is that the PTY in
the ConHost will repaint everything when this happens.
- The PTY when doing `AdjustCursorPosition` with a scroll region
restriction would do the following things:
1. Slide literally everything in the direction it needed to go to take
advantage of rotating the circular buffer. (This would force a
repaint in PTY as the PTY always forces repaint when the buffer
circles.)
2. Copy the lines that weren't supposed to move back to where they were
supposed to go.
3. Backfill the "revealed" region that encompasses what was supposed to
be the newline.
- The invalidations for the three operations above were:
1. Invalidate the number of rows of the delta at the top of the buffer
(this part was wrong)
2. Invalidate the lines that got copied back into position (probably
unnecessary, but OK)
3. Invalidate the revealed/filled-with-spaces line (this is good).
- When we were using a simple single rectangle for invalidation, the
union of the top row of the buffer from 1 and the bottom row of the
buffer from 2 (and 3 was irrelevant as it was already unioned it)
resulted in repainting the entire buffer and all was good.
- When we switched to a bitmap, it dutifully only repainted the top line
and the bottom two lines as the middle ones weren't a consequence of
intersect.
- The logic was wrong. We shouldn't be invalidating rows-from-the-top
for the amount of the delta. The 1 part should be invalidating
everything BUT the lines that were invalidated in parts 2 and 3.
(Arguably part 2 shouldn't be happening at all, but I'm not optimizing
for that right now.)
- So this solves it by restoring an entire screen repaint for this sort
of slide data operation by giving the correct number of invalidated
lines to the bitmap.
## Validation Steps Performed
- Manual validation with the steps described in #5104
- Automatic test `ConptyRoundtripTests::ScrollWithMargins`.
Closes#5104
This repository tends to use `/`s in branch names.
Unfortunately, `branch: "*"` at present only matches a single
level, which means it would match a branch named `foo` but not `bar/foo`.
Given that I don't think this repository is actively using tags,
and given that the general cost for the spell checker isn't particularly
high, it's better to remove the filtering so that all branches get
checked.
Worst case, a branch that is also tagged and has spelling errors
will get two comments complaining about those spelling errors.
This is just some minor whitelisting/additions to dictionaries to catch
up between when the spell checker PR was written and when it was finally
merged.
This is basically taking the spelling output from
499f24a29e and putting it into files.
The choice of files is arbitrary. I'm adding a `math.txt` dictionary
because it's a reasonable example.
The goal here is to get master to a green check mark
## Validation
When I pushed this commit to my fork, the spell check action ran and
gave me a check mark: https://github.com/jsoref/terminal/runs/534783276
This commit introduces a github action to check our spelling and fixes
the following misspelled words so that we come up green.
It also renames TfEditSes to TfEditSession, because Ses is not a word.
currently, excerpt, fallthrough, identified, occurred, propagate,
provided, rendered, resetting, separate, succeeded, successfully,
terminal, transferred, adheres, breaks, combining, preceded,
architecture, populated, previous, setter, visible, window, within,
appxmanifest, hyphen, control, offset, powerpoint, suppress, parsing,
prioritized, aforementioned, check in, build, filling, indices, layout,
mapping, trying, scroll, terabyte, vetoes, viewport, whose
* Propose banner at top of issue templates
Getting tired of obvious low quality issues and I want to provide the warning that we may start closing things without further explanation as our volume is too high to deal with junk issues.
* Add bot rule information too.
* Close microsoft#752 by adding a pull request template
* Apply suggestions from code review by @miniksa and @bitcrazed
Co-Authored-By: Michael Niksa <miniksa@microsoft.com>
Co-Authored-By: Rich Turner <rich@bitcrazed.com>
* Fixed checkboxes
* Make placeholder uniform
[skip ci]
