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

Commits on Sep 19, 2023

  1. Allow inheriting env vars from wt again and other env var changes t…

    …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.
    zadjii-msft authored Sep 19, 2023
    Configuration menu
    Copy the full SHA
    59248de View commit details
    Browse the repository at this point in the history
  2. Option to default to show icon in tab, hide tabicon or make icon in t…

    …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)
    bundgaard authored Sep 19, 2023
    Configuration menu
    Copy the full SHA
    394b942 View commit details
    Browse the repository at this point in the history

Commits on Sep 20, 2023

  1. Don't crash when the checking for updates without a network (#16002)

    You can't catch an A/V. 
    
    Closes #15459
    zadjii-msft authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    523edd7 View commit details
    Browse the repository at this point in the history
  2. Improve performance of scrollbar marks (#16006)

    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 ✅
    lhecker authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    b5333f6 View commit details
    Browse the repository at this point in the history
  3. Merge PublicTerminalCore into TermControl (#15992)

    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.
    DHowett authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    059f770 View commit details
    Browse the repository at this point in the history
  4. Fix enter to restart the first pane in a split (#16001)

    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.
    zadjii-msft authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    9b986a1 View commit details
    Browse the repository at this point in the history
  5. Don't explode if we try to parse an empty keys (#16003)

    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
    zadjii-msft authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    d272fc4 View commit details
    Browse the repository at this point in the history
  6. Use the control that requested the context menu, instead of the activ…

    …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>
    zadjii-msft and mpela81 authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    75f3d4f View commit details
    Browse the repository at this point in the history
  7. Don't update the settings, unless the theme actually changed (#16004)

    `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
    zadjii-msft authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    c1bdcf8 View commit details
    Browse the repository at this point in the history

Commits on Sep 21, 2023

  1. Avoid moving the selection while typing a search query (#15998)

    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 ✅
    lhecker authored Sep 21, 2023
    Configuration menu
    Copy the full SHA
    fc4a37e View commit details
    Browse the repository at this point in the history
  2. Replace WinRT clipboard API with Win32 for pasting (#15360)

    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>
    lhecker and DHowett authored Sep 21, 2023
    Configuration menu
    Copy the full SHA
    d38bb90 View commit details
    Browse the repository at this point in the history
  3. ConPTY: Fix missing flush on console mode changes (#15991)

    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 ✅
    lhecker authored Sep 21, 2023
    Configuration menu
    Copy the full SHA
    41f7ed7 View commit details
    Browse the repository at this point in the history