## Summary of the Pull Request
Updates our `UiaTextRange` to no longer treat the end of the buffer as the "document end". Instead, we consider the "document end" to be the line beneath the cursor or last legible character (whichever is further down). In the event where the last legible character is on the last line of the buffer, we use the "end exclusive" position (left-most point on a line one past the end of the buffer).
When movement of any kind occurs, we clamp each endpoint to the document end. Since the document end is an actual spot in the buffer (most of the time), this should improve stability because we shouldn't be pointing out-of-bounds anymore.
The biggest benefit is that this significantly improves the performance of word navigation because screen readers no longer have to take into account the whitespace following the end of the prompt.
Word navigation tests were added to the `TestTableWriter` (see #10886). 24 of the 85 tests were failing, however, they don't seem to interact with the document end, so I've marked them as skip and will fix them in a follow-up. This PR is large enough as-is, so I'm hoping I can take time in the follow-up to clean some things on the side (aka `preventBoundary` and `allowBottomExclusive` being used interchangeably).
## References
#7000 - Epic
Closes#6986Closes#10925
## Validation Steps Performed
- [X] Tests pass
- [X] @codeofdusk has been personally testing this build (and others)
<!-- 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
Adds a global setting, `experimental.detectHyperlinks`, that controls whether we automatically detect links and make them clickable. Default is set to true.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#9981
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [x] Schema updated.
* [x] I work here
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
When `detectHyperlinks` is set to false, links do not underline on hover and are not clickable.
This PR adds support for the VT line rendition attributes, which allow
for double-width and double-height line renditions. These renditions are
enabled with the `DECDWL` (double-width line) and `DECDHL`
(double-height line) escape sequences. Both reset to the default
rendition with the `DECSWL` (single-width line) escape sequence. For now
this functionality is only supported by the GDI renderer in conhost.
There are a lot of changes, so this is just a general overview of the
main areas affected.
Previously it was safe to assume that the screen had a fixed width, at
least for a given point in time. But now we need to deal with the
possibility of different lines have different widths, so all the
functions that are constrained by the right border (text wrapping,
cursor movement operations, and sequences like `EL` and `ICH`) now need
to lookup the width of the active line in order to behave correctly.
Similarly it used to be safe to assume that buffer and screen
coordinates were the same thing, but that is no longer true. Lots of
places now need to translate back and forth between coordinate systems
dependent on the line rendition. This includes clipboard handling, the
conhost color selection and search, accessibility location tracking and
screen reading, IME editor positioning, "snapping" the viewport, and of
course all the rendering calculations.
For the rendering itself, I've had to introduce a new
`PrepareLineTransform` method that the render engines can use to setup
the necessary transform matrix for a given line rendition. This is also
now used to handle the horizontal viewport offset, since that could no
longer be achieved just by changing the target coordinates (on a double
width line, the viewport offset may be halfway through a character).
I've also had to change the renderer's existing `InvalidateCursor`
method to take a `SMALL_RECT` rather than a `COORD`, to allow for the
cursor being a variable width. Technically this was already a problem,
because the cursor could occupy two screen cells when over a
double-width character, but now it can be anything between one and four
screen cells (e.g. a double-width character on the double-width line).
In terms of architectural changes, there is now a new `lineRendition`
field in the `ROW` class that keeps track of the line rendition for each
row, and several new methods in the `ROW` and `TextBuffer` classes for
manipulating that state. This includes a few helper methods for handling
the various issues discussed above, e.g. position clamping and
translating between coordinate systems.
## Validation Steps Performed
I've manually confirmed all the double-width and double-height tests in
_Vttest_ are now working as expected, and the _VT100 Torture Test_ now
renders correctly (at least the line rendition aspects). I've also got
my own test scripts that check many of the line rendition boundary cases
and have confirmed that those are now passing.
I've manually tested as many areas of the conhost UI that I could think
of, that might be affected by line rendition, including things like
searching, selection, copying, and color highlighting. For
accessibility, I've confirmed that the _Magnifier_ and _Narrator_
correctly handle double-width lines. And I've also tested the Japanese
IME, which while not perfect, is at least useable.
Closes#7865
I was looking at conhost/OpenConsole and noticed it was being pretty
inefficient with allocations due to some usages of std::deque and
std::vector that didn't need to be done quite that way.
So this uses std::vector for the TextBuffer's storage of ROW objects,
which allows one allocation to contiguously reserve space for all the
ROWs - on Desktop this is 9001 ROW objects which means it saves 9000
allocations that the std::deque would have done. Plus it has the
benefit of increasing locality of the ROW objects since deque is going
to chase pointers more often with its data structure.
Then, within each ROW there are CharRow and ATTR_ROW objects that use
std::vector today. This changes them to use Boost's small_vector, which
is a variation of vector that allows for the so-called "small string
optimization." Since we know the typical size of these vectors, we can
pre-reserve the right number of elements directly in the
CharRow/ATTR_ROW instances, avoiding any heap allocations at all for
constructing these objects.
There are a ton of variations on this "small_vector" concept out there
in the world - this one in Boost, LLVM has one called SmallVector,
Electronic Arts' STL has a small_vector, Facebook's folly library has
one...there are a silly number of these out there. But Boost seems like
it's by far the easiest to consume in terms of integration into this
repo, the CI/CD pipeline, licensing, and stuff like that, so I went with
the boost version.
In terms of numbers, I measured the startup path of OpenConsole.exe on
my dev box for Release x64 configuration. My box is an i7-6700k @ 4
Ghz, with 32 GB RAM, not that I think machine config matters much here:
| | Allocation count | Allocated bytes | CPU usage (ms) |
| ------ | ------------------- | ------------------ | -------------- |
| Before | 29,461 | 4,984,640 | 103 |
| After | 2,459 (-91%) | 4,853,931 (-2.6%) | 96 (-7%) |
Along the way, I also fixed a dynamic initializer I happened to spot in
the registry code, and updated some docs.
## Validation Steps Performed
- Ran "runut", "runft" and "runuia" locally and confirmed results are
the same as the main branch
- Profiled the before/after numbers in the Visual Studio profiler, for
the numbers shown in the table
Co-authored-by: Austin Lamb <austinl@microsoft.com>
There are two issue with copy to clipboard when block is selected:
* We don't add new lines for lines that were wrapped
* We remove trailing whitespaces which is not intuitive in block selection.
Fixed the copy logic to always add newlines and not to remove
whitespaces when block is selected.
Even if shift is pressed!
## Detailed Description of the Pull Request / Additional comments
* Added optional parameter to `TextBuffer::GetText`
that allows to apply formatting (includeCRLF / trimming)
to lines that were wrapped
* Changed `Terminal::RetrieveSelectedTextFromBuffer`
to apply the following parameters when block is selected:
* includeCRLF = true
* trimTrailingWhitespaces = false
* apply the formatting above to all rows, including the ones
that were wrapped
## Validation Steps Performed
* Manual tests for both block and standard selection
* Copy with both right-click and command
* Added UT
Closes#6740
I added `enum class` to one thing and decided that that was quite enough
before disabling the `enum class` warning.
Looks like 16.8 made more map/vector operations noexcept, so we have to
re-annotate to remain compliant.
This pull request is the initial implementation of hyperlink auto
detection
Overall design:
- Upon startup, TerminalCore gives the TextBuffer some patterns it
should know about
- Whenever something in the viewport changes (i.e. text
output/scrolling), TerminalControl tells TerminalCore (through a
throttled function for performance) to retrieve the visible pattern
locations from the TextBuffer
- When the renderer encounters a region that is associated with a
pattern, it paints that region differently
References #5001Closes#574
It turns out that we missed part of the OSC 8 spec which indicated that
_hyperlinks with the same ID but different URIs are logically distinct._
> Character cells that have the same target URI and the same nonempty id
> are always underlined together on mouseover.
> The same id is only used for connecting character cells whose URIs is
> also the same. Character cells pointing to different URIs should never
> be underlined together when hovering over.
This pull request fixes that oversight by appending the (hashed) URI to
the generated ID.
When Terminal receives one of these links over ConPTY, it will hash the
URL a second time and therefore append a second hashed ID. This is taken
as an acceptable cost.
Fixes#7698
The `MovementAtExclusiveEnd` test was improperly authored for the
following reasons:
- it should have used `TEST_METHOD_PROPERTY` to cover all of the
TextUnits
- TextUnit::Document (arguably one of the most important) was ommitted
accidentally (`!= TextUnit_Document` was used instead of `<=`)
- The created range was not `EndExclusive`, but rather, the last cell in
the buffer (`EndInclusive`)
The first half of this PR fixes the test.
The second half of this PR expands the test and fixes any related issues
to make the test pass (i.e. #7771):
- `TEST_METHOD_PROPERTY` was added for it to be degenerate (start/end at
`EndExclusive`) or not (last cell of buffer)
- `utr->_start` is now also validated after moving backwards
NOTE: `utr->_start` was not validated when moving forwards because
moving forwards should always fail when at/past the last chell in the
buffer.
Closes#7771
This performs a minor refactor on `TextBuffer::MoveToNextWord` that
relies more heavily on `TextBuffer::GetWordEnd`. Now, the logic is
simplified and looks more like `MoveToPreviousWord`.
This refactor required me to move the `lastCharPos` optimization down to
`GetWordEnd`. So word expansion gets this optimization for free now.
### WPR Traces
The percentages below represent the weight that a function call had. The
test scenario included moving by word on the CMD welcome message until
the last word was reached. Inspect.exe was used to limit any additional
calls that are generally performed by a screen reader.
| function | current | branch |
| -- | -- | -- |
| `UIA:Move` | 34.55% | 29.52% |
There is an improvement of about 5% in a release build of ConHost.
NOTE: `UIA::Move` already calls `Expand` after a move operation is
performed. I'm using this data to represent a performance improvement
across both functions.
Contributes to #5243
<!-- 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
Conhost can now support OSC8 sequences (as specified [here](https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda)). Terminal also supports those sequences and additionally hyperlinks can be opened by Ctrl+LeftClicking on them.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#204
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] Closes#204
* [ ] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] 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
<!-- 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
Added support to:
- parse OSC8 sequences and extract URIs from them (conhost and terminal)
- add hyperlink uri data to textbuffer/screeninformation, associated with a hyperlink id (conhost and terminal)
- attach hyperlink ids to text to allow for uri extraction from the textbuffer/screeninformation (conhost and terminal)
- process ctrl+leftclick to open a hyperlink in the clicked region if present
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Open up a PowerShell tab and type
```PowerShell
${ESC}=[char]27
Write-Host "${ESC}]8;;https://github.com/microsoft/terminal${ESC}\This is a link!${ESC}]8;;${ESC}\"
```
Ctrl+LeftClick on the link correctly brings you to the terminal page on github
## Summary of the Pull Request
Text can have various attributes, such as "bold", "italic", "underlined", etc. The TextAttribute class embodies this. It has methods to set/query these attributes.
This change tweaks a few of the method names to make them match. I.e. for an imaginary text property "Foo", we should have methods along the lines of:
```
IsFoo
SetFoo(bool isFoo)
```
And variations should match: we should have "Foo" and "OverFoo", not "Fooey" and "OverFoo".
I chose to standardize on the adjective form, since that's what we are closest to already. The attributes I attacked here are:
SetItalic**s** --> SetItalic
SetUnderline --> SetUnderline**d**
SetOverline --> SetOverline**d**
("italic" is an adjective; "italics" is a plural noun, representing letters or words in an italic typeface)
And I also added methods for "DoublyUnderlined" for good measure.
I stopped short of renaming the GraphicsOptions enum values to match, too; but I'd be willing to do that in a follow-up change if people wanted it.
## Validation Steps Performed
It builds, and tests still pass.
This is a refactoring of the renderer color calculations to simplify the
implementation, and to make it easier to support additional
color-altering rendition attributes in the future (e.g. _faint_ and
_conceal_).
## References
* This is a followup to PRs #3817 and #6809, which introduced additional
complexity in the color calculations, and which suggested the need for
refactoring.
## Detailed Description of the Pull Request / Additional comments
When we added support for `DECSCNM`, that required the foreground and
background color lookup methods to be able to return the opposite of
what was requested when the reversed mode was set. That made those
methods unnecessarily complicated, and I thought we could simplify them
considerably just by combining the calculations into a single method
that derived both colors at the same time.
And since both conhost and Windows Terminal needed to perform the same
calculations, it also made sense to move that functionality into the
`TextAttribute` class, where it could easily be shared.
In general this way of doing things is a bit more efficient. However, it
does result in some unnecessary work when only one of the colors is
required, as is the case for the gridline painter. So to make that less
of an issue, I've reordered the gridline code a bit so it at least
avoids looking up the colors when no gridlines are needed.
## Validation Steps Performed
Because of the API changes, quite a lot of the unit tests had to be
updated. For example instead of verifying colors with two separate calls
to `LookupForegroundColor` and `LookupBackgroundColor`, that's now
achieved with a single `LookupAttributeColors` call, comparing against a
pair of values. The specifics of the tests haven't changed though, and
they're all still working as expected.
I've also manually confirmed that the various color sequences and
rendition attributes are rendering correctly with the new refactoring.
Looking up the size of the viewport from the underlying dimensions of
the structures seemed like a good idea at the time (so it would only be
in one place), but it turns out to be more of a perf cost than we
expected. Not necessarily on any one hot path, but if we sort by
functions in WPR, it was the top consumer on the Terminal side. This
instead saves the size as a member of the `TextBuffer` and serves that
out. It only changes when it is constructed or resized traditionally, so
it's easy to update/keep track of. It impacted conhost/conpty to a
lesser degree but was still noticeable.
## Validation Steps Performed
- Run `time cat big.txt` under WPR. Checked before and after perf
metrics.
## PR Checklist
* [x] Closes perf itch
* [x] I work here
* [x] Manual test
* [x] Documentation irrelevant.
* [x] Schema irrelevant.
* [x] Am core contributor.
Web apps apparently will paste the <title> as plaintext before the
actual HTML content from the clipboard. Since this seems to be
widespread behavior across web apps, this isn't just a bug in _some
app_, this is a bug on us. We shouldn't emit the title.
This PR removes the title tag from the generated HTML.
Closes#5347
## Summary of the Pull Request
- Added better wide glyph support for UIA. We used to move one _cell_ at a time, so wide glyphs would be read twice.
- Converted a few things to use til::point since I'm already here.
- fixed telemetry for UIA
## PR Checklist
* [x] Closes#1354
## Detailed Description of the Pull Request / Additional comments
The text buffer has a concept of word boundaries, so it makes sense to have a concept of glyph boundaries too.
_start and _end in UiaTextRange are now til::point
## Validation Steps Performed
Verified using Narrator
## Summary of the Pull Request
Currently, when the user resizes the Terminal, we'll snap the visible viewport back to the bottom of the buffer. This PR changes the visible viewport of the Terminal to instead remain in the same relative location it was before the resize.
## References
Made possible by our sponsors at #4741, and listeners like you.
## PR Checklist
* [x] Closes#3494
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
We already hated the `std::optional<short>&` thing I yeet'd into #4741 right at the end to replace a `short*`. So I was already going to change that to a `std::optional<std::reference_wrapper<short>>`, which is more idomatic. But then I was looking through the list of bugs and #3494 caught my eye. I realized it would be trivial to not only track the top of the `mutableViewport` during a resize, but we could use the same code path to track the _visible_ viewport's start as well.
So basically I'm re-using that bit of code in `Reflow` to calculate the visible viewport's position too.
## Validation Steps Performed
Gotta love just resizing things all day, errday
This PR adds support for "Resize with Reflow" to the Terminal. In
conhost, `ResizeWithReflow` is the function that's responsible for
reflowing wrapped lines of text as the buffer gets resized. Now that
#4415 has merged, we can also implement this in the Terminal. Now, when
the Terminal is resized, it will reflow the lines of it's buffer in the
same way that conhost does. This means, the terminal will no longer chop
off the ends of lines as the buffer is too small to represent them.
As a happy side effect of this PR, it also fixed#3490. This was a bug
that plagued me during the investigation into this functionality. The
original #3490 PR, #4354, tried to fix this bug with some heavy conpty
changes. Turns out, that only made things worse, and far more
complicated. When I really got to thinking about it, I realized "conhost
can handle this right, why can't the Terminal?". Turns out, by adding
resize with reflow, I was also able to fix this at the same time.
Conhost does a little bit of math after reflowing to attempt to keep the
viewport in the same relative place after a reflow. By re-using that
logic in the Terminal, I was able to fix#3490.
I also included that big ole test from #3490, because everyone likes
adding 60 test cases in a PR.
## References
* #4200 - this scenario
* #405/#4415 - conpty emits wrapped lines, which was needed for this PR
* #4403 - delayed EOL wrapping via conpty, which was also needed for
this
* #4354 - we don't speak of this PR anymore
## PR Checklist
* [x] Closes#1465
* [x] Closes#3490
* [x] Closes#4771
* [x] Tests added/passed
## EDIT: Changes to this PR on 5 March 2020
I learned more since my original version of this PR. I wrote that in
January, and despite my notes that say it was totally working, it
_really_ wasn't.
Part of the hard problem, as mentioned in #3490, is that the Terminal
might request a resize to (W, H-1), and while conpty is preparing that
frame, or before the terminal has received that frame, the Terminal
resizes to (W, H-2). Now, there aren't enough lines in the terminal
buffer to catch all the lines that conpty is about to emit. When that
happens, lines get duplicated in the buffer. From a UX perspective, this
certainly looks a lot worse than a couple lost lines. It looks like
utter chaos.
So I've introduced a new mode to conpty to try and counteract this
behavior. This behavior I'm calling "quirky resize". The **TL;DR** of
quirky resize mode is that conpty won't emit the entire buffer on a
resize, and will trust that the terminal is prepared to reflow it's
buffer on it's own.
This will enable the quirky resize behavior for applications that are
prepared for it. The "quirky resize" is "don't `InvalidateAll` when the
terminal resizes". This is added as a quirk as to not regress other
terminal applications that aren't prepared for this behavior
(gnome-terminal, conhost in particular). For those kinds of terminals,
when the buffer is resized, it's just going to lose lines. That's what
currently happens for them.
When the quirk is enabled, conpty won't repaint the entire buffer. This
gets around the "duplicated lines" issue that requesting multiple
resizes in a row can cause. However, for these terminals that are
unprepared, the conpty cursor might end up in the wrong position after a
quirky resize.
The case in point is maximizing the terminal. For maximizing
(height->50) from a buffer that's 30 lines tall, with the cursor on
y=30, this is what happens:
* With the quirk disabled, conpty reprints the entire buffer. This is
60 lines that get printed. This ends up blowing away about 20 lines
of scrollback history, as the terminal app would have tried to keep
the text pinned to the bottom of the window. The term. app moved the
viewport up 20 lines, and then the 50 lines of conpty output (30
lines of text, and 20 blank lines at the bottom) overwrote the lines
from the scrollback. This is bad, but not immediately obvious, and
is **what currently happens**.
* With the quirk enabled, conpty doesn't emit any lines, but the
actual content of the window is still only in the top 30 lines.
However, the terminal app has still moved 20 lines down from the
scrollback back into the viewport. So the terminal's cursor is at
y=50 now, but conpty's is at 30. This means that the terminal and
conpty are out of sync, and there's not a good way of re-syncing
these. It's very possible (trivial in `powershell`) that the new
output will jump up to y=30 override the existing output in the
terminal buffer.
The Windows Terminal is already prepared for this quirky behavior, so it
doesn't keep the output at the bottom of the window. It shifts it's
viewport down to match what conpty things the buffer looks like.
What happens when we have passthrough mode and WT is like "I would like
quirky resize"? I guess things will just work fine, cause there won't be
a buffer behind the passthrough app that the terminal cares about. Sure,
in the passthrough case the Terminal could _not_ quirky resize, but the
quirky resize won't be wrong.
## Summary of the Pull Request
`GetTextForClipboard` already exists in the TextBuffer. It makes sense to use that for UIA as well. This changes the behavior or `GetText()` such that it does not remove leading/trailing whitespace anymore. That is more of an expected behavior.
## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection anchors are an accurate representation of what is selected. We just need to move GetText to the TextBuffer. Then we can have those three issues just rely on code from the text buffer. This also means ConHost gets some of this stuff for free 😀
## Detailed Description of the Pull Request / Additional comments
- `TextBuffer::GetTextForClipboard()` --> `GetText()`
- `TextBuffer::GetText()` no longer requires GetForegroundColor/GetBackgroundColor. If either of these are not defined, we return a `TextAndColor` with only the `text` field populated.
- renamed a few parameters for copying text to the clipboard for clarity
- Updated `UiaTextRange::GetText()` to use `TextBuffer::GetText()`
## Validation Steps Performed
Manual tests for UIA using accessibility insights and Windows Terminal's copy action (w/ and w/out shift)
Added tests as well.
## Summary of the Pull Request
1) Improves the performance of word-recognition operations such as word
navigation in UIA and selection.
2) Fixes a bug where attempting to find the next word in UIA, when none
exists, would hang
3) TraceLogging code only runs when somebody is listening
## Detailed Description of the Pull Request / Additional comments
- The concept of a delimiter class got moved to the CharRow.
- The buffer iterator used to save a lot more information than we needed
- I missed updating a tracing function after making GetSelection return
one text range. That is fixed now.
## Validation Steps Performed
Performed Word Navigation under Narrator and NVDA.
NOTE: The release build should be used when testing to optimize
performance
Closes#4703
- When performing chunk selection, the expansion now occurs at the time
of the selection, not the rendering of the selection
- `GetSelectionRects()` was moved to the `TextBuffer` and is now shared
between ConHost and Windows Terminal
- Some of the selection variables were renamed for clarity
- Selection COORDs are now in the Text Buffer coordinate space
- Fixes an issue with Shift+Click after performing a Multi-Click
Selection
## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection
anchors are an accurate representation of what is selected. We just need
to move `GetText` to the `TextBuffer`. Then we can have those three
issues just rely on code from the text buffer. This also means ConHost
gets some of this stuff for free 😀
### TextBuffer
- `GetTextRects` is the abstracted form of `GetSelectionRects`
- `_ExpandTextRow` is still needed to handle wide glyphs properly
### Terminal
- Rename...
- `_boxSelection` --> `_blockSelection` for consistency with ConHost
- `_selectionAnchor` --> `_selectionStart` for consistency with UIA
- `_endSelectionPosition` --> `_selectionEnd` for consistency with
UIA
- Selection anchors are in Text Buffer coordinates now
- Really rely on `SetSelectionEnd` to accomplish appropriate chunk
selection and shift+click actions
## Validation Steps Performed
- Shift+Click
- Multi-Click --> Shift+Click
- Chunk Selection at...
- top of buffer
- bottom of buffer
- random region in scrollback
Closes#4465Closes#4547
## Summary of the Pull Request
This pull request is intended to achieve the following goals...
1) reduce duplicate code
2) remove static functions
3) improve readability
4) improve reliability
5) improve code-coverage for testing
6) establish functioning text buffer navigation in Narrator and NVDA
This also required a change to the wrapper class `XamlUiaTextRange` that has been causing issues with Narrator and NVDA.
See below for additional context.
## References
#3976 - I believe this might have been a result of improperly handling degenerate ranges. Fixed here.
#3895 - reduced the duplicate code. No need to separate into different files
#2160 - same as #3976 above
#1993 - I think just about everything is no longer static
## PR Checklist
* [x] Closes#3895, Closes#1993, Closes#3976, Closes#2160
* [x] CLA signed
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
### UiaTextRange
- converted endpoints into the COORD system in the TextBuffer coordinate space
- `start` is inclusive, `end` is exclusive. A degenerate range is when start == end.
- all functions are no longer static
- `MoveByUnit()` functions now rely on `MoveEndpointByUnit()` functions
- removed unnecessary typedefs like `Endpoint`, `ScreenInfoRow`, etc..
- relied more heavily on existing functionality from `TextBuffer` and `Viewport`
### XamlUiaTextRange
- `GetAttributeValue()` must return a special HRESULT that signifies that the requested attribute is not supported. This was the cause of a number of inconsistencies between Narrator and NVDA.
- `FindText()` should return `nullptr` if nothing was found. #4373 properly fixes this functionality now that Search is a shared module
### TextBuffer
- Word navigation functionality is entirely in `TextBuffer` for proper abstraction
- a total of 6 functions are now dedicated to word navigation to get a good understanding of the differences between a "word" in Accessibility and a "word" in selection
As an example, consider a buffer with this text in it:
" word other "
In selection, a "word" is defined as the range between two delimiters, so the words in the example include [" ", "word", " ", "other", " "].
In accessibility , a "word" includes the delimiters after a range of readable characters, so the words in the example include ["word ", "other "].
Additionally, accessibility word navigation must be able to detect if it is on the first or last word. This resulted in a slight variant of word navigation functions that return a boolean instead of a COORD.
Ideally, these functions can be consolidated, but that is too risky for a PR of this size as it can have an effect on selection.
### Viewport
- the concept of `EndExclusive` is added. This is used by UiaTextRange's `end` anchor as it is exclusive. To signify that the last character in the buffer is included in this buffer, `end` must be one past the end of the buffer. This is `EndExclusive`
- Since many functions check if the given `COORD` is in bounds, a flag must be set to allow `EndExclusive` as a valid `COORD` that is in bounds.
### Testing
- word navigation testing relies more heavily on TextBuffer tests
- additional testing was created for non-movement focused functions of UiaTextRange
- The results have been compared to Microsoft Word and some have been verified by UiAutomation/Narrator contacts as expected results.
## Validation Steps Performed
Tests pass
Narrator works
NVDA works
## Summary of the Pull Request
In pursuit of reflowing the terminal buffer on resize, move the reflow algorithm to the TextBuffer. This does _not_ yet add support for reflowing in the Windows Terminal.
## References
## PR Checklist
* [ ] There's not really an issue for this yet, I'm just breaking this work up into as many PRs as possible to help the inevitable bisect.
* [x] I work here
* [x] Ideally, all the existing tests will pass
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
In `SCREEN_INFORMATION::ResizeScreenBuffer`, the screenbuffer needs to create a new buffer, and copy the contents of the old buffer into the new one. I'm moving that "copy contents from the old buffer to the new one" step to it's own helper, as a static function on `TextBuffer`. That way, when the time comes to implement this for the Terminal, the hard part of the code will already be there.
## Validation Steps Performed
Ideally, all the tests will still pass.
## Summary of the Pull Request
- Enables auditing of some Terminal libraries (Connection, Core, Settings)
- Also audit WinConPTY.LIB since Connection depends on it
## PR Checklist
* [x] Rolls audit out to more things
* [x] I work here
* [x] Tests should still pass
* [x] Am core contributor
## Detailed Description of the Pull Request / Additional comments
This is turning on the auditing of these projects (as enabled by the heavier lifting in the other refactor) and then cleaning up the remaining warnings.
## Validation Steps Performed
- [x] Built it
- [x] Ran the tests
Enables support for word navigation when using an automation client (i.e.: Narrator, etc...). Specifically, adds this functionality to the UiaTextRange class. The only delimiter used is whitespace because that's how words are separated in English.
# General "Word Movement" Expectations
The resulting text range should include any word break characters that are present at the end of the word, but before the start of the next word. (Source)
If you already are on a word, getting the "next word" means you skip the word you are on, and highlight the upcoming word appropriately. (similar idea when moving backwards)
# Word Expansion
Since word selection is supposed to detect word delimiters already, I figured I'd reuse that code. I moved it from TerminalCore to the TextBuffer.
Then I built on top of it by adding an optional additional parameter that decides if you want to include...
- the delimiter run when moving forward
- the character run when moving backwards
It defaults to false so that we don't have to care when using it in selection. But we change it to true when using it in our UiaTextRange
# UiaTextRange
The code is based on character movement. This allows us to actually work with boundary conditions.
The main thing to remember here is that each text range is recorded as a MoveState. The text range is most easily defined when you think about the start Endpoint and the end Endpoint. An Endpoint is just a linear 1-dimensional indexing of the text buffer. Examples:
- Endpoint 0 --> (0,0)
- Endpoint 79 --> (79,0) (when the buffer width is 80)
- Endpoint 80 -->(0,1) (when the buffer width is 80)
- When moving forward, the strategy is to focus on moving the end Endpoint. That way, we properly get the indexing for the "next" word (this also fixes a wrapping issue). Then, we update the start Endpoint. (This is reversed for moving backwards).
- When moving a specific Endpoint, we just have a few extra if statements to properly adjust for moving start vs end.
# Hooking it up
All we really had to do is add an enum. This part was super easy :)
I originally wanted the delimiters to be able to be defined. I'm not so sure about that anymore. Either way, I hardcoded our delimiter into a variable so if we ever want to expand on it or make that customizable, we just modify that variable.
# Defining your own word delimiters
- Import a word delimiter into the constructor of the ScreenInfoUiaProvider (SIUP)
- This defines a word delimiter for all the UiaTextRanges (UTR) created by in this context
- import a word delimiter into the UTR directly
- this provides more control over what a "word" is
- this can be useful if you have an idea of what text a particular UTR will encounter and you want to customize the word navigation for it (i.e consider adding / or \\ for file paths)
The default param of " " is scattered throughout because this is the word delimiter used in the English language.
## Summary of the Pull Request
Operations that erase areas of the screen are typically meant to do so using the current color attributes, but with the rendition attributes reset (what we refer to as meta attributes). This also includes scroll operations that have to clear the area of the screen that has scrolled into view. The only exception is the _Erase Scrollback_ operation, which needs to reset the buffer with the default attributes. This PR updates all of these cases to apply the correct attributes when scrolling and erasing.
## PR Checklist
* [x] Closes#2553
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've not really discussed this with core contributors. I'm ready to accept this work might be rejected in favor of a different grand plan.
## Detailed Description of the Pull Request / Additional comments
My initial plan was to use a special case legacy attribute value to indicate the "standard erase attribute" which could safely be passed through the legacy APIs. But this wouldn't cover the cases that required default attributes to be used. And then with the changes in PR #2668 and #2987, it became clear that our requirements could be better achieved with a couple of new private APIs that wouldn't have to depend on legacy attribute hacks at all.
To that end, I've added the `PrivateFillRegion` and `PrivateScrollRegion` APIs to the `ConGetSet` interface. These are just thin wrappers around the existing `SCREEN_INFORMATION::Write` method and the `ScrollRegion` function respectively, but with a simple boolean parameter to choose between filling with default attributes or the standard erase attributes (i.e the current colors but with meta attributes reset).
With those new APIs in place, I could then update most scroll operations to use `PrivateScrollRegion`, and most erase operations to use `PrivateFillRegion`.
The functions affected by scrolling included:
* `DoSrvPrivateReverseLineFeed` (the RI command)
* `DoSrvPrivateModifyLinesImpl` (the IL and DL commands)
* `AdaptDispatch::_InsertDeleteHelper` (the ICH and DCH commands)
* `AdaptDispatch::_ScrollMovement` (the SU and SD commands)
The functions affected by erasing included:
* `AdaptDispatch::_EraseSingleLineHelper` (the EL command, and most ED variants)
* `AdaptDispatch::EraseCharacters` (the ECH command)
While updating these erase methods, I noticed that both of them also required boundary fixes similar to those in PR #2505 (i.e. the horizontal extent of the erase operation should apply to the full width of the buffer, and not just the current viewport width), so I've addressed that at the same time.
In addition to the changes above, there were also a few special cases, the first being the line feed handling, which required updating in a number of places to use the correct erase attributes:
* `SCREEN_INFORMATION::InitializeCursorRowAttributes` - this is used to initialise the rows that pan into view when the viewport is moved down the buffer.
* `TextBuffer::IncrementCircularBuffer` - this occurs when we scroll passed the very end of the buffer, and a recycled row now needs to be reinitialised.
* `AdjustCursorPosition` - when within margin boundaries, this relies on a couple of direct calls to `ScrollRegion` which needed to be passed the correct fill attributes.
The second special case was the full screen erase sequence (`ESC 2 J`), which is handled separately from the other ED sequences. This required updating the `SCREEN_INFORMATION::VtEraseAll` method to use the standard erase attributes, and also required changes to the horizontal extent of the filled area, since it should have been clearing the full buffer width (the same issue as the other erase operations mentioned above).
Finally, there was the `AdaptDispatch::_EraseScrollback` method, which uses both scroll and fill operations, which could now be handled by the new `PrivateScrollRegion` and `PrivateFillRegion` APIs. But in this case we needed to fill with the default attributes rather than the standard erase attributes. And again this implementation needed some changes to make sure the full width of the active area was retained after the erase, similar to the horizontal boundary issues with the other erase operations.
Once all these changes were made, there were a few areas of the code that could then be simplified quite a bit. The `FillConsoleOutputCharacterW`, `FillConsoleOutputAttribute`, and `ScrollConsoleScreenBufferW` were no longer needed in the `ConGetSet` interface, so all of that code could now be removed. The `_EraseSingleLineDistanceHelper` and `_EraseAreaHelper` methods in the `AdaptDispatch` class were also no longer required and could be removed.
Then there were the hacks to handle legacy default colors in the `FillConsoleOutputAttributeImpl` and `ScrollConsoleScreenBufferWImpl` implementations. Since those hacks were only needed for VT operations, and the VT code no longer calls those methods, there was no longer a need to retain that behaviour (in fact there are probably some edge cases where that behaviour might have been considered a bug when reached via the public console APIs).
## Validation Steps Performed
For most of the scrolling operations there were already existing tests in place, and those could easily be extended to check that the meta attributes were correctly reset when filling the revealed lines of the scrolling region.
In the screen buffer tests, I made updates of that sort to the `ScrollOperations` method (handling SU, SD, IL, DL, and RI), the `InsertChars` and `DeleteChars` methods (ICH and DCH), and the `VtNewlinePastViewport` method (LF). I also added a new `VtNewlinePastEndOfBuffer` test to check the case where the line feed causes the viewport to pan past the end of the buffer.
The erase operations, however, were being covered by adapter tests, and those aren't really suited for this kind of functionality (the same sort of issue came up in PR #2505). As a result I've had to reimplement those tests as screen buffer tests.
Most of the erase operations are covered by the `EraseTests` method, except the for the scrollback erase which has a dedicated `EraseScrollbackTests` method. I've also had to replace the `HardReset` adapter test, but that was already mostly covered by the `HardResetBuffer` screen buffer test, which I've now extended slightly (it could do with some more checks, but I think that can wait for a future PR when we're fixing other RIS issues).
## Summary of the Pull Request
RTF data is now copied to the clipboard. Tested by copy pasting text from terminal to WordPad.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#2487
* [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: #2487
<!-- 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
Mostly similar to PR #1224. Added a new static method `GenRTF` in `TextBuffer` that is responsible
for generating the RTF representation of a given text. The generated RTF is added to the `DataPackage` that is ultimately passed to the clipboard.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Validated by copy pasting text from the terminal to WordPad. Validated with different colors to make sure that is working. (MS Word seems to prefer HTML data from the clipboard instead of RTF.)
<hr>
* Copy RTF data to the clipboard
* Added comment explaining various parts of the header
* Fixed static code analysis issues and added noexcept to GenRTF()
* Removed noexcept
We now truncate the font name as it goes out to GDI APIs, in console API
servicing, and in the propsheet.
I attempted to defer truncating the font to as far up the stack as
possible, so as to make FontInfo usable for the broadest set of cases.
There were a couple questions that came up: I know that `Settings` gets
memset (memsat?) by the registry deserializer, and perhaps that's
another place for us to tackle. Right now, this pull request enables
fonts whose names are >= 32 characters _in Windows Terminal only_, but
the underpinnings are there for conhost as well. We'd need to explicitly
break at the API, or perhaps return a failure or log something to
telemetry.
* Should we log truncation at the API boundary to telemetry?
-> Later; followup filed (#3123)
* Should we fix Settings here, or later?
-> Later; followup filed (#3123)
* `TrueTypeFontList` is built out of things in winconp, the private
console header. Concern about interop structures.
-> Not used for interop, followup filed to clean it up (#3123)
* Is `unsigned int` right for codepage? For width?
-> Yes: codepage became UINT (from WORD) when we moved from Win16 to
Win32
This commit also includes a workaround for #3170. Growing
CONSOLE_INFORMATION made us lose the struct layout lottery during
release builds, and this was an expedient fix.
Closes#602.
Related to #3123.
This adds the WPF control to our project, courtesy of the Visual Studio team.
It re-hosts the Terminal Control components inside a reusable WPF adapter so it can be composed onto C# type surfaces like Visual Studio requires.
EraseInLine calls `FillConsoleOutputCharacterW()`. In filling the row with
chars, we were setting the wrap flag. We need to specifically not do this on
ANY _FILL_ operation. Now a fill operation UNSETS the wrap flag if we fill to
the end of the line.
Originally, we had a boolean `setWrap` that would mean...
- **true**: if writing to the end of the row, SET the wrap value to true
- **false**: if writing to the end of the row, DON'T CHANGE the wrap value
Now we're making this bool a std::optional to allow for a ternary state. This
allows for us to handle the following cases completely. Refer to the table
below:
,- current wrap value
| ,- are we filling the last cell in the row?
| | ,- new wrap value
| | | ,- comments
|-- |-- |-- |
| 0 | 0 | 0 |
| 0 | 1 | 0 |
| 0 | 1 | 1 | THIS CASE WAS HANDLED CORRECTLY
| 1 | 0 | 0 | THIS CASE WAS UNHANDLED
| 1 | 0 | 1 |
| 1 | 1 | 1 |
To handle that special case (1-0-0), we need to UNSET the wrap. So now, we have
~setWrap~ `wrap` mean the following:
- **true**: if writing to the end of the row, SET the wrap value to TRUE
- **false**: if writing to the end of the row, SET the wrap value to FALSE
- **nullopt**: leave the wrap value as it is
Closes#1126
* Move Clipboard::GenHTML to TextBuffer (add params)
Refactor RetrieveSelectedTextFromBuffer
Modify CopyToClipboardEventArgs to include HTML data
* minor code format fix
* PR Changes
NOTE: refactoring text buffer code is a separate task. New issue to be created.
* Refactor TextBuffer::GenHTML (#2038)
Fixes#1846.
* nit change
* x86 build fix
* nit changes
