Skip to content

Commit

Permalink
I'm committing everything just in case, but I'm gonna revert and just…
Browse files Browse the repository at this point in the history
… do the part that fixed it
  • Loading branch information
zadjii-msft committed Feb 15, 2023
1 parent bc80943 commit 667b658
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 63 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ namespace winrt::TerminalApp::implementation
}
}

tabViewItem.PointerReleased({ this, &TerminalPage::_OnTabClick });
tabViewItem.PointerReleased({ get_weak(), &TerminalPage::_OnTabClick });

// When the tab requests close, try to close it (prompt for approval, if required)
newTabImpl->CloseRequested([weakTab, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
Expand Down
77 changes: 48 additions & 29 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ using namespace ::Microsoft::Console;
using namespace ::Microsoft::Terminal::Core;
using namespace std::chrono_literals;

#define HOOKUP_ACTION(action) _actionDispatch->action({ this, &TerminalPage::_Handle##action });
#define HOOKUP_ACTION(action) _actionDispatch->action({ get_weak(), &TerminalPage::_Handle##action });

namespace winrt
{
Expand All @@ -60,6 +60,25 @@ namespace winrt::TerminalApp::implementation
InitializeComponent();
}

TerminalPage::~TerminalPage()
{
// Dummy line to get a breakpoint somewnere. We can remove this.
_WindowProperties = nullptr;
}

void TerminalPage::BigRedButton()
{
_windowIdToast = nullptr;
_windowRenameFailedToast = nullptr;

_actionDispatch = nullptr;
_tabView = nullptr;
_tabRow = nullptr;
_tabContent = nullptr;
_newTabButton = nullptr;
_tabColorPicker = nullptr;
_settings = nullptr;
}
// Method Description:
// - implements the IInitializeWithWindow interface from shobjidl_core.
// - We're going to use this HWND as the owner for the ConPTY windows, via
Expand Down Expand Up @@ -222,7 +241,7 @@ namespace winrt::TerminalApp::implementation
_RegisterActionCallbacks();

// Hook up inbound connection event handler
ConptyConnection::NewConnection({ this, &TerminalPage::_OnNewConnection });
ConptyConnection::NewConnection({ get_weak(), &TerminalPage::_OnNewConnection });

//Event Bindings (Early)
_newTabButton.Click([weakThis{ get_weak() }](auto&&, auto&&) {
Expand All @@ -237,9 +256,9 @@ namespace winrt::TerminalApp::implementation
page->NewTerminalByDrop(e);
}
});
_tabView.SelectionChanged({ this, &TerminalPage::_OnTabSelectionChanged });
_tabView.TabCloseRequested({ this, &TerminalPage::_OnTabCloseRequested });
_tabView.TabItemsChanged({ this, &TerminalPage::_OnTabItemsChanged });
_tabView.SelectionChanged({ get_weak(), &TerminalPage::_OnTabSelectionChanged });
_tabView.TabCloseRequested({ get_weak(), &TerminalPage::_OnTabCloseRequested });
_tabView.TabItemsChanged({ get_weak(), &TerminalPage::_OnTabItemsChanged });

_CreateNewTabFlyout();

Expand All @@ -248,16 +267,16 @@ namespace winrt::TerminalApp::implementation
// When the visibility of the command palette changes to "collapsed",
// the palette has been closed. Toss focus back to the currently active
// control.
CommandPalette().RegisterPropertyChangedCallback(UIElement::VisibilityProperty(), [this](auto&&, auto&&) {
if (CommandPalette().Visibility() == Visibility::Collapsed)
{
_FocusActiveControl(nullptr, nullptr);
}
});
CommandPalette().DispatchCommandRequested({ this, &TerminalPage::_OnDispatchCommandRequested });
CommandPalette().CommandLineExecutionRequested({ this, &TerminalPage::_OnCommandLineExecutionRequested });
CommandPalette().SwitchToTabRequested({ this, &TerminalPage::_OnSwitchToTabRequested });
CommandPalette().PreviewAction({ this, &TerminalPage::_PreviewActionHandler });
// CommandPalette().RegisterPropertyChangedCallback(UIElement::VisibilityProperty(), [this](auto&&, auto&&) {
// if (CommandPalette().Visibility() == Visibility::Collapsed)
// {
// _FocusActiveControl(nullptr, nullptr);
// }
// });
CommandPalette().DispatchCommandRequested({ get_weak(), &TerminalPage::_OnDispatchCommandRequested });
CommandPalette().CommandLineExecutionRequested({ get_weak(), &TerminalPage::_OnCommandLineExecutionRequested });
CommandPalette().SwitchToTabRequested({ get_weak(), &TerminalPage::_OnSwitchToTabRequested });
CommandPalette().PreviewAction({ get_weak(), &TerminalPage::_PreviewActionHandler });

// Settings AllowDependentAnimations will affect whether animations are
// enabled application-wide, so we don't need to check it each time we
Expand All @@ -269,7 +288,7 @@ namespace winrt::TerminalApp::implementation
// window will be, so they can subdivide that space.
//
// _OnFirstLayout will remove this handler so it doesn't get called more than once.
_layoutUpdatedRevoker = _tabContent.LayoutUpdated(winrt::auto_revoke, { this, &TerminalPage::_OnFirstLayout });
_layoutUpdatedRevoker = _tabContent.LayoutUpdated(winrt::auto_revoke, { get_weak(), &TerminalPage::_OnFirstLayout });

_isAlwaysOnTop = _settings.GlobalSettings().AlwaysOnTop();

Expand Down Expand Up @@ -788,7 +807,7 @@ namespace winrt::TerminalApp::implementation
ico.Symbol(WUX::Controls::Symbol::Setting);
settingsItem.Icon(ico);

settingsItem.Click({ this, &TerminalPage::_SettingsButtonOnClick });
settingsItem.Click({ get_weak(), &TerminalPage::_SettingsButtonOnClick });
newTabFlyout.Items().Append(settingsItem);

auto actionMap = _settings.ActionMap();
Expand All @@ -811,7 +830,7 @@ namespace winrt::TerminalApp::implementation
commandPaletteIcon.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" });
commandPaletteFlyout.Icon(commandPaletteIcon);

commandPaletteFlyout.Click({ this, &TerminalPage::_CommandPaletteButtonOnClick });
commandPaletteFlyout.Click({ get_weak(), &TerminalPage::_CommandPaletteButtonOnClick });
newTabFlyout.Items().Append(commandPaletteFlyout);

const auto commandPaletteKeyChord{ actionMap.GetKeyBindingForAction(ShortcutAction::ToggleCommandPalette) };
Expand All @@ -833,7 +852,7 @@ namespace winrt::TerminalApp::implementation
aboutIcon.Symbol(WUX::Controls::Symbol::Help);
aboutFlyout.Icon(aboutIcon);

aboutFlyout.Click({ this, &TerminalPage::_AboutButtonOnClick });
aboutFlyout.Click({ get_weak(), &TerminalPage::_AboutButtonOnClick });
newTabFlyout.Items().Append(aboutFlyout);
}

Expand All @@ -845,9 +864,9 @@ namespace winrt::TerminalApp::implementation
// Since the previous focus location might be discarded in the background,
// e.g., the command palette will be dismissed by the menu,
// and then closing the fly-out will move the focus to wrong location.
newTabFlyout.Opening([this](auto&&, auto&&) {
_FocusCurrentTab(true);
});
// newTabFlyout.Opening([this](auto&&, auto&&) {
// _FocusCurrentTab(true);
// });
_newTabButton.Flyout(newTabFlyout);
}

Expand Down Expand Up @@ -1554,16 +1573,16 @@ namespace winrt::TerminalApp::implementation
// - term: The newly created TermControl to connect the events for
void TerminalPage::_RegisterTerminalEvents(TermControl term)
{
term.RaiseNotice({ this, &TerminalPage::_ControlNoticeRaisedHandler });
term.RaiseNotice({ get_weak(), &TerminalPage::_ControlNoticeRaisedHandler });

// Add an event handler when the terminal's selection wants to be copied.
// When the text buffer data is retrieved, we'll copy the data into the Clipboard
term.CopyToClipboard({ this, &TerminalPage::_CopyToClipboardHandler });
term.CopyToClipboard({ get_weak(), &TerminalPage::_CopyToClipboardHandler });

// Add an event handler when the terminal wants to paste data from the Clipboard.
term.PasteFromClipboard({ this, &TerminalPage::_PasteFromClipboardHandler });
term.PasteFromClipboard({ get_weak(), &TerminalPage::_PasteFromClipboardHandler });

term.OpenHyperlink({ this, &TerminalPage::_OpenHyperlinkHandler });
term.OpenHyperlink({ get_weak(), &TerminalPage::_OpenHyperlinkHandler });

term.HidePointerCursor({ get_weak(), &TerminalPage::_HidePointerCursorHandler });
term.RestorePointerCursor({ get_weak(), &TerminalPage::_RestorePointerCursorHandler });
Expand Down Expand Up @@ -3405,7 +3424,7 @@ namespace winrt::TerminalApp::implementation
}

// GH#8767 - let unhandled keys in the SUI try to run commands too.
sui.KeyDown({ this, &TerminalPage::_KeyDownHandler });
sui.KeyDown({ get_weak(), &TerminalPage::_KeyDownHandler });

sui.OpenJson([weakThis{ get_weak() }](auto&& /*s*/, winrt::Microsoft::Terminal::Settings::Model::SettingsTarget e) {
if (auto page{ weakThis.get() })
Expand Down Expand Up @@ -3436,7 +3455,7 @@ namespace winrt::TerminalApp::implementation
// Update the state of the close button to match the current theme
_updateTabCloseButton(tabViewItem);

tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick });
tabViewItem.PointerPressed({ get_weak(), &TerminalPage::_OnTabClick });

// When the tab requests close, try to close it (prompt for approval, if required)
newTabImpl->CloseRequested([weakTab, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
Expand Down Expand Up @@ -3850,7 +3869,7 @@ namespace winrt::TerminalApp::implementation
else if (key == Windows::System::VirtualKey::Escape)
{
// User wants to discard the changes they made
WindowRenamerTextBox().Text(WindowProperties().WindowName());
// WindowRenamerTextBox().Text(WindowProperties().WindowName());
WindowRenamer().IsOpen(false);
_renamerPressedEnter = false;
}
Expand Down
10 changes: 6 additions & 4 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ namespace winrt::TerminalApp::implementation
{
public:
TerminalPage();
void BigRedButton();
~TerminalPage();

// This implements shobjidl's IInitializeWithWindow, but due to a XAML Compiler bug we cannot
// put it in our inheritance graph. https://github.com/microsoft/microsoft-ui-xaml/issues/3331
Expand Down Expand Up @@ -116,10 +118,10 @@ namespace winrt::TerminalApp::implementation
const winrt::hstring cwd = L"");

// For the sake of XAML binding:
winrt::hstring WindowName() const noexcept { return _WindowProperties.WindowName(); };
uint64_t WindowId() const noexcept { return _WindowProperties.WindowId(); };
winrt::hstring WindowIdForDisplay() const noexcept { return _WindowProperties.WindowIdForDisplay(); };
winrt::hstring WindowNameForDisplay() const noexcept { return _WindowProperties.WindowNameForDisplay(); };
winrt::hstring WindowName() const noexcept { return L""; /*return _WindowProperties.WindowName();*/ };
uint64_t WindowId() const noexcept { return 1u; /*return _WindowProperties.WindowId();*/ };
winrt::hstring WindowIdForDisplay() const noexcept { return L""; /*return _WindowProperties.WindowIdForDisplay();*/ };
winrt::hstring WindowNameForDisplay() const noexcept { return L""; /*return _WindowProperties.WindowNameForDisplay();*/ };

void SetNumberOfOpenWindows(const uint64_t value);

Expand Down
56 changes: 31 additions & 25 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,18 @@ namespace winrt::TerminalApp::implementation
_isElevated = ::Microsoft::Console::Utils::IsElevated();
}

TerminalWindow::~TerminalWindow()
{
_root->BigRedButton();
_root = nullptr;
}

// Method Description:
// - Implements the IInitializeWithWindow interface from shobjidl_core.
HRESULT TerminalWindow::Initialize(HWND hwnd)
{
_root = winrt::make_self<TerminalPage>();
_root->WindowProperties(*this);
// _root->WindowProperties(*this);
_dialog = ContentDialog{};

// Pass commandline args into the TerminalPage. If we were supposed to
Expand Down Expand Up @@ -237,30 +243,30 @@ namespace winrt::TerminalApp::implementation
}

_root->SetSettings(_settings, false);
_root->Loaded({ this, &TerminalWindow::_OnLoaded });
_root->Initialized([this](auto&&, auto&&) {
// GH#288 - When we finish initialization, if the user wanted us
// launched _fullscreen_, toggle fullscreen mode. This will make sure
// that the window size is _first_ set up as something sensible, so
// leaving fullscreen returns to a reasonable size.
const auto launchMode = this->GetLaunchMode();
if (IsQuakeWindow() || WI_IsFlagSet(launchMode, LaunchMode::FocusMode))
{
_root->SetFocusMode(true);
}

// The IslandWindow handles (creating) the maximized state
// we just want to record it here on the page as well.
if (WI_IsFlagSet(launchMode, LaunchMode::MaximizedMode))
{
_root->Maximized(true);
}

if (WI_IsFlagSet(launchMode, LaunchMode::FullscreenMode) && !IsQuakeWindow())
{
_root->SetFullscreen(true);
}
});
_root->Loaded({ get_weak(), &TerminalWindow::_OnLoaded });
// _root->Initialized([this](auto&&, auto&&) {
// // GH#288 - When we finish initialization, if the user wanted us
// // launched _fullscreen_, toggle fullscreen mode. This will make sure
// // that the window size is _first_ set up as something sensible, so
// // leaving fullscreen returns to a reasonable size.
// const auto launchMode = this->GetLaunchMode();
// if (IsQuakeWindow() || WI_IsFlagSet(launchMode, LaunchMode::FocusMode))
// {
// _root->SetFocusMode(true);
// }

// // The IslandWindow handles (creating) the maximized state
// // we just want to record it here on the page as well.
// if (WI_IsFlagSet(launchMode, LaunchMode::MaximizedMode))
// {
// _root->Maximized(true);
// }

// if (WI_IsFlagSet(launchMode, LaunchMode::FullscreenMode) && !IsQuakeWindow())
// {
// _root->SetFullscreen(true);
// }
// });
_root->Create();

_RefreshThemeRoutine();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace winrt::TerminalApp::implementation
{
public:
TerminalWindow(const TerminalApp::SettingsLoadEventArgs& settingsLoadedResult);
~TerminalWindow() = default;
~TerminalWindow();

STDMETHODIMP Initialize(HWND hwnd);

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ AppHost::~AppHost()
_revokers = {};

_showHideWindowThrottler.reset();

_window = nullptr;
_windowLogic = nullptr;
}

bool AppHost::OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down)
Expand Down
7 changes: 5 additions & 2 deletions src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,12 @@ WindowEmperor::~WindowEmperor()
// for this thread. It's a real thinker.
//
// I need someone to help take a look at this with me.

_dispatcher = nullptr;
_getWindowLayoutThrottler.reset();
_manager = nullptr;
_app.Close();
_app = nullptr;
// Sleep(20000);
}

void _buildArgsFromCommandline(std::vector<winrt::hstring>& args)
Expand Down Expand Up @@ -142,7 +145,7 @@ void WindowEmperor::CreateNewWindowThread(Remoting::WindowRequestedArgs args, co
// Add a callback to the window's logic to let us know when the window's
// quake mode state changes. We'll use this to check if we need to add
// or remove the notification icon.
sender->Logic().IsQuakeWindowChanged([this](auto&&, auto&&) -> winrt::fire_and_forget {
sender->Logic().IsQuakeWindowChanged([this](auto&&, auto &&) -> winrt::fire_and_forget {
co_await wil::resume_foreground(this->_dispatcher);
this->_checkWindowsForNotificationIcon();
});
Expand Down
8 changes: 8 additions & 0 deletions src/cascadia/WindowsTerminal/WindowThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ void WindowThread::Start()
const auto exitCode = WindowProc();
_host = nullptr;

{
MSG msg = {};
while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE))
{
::DispatchMessageW(&msg);
}
}

_ExitedHandlers(_peasant.GetID());
return exitCode;
});
Expand Down

0 comments on commit 667b658

Please sign in to comment.