Skip to content

Commit

Permalink
Quake logic seems to work again.
Browse files Browse the repository at this point in the history 
  Down to just 10 TODOs left
  • Loading branch information
@zadjii-msft
zadjii-msft committed Feb 9, 2023
1 parent e214624 commit c69f0bc
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 108 deletions.
15 changes: 0 additions & 15 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4255,27 +4255,12 @@ namespace winrt::TerminalApp::implementation
_PropertyChangedHandlers(*this, WUX::Data::PropertyChangedEventArgs{ L"WindowNameForDisplay" });
_PropertyChangedHandlers(*this, WUX::Data::PropertyChangedEventArgs{ L"WindowIdForDisplay" });

// if (changed)
// {
// DON'T display the confirmation if this is the name we were
// given on startup!
if (page->_startupState == StartupState::Initialized)
{
page->IdentifyWindow();

// TODO! This is wacky. Reconcile with oldIsQuakeMode in TerminalWindow::WindowName

// // If we're entering quake mode, or leaving it
// if (IsQuakeWindow() != oldIsQuakeMode)
// {
// // If we're entering Quake Mode from ~Focus Mode, then this will enter Focus Mode
// // If we're entering Quake Mode from Focus Mode, then this will do nothing
// // If we're leaving Quake Mode (we're already in Focus Mode), then this will do nothing
// SetFocusMode(true);
// _IsQuakeWindowChangedHandlers(*this, nullptr);
// }
}
// }
}
}

Expand Down
25 changes: 2 additions & 23 deletions src/cascadia/TerminalApp/TerminalPage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,6 @@ namespace winrt::TerminalApp::implementation
_ProposedName{ name } {};
};

// struct WindowProperties : WindowPropertiesT<WindowProperties>
// {
// // Normally, WindowName and WindowId would be
// // WINRT_OBSERVABLE_PROPERTY's, but we want them to raise
// // WindowNameForDisplay and WindowIdForDisplay instead
// winrt::hstring WindowName() const noexcept;
// winrt::fire_and_forget WindowName(const winrt::hstring& value);
// uint64_t WindowId() const noexcept;
// void WindowId(const uint64_t& value);
// winrt::hstring WindowIdForDisplay() const noexcept;
// winrt::hstring WindowNameForDisplay() const noexcept;
// bool IsQuakeWindow() const noexcept;

// public:
// };

struct TerminalPage : TerminalPageT<TerminalPage>
{
public:
Expand All @@ -79,7 +63,6 @@ namespace winrt::TerminalApp::implementation

void Create();

// bool ShouldUsePersistedLayout(Microsoft::Terminal::Settings::Model::CascadiaSettings& settings) const;
bool ShouldImmediatelyHandoffToElevated(const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings) const;
void HandoffToElevated(const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);

Expand Down Expand Up @@ -140,7 +123,6 @@ namespace winrt::TerminalApp::implementation

void SetNumberOfOpenWindows(const uint64_t value);

// bool IsQuakeWindow() const noexcept;
bool IsElevated() const noexcept;

void OpenSettingsUI();
Expand All @@ -152,8 +134,6 @@ namespace winrt::TerminalApp::implementation
void WindowProperties(const TerminalApp::IWindowProperties& props);
winrt::fire_and_forget WindowNameChanged();

// WINRT_PROPERTY(TerminalApp::IWindowProperties, WindowProperties, nullptr);

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);

// -------------------------------- WinRT Events ---------------------------------
Expand All @@ -169,7 +149,7 @@ namespace winrt::TerminalApp::implementation
TYPED_EVENT(Initialized, IInspectable, winrt::Windows::UI::Xaml::RoutedEventArgs);
TYPED_EVENT(IdentifyWindowsRequested, IInspectable, IInspectable);
TYPED_EVENT(RenameWindowRequested, Windows::Foundation::IInspectable, winrt::TerminalApp::RenameWindowRequestedArgs);
TYPED_EVENT(IsQuakeWindowChanged, IInspectable, IInspectable);

TYPED_EVENT(SummonWindowRequested, IInspectable, IInspectable);
TYPED_EVENT(CloseRequested, IInspectable, IInspectable);
TYPED_EVENT(OpenSystemMenu, IInspectable, IInspectable);
Expand Down Expand Up @@ -208,8 +188,7 @@ namespace winrt::TerminalApp::implementation
bool _isFullscreen{ false };
bool _isMaximized{ false };
bool _isAlwaysOnTop{ false };
// winrt::hstring _WindowName{};
// uint64_t _WindowId{ 0 };

std::optional<uint32_t> _loadFromPersistedLayoutIdx{};
uint64_t _numOpenWindows{ 0 };

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ namespace TerminalApp
event Windows.Foundation.TypedEventHandler<Object, Object> SetTaskbarProgress;
event Windows.Foundation.TypedEventHandler<Object, Object> IdentifyWindowsRequested;
event Windows.Foundation.TypedEventHandler<Object, RenameWindowRequestedArgs> RenameWindowRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> IsQuakeWindowChanged;

event Windows.Foundation.TypedEventHandler<Object, Object> SummonWindowRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> CloseRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> OpenSystemMenu;
Expand Down
56 changes: 31 additions & 25 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1116,29 +1116,6 @@ namespace winrt::TerminalApp::implementation
}
}
////////////////////////////////////////////////////////////////////////////

void TerminalWindow::IdentifyWindow()
{
if (_root)
{
_root->IdentifyWindow();
}
}

void TerminalWindow::RenameFailed()
{
if (_root)
{
_root->RenameFailed();
}
}

// TODO!
// bool TerminalWindow::IsQuakeWindow() const noexcept
// {
// return _root->IsQuakeWindow();
// }

void TerminalWindow::RequestExitFullscreen()
{
_root->SetFullscreen(false);
Expand All @@ -1157,6 +1134,22 @@ namespace winrt::TerminalApp::implementation

////////////////////////////////////////////////////////////////////////////

void TerminalWindow::IdentifyWindow()
{
if (_root)
{
_root->IdentifyWindow();
}
}

void TerminalWindow::RenameFailed()
{
if (_root)
{
_root->RenameFailed();
}
}

// WindowName is a otherwise generic WINRT_OBSERVABLE_PROPERTY, but it needs
// to raise a PropertyChanged for WindowNameForDisplay, instead of
// WindowName.
Expand All @@ -1167,15 +1160,25 @@ namespace winrt::TerminalApp::implementation

void TerminalWindow::WindowName(const winrt::hstring& value)
{
// TODO!
// const auto oldIsQuakeMode = IsQuakeWindow();
const auto oldIsQuakeMode = IsQuakeWindow();

const auto changed = _WindowName != value;
if (changed)
{
_WindowName = value;
if (_root)
{
_root->WindowNameChanged();

// If we're entering quake mode, or leaving it
if (IsQuakeWindow() != oldIsQuakeMode)
{
// If we're entering Quake Mode from ~Focus Mode, then this will enter Focus Mode
// If we're entering Quake Mode from Focus Mode, then this will do nothing
// If we're leaving Quake Mode (we're already in Focus Mode), then this will do nothing
_root->SetFocusMode(true);
_IsQuakeWindowChangedHandlers(*this, nullptr);
}
}
}
}
Expand Down Expand Up @@ -1229,6 +1232,7 @@ namespace winrt::TerminalApp::implementation
{
return WindowName() == QuakeWindowName;
}

////////////////////////////////////////////////////////////////////////////

bool TerminalWindow::ShouldImmediatelyHandoffToElevated()
Expand Down Expand Up @@ -1260,4 +1264,6 @@ namespace winrt::TerminalApp::implementation
return;
}
}

////////////////////////////////////////////////////////////////////////////
};
5 changes: 2 additions & 3 deletions src/cascadia/TerminalApp/TerminalWindow.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ namespace winrt::TerminalApp::implementation
winrt::fire_and_forget UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args);

bool HasCommandlineArguments() const noexcept;
// bool HasSettingsStartupActions() const noexcept;
int32_t SetStartupCommandline(array_view<const winrt::hstring> actions);
int32_t ExecuteCommandline(array_view<const winrt::hstring> actions, const winrt::hstring& cwd);
void SetSettingsStartupArgs(const std::vector<winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs>& actions);
Expand All @@ -78,7 +77,6 @@ namespace winrt::TerminalApp::implementation
void SetNumberOfOpenWindows(const uint64_t num);
bool ShouldUsePersistedLayout() const;

// bool IsQuakeWindow() const noexcept;
void RequestExitFullscreen();

Windows::Foundation::Size GetLaunchDimensions(uint32_t dpi);
Expand Down Expand Up @@ -190,13 +188,14 @@ namespace winrt::TerminalApp::implementation
FORWARDED_TYPED_EVENT(SetTaskbarProgress, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, SetTaskbarProgress);
FORWARDED_TYPED_EVENT(IdentifyWindowsRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, IdentifyWindowsRequested);
FORWARDED_TYPED_EVENT(RenameWindowRequested, Windows::Foundation::IInspectable, winrt::TerminalApp::RenameWindowRequestedArgs, _root, RenameWindowRequested);
FORWARDED_TYPED_EVENT(IsQuakeWindowChanged, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, IsQuakeWindowChanged);
FORWARDED_TYPED_EVENT(SummonWindowRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, SummonWindowRequested);
FORWARDED_TYPED_EVENT(CloseRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, CloseRequested);
FORWARDED_TYPED_EVENT(OpenSystemMenu, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, OpenSystemMenu);
FORWARDED_TYPED_EVENT(QuitRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, QuitRequested);
FORWARDED_TYPED_EVENT(ShowWindowChanged, Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Control::ShowWindowArgs, _root, ShowWindowChanged);

TYPED_EVENT(IsQuakeWindowChanged, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable);

TYPED_EVENT(SystemMenuChangeRequested, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::SystemMenuChangeArgs);

TYPED_EVENT(SettingsChanged, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::SettingsLoadEventArgs);
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalApp/TerminalWindow.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ namespace TerminalApp
Boolean IsElevated();

Boolean HasCommandlineArguments();
// Boolean HasSettingsStartupActions();

Int32 SetStartupCommandline(String[] commands);

Expand All @@ -83,7 +82,6 @@ namespace TerminalApp
void SetNumberOfOpenWindows(UInt64 num);
void RenameFailed();
void RequestExitFullscreen();
// Boolean IsQuakeWindow();

Windows.Foundation.Size GetLaunchDimensions(UInt32 dpi);
Boolean CenterOnLaunch { get; };
Expand Down
27 changes: 1 addition & 26 deletions src/cascadia/WindowsTerminal/AppHost.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ void AppHost::_HandleCommandlineArgs()
const auto numPeasants = _windowManager2.GetNumberOfPeasants();
if (numPeasants == 1)
{
// TODO! this is vaugely off by one. Not sure, but if you restore 2 windows, you seem to get two copies of the second. Yikes.
const auto layouts = ApplicationState::SharedInstance().PersistedWindowLayouts();
if (_appLogic.ShouldUsePersistedLayout() &&
layouts &&
Expand Down Expand Up @@ -439,16 +440,6 @@ void AppHost::AppTitleChanged(const winrt::Windows::Foundation::IInspectable& /*
// - <none>
void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::TerminalApp::LastTabClosedEventArgs& /*args*/)
{
// TODO!
// if (_windowManager2.IsMonarch() && _notificationIcon)
// {
// _DestroyNotificationIcon();
// }
// else if (_window->IsQuakeWindow())
// {
// _HideNotificationIconRequested(nullptr, nullptr);
// }

// We don't want to try to save layouts if we are about to close.
_windowManager2.GetWindowLayoutRequested(_GetWindowLayoutRequestedToken);

Expand Down Expand Up @@ -1102,22 +1093,6 @@ void AppHost::_HandleSettingsChanged(const winrt::Windows::Foundation::IInspecta
void AppHost::_IsQuakeWindowChanged(const winrt::Windows::Foundation::IInspectable&,
const winrt::Windows::Foundation::IInspectable&)
{
// // We want the quake window to be accessible through the notification icon.
// // This means if there's a quake window _somewhere_, we want the notification icon
// // to show regardless of the notification icon settings.
// // This also means we'll need to destroy the notification icon if it was created
// // specifically for the quake window. If not, it should not be destroyed.
// if (!_window->IsQuakeWindow() && _windowLogic.IsQuakeWindow())
// {
// _ShowNotificationIconRequested(nullptr, nullptr);
// }
// else if (_window->IsQuakeWindow() && !_windowLogic.IsQuakeWindow())
// {
// _HideNotificationIconRequested(nullptr, nullptr);
// }

// TODO! I think we need the emperor to listen to windowLogic's IsQuakeWindowChanged event, to replicate this

_window->IsQuakeWindow(_windowLogic.IsQuakeWindow());
}

Expand Down
27 changes: 14 additions & 13 deletions src/cascadia/WindowsTerminal/WindowEmperor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ void WindowEmperor::CreateNewWindowThread(Remoting::WindowRequestedArgs args, co
auto func = [this, args, peasant, firstWindow]() {
auto window{ std::make_shared<WindowThread>(_app.Logic(), args, _manager, peasant) };
_windows.push_back(window);

window->Logic().IsQuakeWindowChanged([this](auto&&, auto &&) -> winrt::fire_and_forget {
co_await wil::resume_foreground(this->_dispatcher);
this->_checkWindowsForNotificationIcon();
});

return window->WindowProc();
};

Expand All @@ -176,22 +182,12 @@ void WindowEmperor::_becomeMonarch()

////////////////////////////////////////////////////////////////////////////

// if (_windowManager2.DoesQuakeWindowExist() ||
// _window->IsQuakeWindow() ||
// (_windowLogic.GetAlwaysShowNotificationIcon() || _windowLogic.GetMinimizeToNotificationArea()))
// {
// _CreateNotificationIcon();
// }

// // These events are coming from peasants that become or un-become quake windows.
// _revokers.ShowNotificationIconRequested = _windowManager2.ShowNotificationIconRequested(winrt::auto_revoke, { this, &AppHost::_ShowNotificationIconRequested });
// _revokers.HideNotificationIconRequested = _windowManager2.HideNotificationIconRequested(winrt::auto_revoke, { this, &AppHost::_HideNotificationIconRequested });
_checkWindowsForNotificationIcon();

////////////////////////////////////////////////////////////////////////////

// // Set the number of open windows (so we know if we are the last window)
// // and subscribe for updates if there are any changes to that number.
// _windowLogic.SetNumberOfOpenWindows(_windowManager2.GetNumberOfPeasants());
// Set the number of open windows (so we know if we are the last window)
// and subscribe for updates if there are any changes to that number.

_WindowCreatedToken = _manager.WindowCreated({ this, &WindowEmperor::_numberOfWindowsChanged });
_WindowClosedToken = _manager.WindowClosed({ this, &WindowEmperor::_numberOfWindowsChanged });
Expand Down Expand Up @@ -224,6 +220,11 @@ void WindowEmperor::_numberOfWindowsChanged(const winrt::Windows::Foundation::II
{
_windowThread->Logic().SetNumberOfOpenWindows(numWindows);
}

// If we closed out the quake window, and don't otherwise need the tray
// icon, let's get rid of it.
_checkWindowsForNotificationIcon();

// TODO! apparently, we crash when whe actually post a quit, handle it, and
// then leak all our threads. That's not good, but also, this should only
// happen once our threads all exited. So figure that out.
Expand Down

1 comment on commit c69f0bc

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@check-spelling-bot Report

🔴 Please review

See the 📜action log for details.

Unrecognized words (13) 
APeasant
Apphost
applogic
backchannel
becuase
chould
connectecd
definted
fuction
initilized
propogating
vaugely
whe
Previously acknowledged words that are now absent CLA demoable everytime Hirots inthread reingest unmark :arrow_right:
To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the git@github.com:microsoft/terminal.git repository
on the dev/migrie/oop/3/ainulindale branch (ℹ️ how do I use this?):

curl -s -S -L 'https://github.com/raw/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/4138082543/attempts/1'
Errors (1)

See the 📜action log for details.

❌ Errors Count
❌ forbidden-pattern 1

See ❌ Event descriptions for more information.

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Please sign in to comment.