-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Fix ShowWindow(GetConsoleWindow())
#13118
Changes from all commits
3ab3bef
b89ed13
9525beb
bbae235
ad5295a
50cd051
f04b892
0dc6e0d
478cb9f
5f6ff34
33b8a20
ed5222c
f1dda12
b3e2987
60f391b
ee18932
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1196,8 +1196,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
|
||
void ControlCore::_terminalShowWindowChanged(bool showOrHide) | ||
{ | ||
auto showWindow = winrt::make_self<implementation::ShowWindowArgs>(showOrHide); | ||
_ShowWindowChangedHandlers(*this, *showWindow); | ||
if (_initializedTerminal) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Vague horror that we might be getting this callback before we have finished initialization...! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would actually bet that code is vestigial at this point. I'd guess leftover from me experimenting with the debouncing. That being said, I thought that 0dc6e0d wasn't needed, and it totally was. |
||
{ | ||
auto showWindow = winrt::make_self<implementation::ShowWindowArgs>(showOrHide); | ||
_ShowWindowChangedHandlers(*this, *showWindow); | ||
} | ||
} | ||
|
||
bool ControlCore::HasSelection() const | ||
|
@@ -1703,10 +1706,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
// - <none> | ||
void ControlCore::WindowVisibilityChanged(const bool showOrHide) | ||
{ | ||
// show is true, hide is false | ||
if (auto conpty{ _connection.try_as<TerminalConnection::ConptyConnection>() }) | ||
if (_initializedTerminal) | ||
{ | ||
conpty.ShowHide(showOrHide); | ||
// show is true, hide is false | ||
if (auto conpty{ _connection.try_as<TerminalConnection::ConptyConnection>() }) | ||
{ | ||
conpty.ShowHide(showOrHide); | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -300,18 +300,34 @@ bool VtIo::IsUsingVt() const | |
|
||
if (_pPtySignalInputThread) | ||
{ | ||
// IMPORTANT! Start the pseudo window on this thread. This thread has a | ||
// message pump. If you DON'T, then a DPI change in the owning hwnd will | ||
// cause us to get a dpi change as well, which we'll never deque and | ||
// handle, effectively HANGING THE OWNER HWND. | ||
// Let the signal thread know that the console is connected. | ||
// | ||
// Let the signal thread know that the console is connected | ||
// By this point, the pseudo window should have already been created, by | ||
// ConsoleInputThreadProcWin32. That thread has a message pump, which is | ||
// needed to ensure that DPI change messages to the owning terminal | ||
// window don't end up hanging because the pty didn't also process it. | ||
_pPtySignalInputThread->ConnectConsole(); | ||
} | ||
|
||
return S_OK; | ||
} | ||
|
||
// Method Description: | ||
// - Create our pseudo window. This is exclusively called by | ||
// ConsoleInputThreadProcWin32 on the console input thread. | ||
// * It needs to be called on that thread, before any other calls to | ||
// LocatePseudoWindow, to make sure that the input thread is the HWND's | ||
Comment on lines
+318
to
+319
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any way to guarantee that this happens before LPW? |
||
// message thread. | ||
// * It needs to be plumbed through the signal thread, because the signal | ||
// thread knows if someone should be marked as the window's owner. It's | ||
// VERY IMPORTANT that any initial owners are set up when the window is | ||
// first created. | ||
// - Refer to GH#13066 for details. | ||
void VtIo::CreatePseudoWindow() | ||
{ | ||
_pPtySignalInputThread->CreatePseudoWindow(); | ||
} | ||
|
||
// Method Description: | ||
// - Create and start the signal thread. The signal thread can be created | ||
// independent of the i/o threads, and doesn't require a client first | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to admit, I am VERY worried that I read two comments that say "conpty assumes it's hidden" and then this banger that makes ConptyConnection assume it is visible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it makes sense, it was just surprising