Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pull] main from microsoft:main #165

Merged
merged 12 commits into from
Sep 22, 2023
Merged

[pull] main from microsoft:main #165

merged 12 commits into from
Sep 22, 2023

Conversation

pull[bot]
Copy link

@pull pull bot commented Sep 20, 2023

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

zadjii-msft and others added 2 commits September 19, 2023 15:03
…oo (#15897)

This PR is a few things:

* part the first: Convert the `compatibility.reloadEnvironmentVariables`
setting to a per-profile one.
* The settings should migrate it from the user's old global place to the
new one.
  * We also added it to "Profile>Advanced" while I was here.
* Adds a new pair of commandline flags to `new-tab` and `split-pane`:
`--inheritEnvironment` / `--reloadEnvironment`
* On `wt` launch, bundle the entire environment that `wt` was spawned
with, and put it into the `Remoting.CommandlineArgs`, and give them to
the monarch (and ultimately, down to `TerminalPage` with the
`AppCommandlineArgs`). DO THIS ALWAYS.
* As a part of this, we’ll default to _reloading_ if there’s no explicit
commandline set, and _inheriting_ if there is.
* For example, `wt -- cmd` would inherit, and `wt -p “Command Prompt”`
would reload.[^1]
* This is a little wacky, but we’re trying to separate out the
intentions here:
* `wt -- cmd` feels like “I want to run cmd.exe (in a terminal tab)”.
That feels like the user would _like_ environment variables from the
calling process. They’re doing something more manual, so they get more
refined control over it.
* `wt` (or `wt -p “Command Prompt”`) is more like, “I want to run the
Terminal (or, my Command Prompt profile) using whatever the Terminal
would normally do”. So that feels more like a situation where it should
just reload by default. (Of course, this will respect their settings
here)

## References and Relevant Issues

#15496 (comment)
has more notes.

## Detailed Description of the Pull Request / Additional comments
This is so VERY much plumbing. I'll try to leave comments in the
interesting parts.

## PR Checklist
- [x] This is not _all_ of #15496. We're also going to do a `-E foo=bar`
arg on top of this.
- [x] Tests added/passed
- [x] Schema updated

[^1]: In both these cases, plus the `environment` setting, of course.
…ab monochrome. (#15948)

Adding enum iconstyle for hiding the icon in the tab #8157 

## Summary of the Pull Request
Please confirm if I am on the right track.
## References and Relevant Issues

## Detailed Description of the Pull Request / Additional comments

## Validation Steps Performed

## PR Checklist
- [ ] Closes #8157
- [x] 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 (if necessary)
@pull pull bot added the ⤵️ pull label Sep 20, 2023
zadjii-msft and others added 10 commits September 20, 2023 06:35
This replaces the use of a `<Canvas>` with an `<Image>` for drawing
scrollbar marks. Otherwise, WinUI struggles with the up to ~9000 UI
elements as they get dirtied every time the scrollbar moves.
(FWIW 9000 is not a lot and it should not struggle with that.)

The `<Image>` element has the benefit that we can get hold of a CPU-side
bitmap which we can manually draw our marks into and then swap them into
the UI tree. It draws the same 9000 elements, but now WinUI doesn't
struggle anymore because only 1 element gets invalidated every time.

Closes #15955

## Validation Steps Performed
* Fill the buffer with "e"
* Searching for "e" fills the entire thumb range with white ✅
* ...doesn't lag when scrolling around ✅
* ...updates quickly when adding newlines at the end ✅
* Marks sort of align with their scroll position ✅
This pull request moves HwndTerminal into Microsoft.Terminal.Control.Lib
and removes PublicTerminalCore completely.

Microsoft.Terminal.Control.dll now exports the C API from HwndTerminal.

This adds ~100kb to Microsoft.Terminal.Control.dll and ~1400kb to the
WPF package (per architecture) but with the coming interactivity
platform merge it's going to benefit us big time.
I noticed this last week, but forgot to file. If you have a pair of
splits, and `exit -1` the first, you can't use `enter` to restart it.

This PR fixes that. Basically, `TerminalPage` registers it's
`_restartPaneConnection` handler when it makes a new `Pane` object. It
registers the callback straight to the `Pane`. However, when a `Pane`
gets split, it makes a _new_ `Pane` object, and moves the original
content into the new pane. `TerminalPage` however, would never hook up
its own callback to that newly created pane.

This fixes that.
Saving the SUI with an empty "keys" will persist `"keys": ""` to the
JSON.

The keychord parser tries to parse that.
`KeyChordSerialization.cpp@_fromString` returns a KeyChord with both
vkey and scancode set to 0, and the ctor asserts and explodes.

We shouldn't do that. 

Closes #13221
…e one (#15999)

As mentioned in #15760

> > When you right-click on a non-active pane, it becomes active, but the context menu may be displayed before this happens, thus showing the Restart Connection item based the wrong pane's status.
> 
> As far as I can see, when a pane is (right)clicked:
> 
> 1. If unfocused, `Focus` is called. This goes through the `GotFocus` handler which eventually calls `tab->_UpdateActivePane(sender);`
> 2. `PointerPressed` is raised which eventually shows the context menu
> 
> The first point is done asynchronously, so may update the active pane too late when the menu is already displayed (despite both end up in the UI thread).

To fix this: we plumb the control that the context menu was opened for all the way through to where the event is actually handled (in `_PopulateContextMenu`)

* [x] Tested manually

Co-authored-by: Marco Pelagatti <1140981+mpela81@users.noreply.github.com>
`ImmersiveColorSet` gets sent more often than just on a theme change. It notably gets sent when the PC is locked, or the UAC prompt opens.

## Validation Steps Performed

Tested manually by setting the font to `garbo`and:
* locking, then logging back in. No dialog ✅ 
* UAC via run dialog + `regedit`. No dialog ✅ 
* Actually changing the OS theme. Dialog ✅  

Closes #15732
This commit fixes 2 issues:
* `ControlCore::ScrollMarks()` would call `ResetIfStale`
  again while the search prompt hasn't changed.
  This has been fixed by using `_cachedSearchResultRows` as
  the indicator for whether it needs to be recreated or not.
* While typing a search query, the selection would move among the
  results with each typed character, because `MovePastCurrentSelection`
  would do what its name indicates. It has been renamed and rewritten
  to be `MoveToCurrentSelection`. To avoid breaking UIA, the previous
  `MovePastPoint` implementation was kept.

Since the new `MoveToCurrentSelection` function would not move past the
current selection anymore, changing the direction would not move past
the current result either. To fix this, we now don't invalidate the
search cache when changing the direction.

Closes #15954

## Validation Steps Performed
* Run ``"helloworld`n"*20`` in pwsh
* Search for "helloworld"
* While typing the characters the selection doesn't move ✅
* ...nor when searching downwards ✅
* ...nor when erasing parts of it ✅
* ...and it behaves identical in conhost ✅
The Win32 API is significantly faster than the WinRT one, in the order
of around 300-1000x depending on the CPU and CPU load.

This might slightly improve the situation around #15315, but I suspect
that it requires many more fixes. For instance, we don't really have a
single text input "queue" into which we write. Multiple routines that
`resume_background` just to `WriteFile` into the input pipe are thus
racing against each other, contributing to the laggy feeling.
I also fear that the modern Windows text stack might be inherently
RPC based too, producing worse lag with rising CPU load.

This might fix #14323

## Validation Steps Performed
* Paste text from Edge ✅
* Paste text from Notepad ✅
* Right click the address bar in Explorer, choose "Copy address",
  paste text into WT ✅

---------

Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
Previously, all unknown escape sequences would lead to an immediate call
to `VtEngine::_Flush()`. This lead to problems with nushell which uses
FTCS marks that were unknown to us. Combined with the linewise redrawing
that nushell does, Terminal would get the prompt in two separate frames,
causing a slight flickering.

#14677 fixed this by suppressing the `_Flush()` call when unknown
sequences are encountered. Unfortunately, this triggered a bug due
to our somewhat "inconsistent" architecture in conhost:
`XtermEngine::WriteTerminalW` isn't just used to flush unknown sequences
but also used directly by `InputBuffer::PassThroughWin32MouseRequest`
to write its mouse sequence directly to the ConPTY host.
`VtEngine` already contains a number of specialized member functions
like `RequestWin32Input()` to ensure that `_Flush()` is called
immediately and another member could've been added to solve this issue.
This commit now adds `RequestMouseMode` in the same vein.

But I believe we can make the system more robust in general by using
eager flushing by default (= safe), similar to how a `write()` on a
TCP socket flushes by default, and instead only selectively pause and
unpause flushing with a system similar to `TCP_CORK`.

This seems to work fairly well, as it solves:
* The original nushell bug
* The new bug
* Improves overall throughput by ~33% (due to less flushing)

In particular the last point is noteworthy, as this commit removes
the last performance bottleneck in ConPTY that isn't `VtEngine`.
Around ~95% of all CPU and wall time is spent in there now and any
improvements to `VtEngine` should yield immediately results.

Closes #15711

## Validation Steps Performed
* Clone/Run https://github.com/chrisant996/repro_enable_mouse_input
* Hold Ctrl+Alt and circle with the mouse over the viewport
* Repro.exe prints the current cursor coordinates ✅
* Run nushell
* No flickering when typing in the prompt ✅
@pull pull bot merged commit 41f7ed7 into TheRakeshPurohit:main Sep 22, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants