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

Persist window layout on window close #10972

Merged
40 commits merged into from Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
405c853
GH766 Persist tab layout on window close
Rosefield Aug 18, 2021
6297417
Add missing comment
Rosefield Aug 18, 2021
a265e81
Try to save pane color information
Rosefield Aug 18, 2021
48cde20
formatting
Rosefield Aug 18, 2021
4b897d0
Also persist window size and location information
Rosefield Aug 18, 2021
773cc79
Remove state if the user manually closed all of their tabs (instead o…
Rosefield Aug 18, 2021
37e5bd4
Capitalize to make the spellchecker happy?
Rosefield Aug 18, 2021
1b6dfc5
Also format. Again. Maybe Ill learn my lesson one of these days.
Rosefield Aug 18, 2021
c1722c7
Update setting name to better reflect what it actually does
Rosefield Aug 19, 2021
d6997f9
Use inheritable settings correctly for the applied color scheme
Rosefield Aug 19, 2021
8c64dd5
Also save the tabs focused pane and zoom information
Rosefield Aug 19, 2021
97bce32
Fix pane id calculation, save the correct split size
Rosefield Aug 19, 2021
ec79c93
Foiled by whitespace again.
Rosefield Aug 19, 2021
d7ce2de
hyphenation to appease the spell checker
Rosefield Aug 19, 2021
91c3d69
Update comment on Pane::BuildStartupActions
Rosefield Aug 19, 2021
ef80c66
Switch to serializing an array of window layouts so that the schema c…
Rosefield Aug 20, 2021
0a3ccd1
Add consts for json keys
Rosefield Aug 22, 2021
8d6a7ee
use consts in more places
Rosefield Aug 23, 2021
0a6d4f7
Add to schema
Rosefield Aug 23, 2021
c0f7eaa
Save the correct location of the terminal.
Rosefield Aug 23, 2021
2062eb3
Save if we are the only window open, instead of if we are the first w…
Rosefield Aug 23, 2021
464ca59
Code review changes. Also fix restoring position on startup.
Rosefield Aug 24, 2021
3b02719
even more formatting.
Rosefield Aug 24, 2021
d204f00
minor cleanup
Rosefield Aug 24, 2021
1cd14b3
Put json converters in more reasonable places, improve the WindowLayo…
Rosefield Aug 24, 2021
cfe3583
try to move the actions more forcefully
Rosefield Aug 26, 2021
072822a
Switch to using a first window behavior setting (for future expansion…
Rosefield Aug 26, 2021
b8b7d48
code review: comments, better event handlers, and exception logging.
Rosefield Aug 26, 2021
4b1bbbd
Try to more accurately detect if the user provided no commandline arg…
Rosefield Aug 26, 2021
50cf7fb
Merge remote-tracking branch 'origin/main' into feature/gh766-persist…
Rosefield Aug 31, 2021
81af065
even merges arent safe from the formatter.
Rosefield Aug 31, 2021
ef21ca6
remove old setting from the schema.
Rosefield Aug 31, 2021
ba56ded
Merge remote-tracking branch 'origin/main' into feature/gh766-persist…
Rosefield Aug 31, 2021
6aeabb1
Make the NewTerminalArgs::Equals more like the other Equals implement…
Rosefield Sep 2, 2021
7f14c7d
formatting
Rosefield Sep 2, 2021
8e22231
Add a feature flag to disable this feature in release.
Rosefield Sep 3, 2021
2f39ed9
Merge remote-tracking branch 'origin/main' into feature/gh766-persist…
Rosefield Sep 3, 2021
735de10
formatting
Rosefield Sep 3, 2021
230a1ab
Code review: Make more things const that can be const
Rosefield Sep 3, 2021
8f38e1b
Initialize properties that were uninitialized
Rosefield Sep 8, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/cascadia/profiles.schema.json
Expand Up @@ -1231,6 +1231,15 @@
"description": "When set to true, this enables the launch of Windows Terminal at startup. Setting this to false will disable the startup task entry. If the Windows Terminal startup task entry is disabled either by org policy or by user action this setting will have no effect.",
"type": "boolean"
},
"firstWindowPreference": {
"default": "defaultProfile",
"description": "Defines what behavior the terminal takes when it starts. \"defaultProfile\" will have the terminal launch with one tab of the default profile, and \"persistedWindowLayout\" will cause the terminal to save its layout on close and reload it on open.",
"enum": [
"defaultProfile",
"persistedWindowLayout"
],
"type": "string"
},
Comment on lines +1234 to +1242
Copy link
Member

Choose a reason for hiding this comment

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

Boy this sure is a mouthful, but hey it works great in the SUI so who cares

"launchMode": {
"default": "default",
"description": "Defines whether the terminal will launch as maximized, full screen, or in a window. Setting this to \"focus\" is equivalent to launching the terminal in the \"default\" mode, but with the focus mode enabled. Similar, setting this to \"maximizedFocus\" will result in launching the terminal in a maximized window with the focus mode enabled.",
Expand Down
41 changes: 41 additions & 0 deletions src/cascadia/Remoting/Monarch.cpp
Expand Up @@ -91,6 +91,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TraceLoggingUInt64(newPeasantsId, "peasantID", "the ID of the new peasant"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));

_WindowCreatedHandlers(nullptr, nullptr);
return newPeasantsId;
}
catch (...)
Expand All @@ -107,6 +109,45 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
}
}

// Method Description:
// - Tells the monarch that a peasant is being closed.
// Arguments:
// - peasantId: the id of the peasant
// Return Value:
// - <none>
void Monarch::SignalClose(const uint64_t peasantId)
{
_peasants.erase(peasantId);
_WindowClosedHandlers(nullptr, nullptr);
}

// Method Description:
// - Counts the number of living peasants.
// Arguments:
// - <none>
// Return Value:
// - the number of active peasants.
uint64_t Monarch::GetNumberOfPeasants()
{
auto num = 0;
auto callback = [&](auto&& p, auto&& /*id*/) {
// Check that the peasant is alive, and if so increment the count
p.GetID();
num += 1;
};
auto onError = [](auto&& id) {
TraceLoggingWrite(g_hRemotingProvider,
"Monarch_GetNumberOfPeasants_Failed",
TraceLoggingInt64(id, "peasantID", "The ID of the peasant which we could not enumerate"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));
};

_forAllPeasantsIgnoringTheDead(callback, onError);

return num;
}

// Method Description:
// - Event handler for the Peasant::WindowActivated event. Used as an
// opportunity for us to update our internal stack of the "most recent
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/Remoting/Monarch.h
Expand Up @@ -47,6 +47,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
uint64_t GetPID();

uint64_t AddPeasant(winrt::Microsoft::Terminal::Remoting::IPeasant peasant);
void SignalClose(const uint64_t peasantId);

uint64_t GetNumberOfPeasants();

winrt::Microsoft::Terminal::Remoting::ProposeCommandlineResult ProposeCommandline(const winrt::Microsoft::Terminal::Remoting::CommandlineArgs& args);
void HandleActivatePeasant(const winrt::Microsoft::Terminal::Remoting::WindowActivatedArgs& args);
Expand All @@ -58,6 +61,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TYPED_EVENT(FindTargetWindowRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs);
TYPED_EVENT(ShowTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(HideTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(WindowCreated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(WindowClosed, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);

private:
uint64_t _ourPID;
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/Remoting/Monarch.idl
Expand Up @@ -37,15 +37,19 @@ namespace Microsoft.Terminal.Remoting

UInt64 GetPID();
UInt64 AddPeasant(IPeasant peasant);
UInt64 GetNumberOfPeasants();
ProposeCommandlineResult ProposeCommandline(CommandlineArgs args);
void HandleActivatePeasant(WindowActivatedArgs args);
void SummonWindow(SummonWindowSelectionArgs args);
void SignalClose(UInt64 peasantId);

void SummonAllWindows();
Windows.Foundation.Collections.IMapView<UInt64, String> GetPeasantNames { get; };

event Windows.Foundation.TypedEventHandler<Object, FindTargetWindowArgs> FindTargetWindowRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> ShowTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> HideTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> WindowCreated;
event Windows.Foundation.TypedEventHandler<Object, Object> WindowClosed;
};
}
32 changes: 31 additions & 1 deletion src/cascadia/Remoting/WindowManager.cpp
Expand Up @@ -54,6 +54,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// monarch!
CoRevokeClassObject(_registrationHostClass);
_registrationHostClass = 0;
SignalClose();
_monarchWaitInterrupt.SetEvent();

// A thread is joinable once it's been started. Basically this just
Expand All @@ -64,6 +65,18 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
}
}

void WindowManager::SignalClose()
{
if (_monarch)
{
try
{
_monarch.SignalClose(_peasant.GetID());
}
CATCH_LOG()
}
}

void WindowManager::ProposeCommandline(const Remoting::CommandlineArgs& args)
{
// If we're the king, we _definitely_ want to process the arguments, we were
Expand Down Expand Up @@ -250,9 +263,13 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// Here, we're the king!
//
// This is where you should do any additional setup that might need to be
// done when we become the king. THis will be called both for the first
// done when we become the king. This will be called both for the first
// window, and when the current monarch dies.

auto weakThis{ get_weak() };

_monarch.WindowCreated({ get_weak(), &WindowManager::_WindowCreatedHandlers });
_monarch.WindowClosed({ get_weak(), &WindowManager::_WindowClosedHandlers });
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
_monarch.FindTargetWindowRequested({ this, &WindowManager::_raiseFindTargetWindowRequested });
_monarch.ShowTrayIconRequested([this](auto&&, auto&&) { _ShowTrayIconRequestedHandlers(*this, nullptr); });
_monarch.HideTrayIconRequested([this](auto&&, auto&&) { _HideTrayIconRequestedHandlers(*this, nullptr); });
Expand Down Expand Up @@ -526,6 +543,19 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
return _monarch.GetPeasantNames();
}

uint64_t WindowManager::GetNumberOfPeasants()
{
if (_monarch)
Copy link
Member

Choose a reason for hiding this comment

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

Same deal here with the try/catch. Can we actually even not have a null _monarch? I didn't think we could

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea which is why I was just cautious.

{
try
{
return _monarch.GetNumberOfPeasants();
}
CATCH_LOG()
}
return 0;
}

// Method Description:
// - Ask the monarch to show a tray icon.
// Arguments:
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/Remoting/WindowManager.h
Expand Up @@ -39,16 +39,20 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
winrt::Microsoft::Terminal::Remoting::Peasant CurrentWindow();
bool IsMonarch();
void SummonWindow(const Remoting::SummonWindowSelectionArgs& args);
void SignalClose();

void SummonAllWindows();
Windows::Foundation::Collections::IMapView<uint64_t, winrt::hstring> GetPeasantNames();
uint64_t GetNumberOfPeasants();

winrt::fire_and_forget RequestShowTrayIcon();
winrt::fire_and_forget RequestHideTrayIcon();
bool DoesQuakeWindowExist();

TYPED_EVENT(FindTargetWindowRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs);
TYPED_EVENT(BecameMonarch, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(WindowCreated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(WindowClosed, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(ShowTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(HideTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);

Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/Remoting/WindowManager.idl
Expand Up @@ -8,17 +8,21 @@ namespace Microsoft.Terminal.Remoting
{
WindowManager();
void ProposeCommandline(CommandlineArgs args);
void SignalClose();
Boolean ShouldCreateWindow { get; };
IPeasant CurrentWindow();
Boolean IsMonarch { get; };
void SummonWindow(SummonWindowSelectionArgs args);
void SummonAllWindows();
void RequestShowTrayIcon();
void RequestHideTrayIcon();
UInt64 GetNumberOfPeasants();
Boolean DoesQuakeWindowExist();
Windows.Foundation.Collections.IMapView<UInt64, String> GetPeasantNames();
event Windows.Foundation.TypedEventHandler<Object, FindTargetWindowArgs> FindTargetWindowRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> BecameMonarch;
event Windows.Foundation.TypedEventHandler<Object, Object> WindowCreated;
event Windows.Foundation.TypedEventHandler<Object, Object> WindowClosed;
event Windows.Foundation.TypedEventHandler<Object, Object> ShowTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> HideTrayIconRequested;
};
Expand Down
47 changes: 42 additions & 5 deletions src/cascadia/TerminalApp/AppLogic.cpp
Expand Up @@ -596,12 +596,30 @@ namespace winrt::TerminalApp::implementation
LoadSettings();
}

// Use the default profile to determine how big of a window we need.
const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, nullptr, nullptr) };

auto proposedSize = TermControl::GetProposedDimensions(settings.DefaultSettings(), dpi);
winrt::Windows::Foundation::Size proposedSize{};

const float scale = static_cast<float>(dpi) / static_cast<float>(USER_DEFAULT_SCREEN_DPI);
if (_root->ShouldUsePersistedLayout(_settings))
{
auto layouts = ApplicationState::SharedInstance().PersistedWindowLayouts();
Rosefield marked this conversation as resolved.
Show resolved Hide resolved

if (layouts && layouts.Size() > 0 && layouts.GetAt(0).InitialSize())
{
proposedSize = layouts.GetAt(0).InitialSize().Value();
// The size is saved as a non-scaled real pixel size,
// so we need to scale it appropriately.
proposedSize.Height = proposedSize.Height * scale;
proposedSize.Width = proposedSize.Width * scale;
Comment on lines +609 to +612
Copy link
Member

Choose a reason for hiding this comment

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

This may cause an issue when you restore your window on the wrong DPI screen; worth worrying about?

Copy link
Contributor Author

@Rosefield Rosefield Sep 8, 2021

Choose a reason for hiding this comment

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

When we save we save the "actual" pixel size instead of the visual size(i.e. without any scaling). So if you save on a screen at 150% scaling, and reopen on a screen at 100% scaling, then it will appear smaller (assuming both screens have the same dpi). Whereas if you save on a 150% screen and open on a 150% screen it will appear the same size because we scale it on load.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me! Thanks

}
}

if (proposedSize.Width == 0 && proposedSize.Height == 0)
{
// Use the default profile to determine how big of a window we need.
const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, nullptr, nullptr) };

proposedSize = TermControl::GetProposedDimensions(settings.DefaultSettings(), dpi);
}
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

// GH#2061 - If the global setting "Always show tab bar" is
// set or if "Show tabs in title bar" is set, then we'll need to add
Expand Down Expand Up @@ -683,7 +701,18 @@ namespace winrt::TerminalApp::implementation
LoadSettings();
}

const auto initialPosition{ _settings.GlobalSettings().InitialPosition() };
auto initialPosition{ _settings.GlobalSettings().InitialPosition() };

if (_root->ShouldUsePersistedLayout(_settings))
{
auto layouts = ApplicationState::SharedInstance().PersistedWindowLayouts();
Rosefield marked this conversation as resolved.
Show resolved Hide resolved

if (layouts && layouts.Size() > 0 && layouts.GetAt(0).InitialPosition())
{
initialPosition = layouts.GetAt(0).InitialPosition().Value();
}
}

return {
initialPosition.X ? initialPosition.X.Value() : defaultInitialX,
initialPosition.Y ? initialPosition.Y.Value() : defaultInitialY
Expand Down Expand Up @@ -1429,6 +1458,14 @@ namespace winrt::TerminalApp::implementation
}
}

void AppLogic::SetNumberOfOpenWindows(const uint64_t num)
{
if (_root)
{
_root->SetNumberOfOpenWindows(num);
}
}

void AppLogic::RenameFailed()
{
if (_root)
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Expand Up @@ -69,6 +69,7 @@ namespace winrt::TerminalApp::implementation
void WindowName(const winrt::hstring& name);
uint64_t WindowId();
void WindowId(const uint64_t& id);
void SetNumberOfOpenWindows(const uint64_t num);
bool IsQuakeWindow() const noexcept;

Windows::Foundation::Size GetLaunchDimensions(uint32_t dpi);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.idl
Expand Up @@ -53,6 +53,7 @@ namespace TerminalApp
void IdentifyWindow();
String WindowName;
UInt64 WindowId;
void SetNumberOfOpenWindows(UInt64 num);
void RenameFailed();
Boolean IsQuakeWindow();

Expand Down