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

Respect the startup info state initially passed to wt via ShellExecute #13838

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Aug 24, 2022

Original description, pre-process model v3:

This is just the SHOWDEFAULT bit from #12979. This seems to also work now, but I'm PR'ing it separately so it can be a separate revert from #13811, if it is problematic.

More accurately:
This PR enables terminal windows to use the wShowCmd from the STARTUPINFO passed to windowsterminal.exe to set the initial visibility of the window. We can't just use SW_SHOWDEFAULT, because all the windows are running in the initial process! After the first window, the subsequent ones would ignore any params passed to their originating windowsterminal.exe processes. To mitigate, we pass that wShowCmd info from the source process, to the actual running terminal process. That accounts for most of the delta here.

Closes #9053

This doesn't do the same for defterm-initiated connections. This is because we don't need to! Defterm very explicitly rejects handoff for minimized console apps. This is probably for the best! I put an attempt in 66f8b25 before I forgot that it was filtered long before the Terminal. NOT doing this for /min saves us all sorts of "what happens if start /min cmd tries to glom?" or "what if someone does start /min cmd && start /max cmd and they glom together?".


Also closes #15193, which was introduced as a part of this.

zadjii-msft and others added 26 commits July 18, 2022 14:59
  This hooks up the defterm interface to pass more info along in the startup. It's notably not the actual link title, but it does work from a plumbing standpoint

  This is for #9458
@ghost ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Aug 24, 2022
@zadjii-msft
Copy link
Member Author

zadjii-msft commented Apr 6, 2023

This almost works now with the changes from #13811 merged in. Process model v3 changed things a bit though - it works great for the first invocation. Subsequent ones fail, because SHOWDEFAULT isn't passed from the originating terminal process to the subsequent one. We may actually need a --minimized flag or something, to pass to the monarch...

@zadjii-msft zadjii-msft marked this pull request as ready for review April 6, 2023 14:43
@zadjii-msft
Copy link
Member Author

We may need a --minimized flag or something, to pass to the monarch...

oh it was so much easier than that

@github-actions

This comment has been minimized.

Base automatically changed from dev/migrie/b/9053-attempt-2 to main April 6, 2023 18:03
DHowett added a commit that referenced this pull request Apr 11, 2023
DHowett added a commit that referenced this pull request Apr 14, 2023
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 18, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the dev/migrie/b/9053-part-3-the-actual-doing-of-the-thing branch April 18, 2023 16:23
@microsoft-github-policy-service microsoft-github-policy-service bot added Area-ShellExtension For issues related to the explorer right-click context menu Priority-1 A description (P1) Severity-Blocking We won't ship a release like this! No-siree. labels Apr 18, 2023
zadjii-msft added a commit that referenced this pull request Apr 20, 2023
  We had a report in a mail thread that someone's Terminal windows were getting created hidden, and never showing themselves.

  As a theory, I'm guessing that dwFlags didn't say that we should actually use `wShowWindow`. So, to be more correct, let's actually obey that.

  I'm gonna send this package to them to see if it fixes them.

  Related to #14957.

  Likely regressed in #13838.
zadjii-msft added a commit that referenced this pull request Apr 25, 2023
We had a report in a mail thread that someone's Terminal windows were
getting created hidden, and never showing themselves.

As a theory, I'm guessing that dwFlags didn't say that we should
actually use `wShowWindow`. So, to be more correct, let's actually obey
that.

I'm gonna send this package to them to see if it fixes them.

Related to #14957.

Likely regressed in #13838.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-ShellExtension For issues related to the explorer right-click context menu Area-User Interface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Open Terminal here" doesn't show a window on 1.18 START /MIN WT?
3 participants