starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-08 13:49:31 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Get rid of UpdateTerminalSettings (#17009)

Browse Source 
As @lhecker noted in the #16172 review, `UpdateTerminalSettings` is
wacky. We can just pass the cache in at the start, then reset it and
reuse it in `UpdateSettings`. One fewer `try_as`!
This commit is contained in:
Mike Griese
2024-04-05 07:45:21 -07:00
committed by GitHub
parent 67ae9f6c3e
commit 1a6ba43dd2
11 changed files with 49 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
src/cascadia/TerminalApp/Pane.cpp
View File

@@ -1280,21 +1280,11 @@ void Pane::_FocusFirstChild()
}
}
void Pane::UpdateSettings(const CascadiaSettings& settings, const winrt::TerminalApp::TerminalSettingsCache& cache)
void Pane::UpdateSettings(const CascadiaSettings& settings)
{
if (_content)
{
// We need to do a bit more work here for terminal
// panes. They need to know about the profile that was used for
// them, and about the focused/unfocused settings.
if (const auto& terminalPaneContent{ _content.try_as<TerminalPaneContent>() })
{
terminalPaneContent.UpdateTerminalSettings(cache);
}
else
{
_content.UpdateSettings(settings);
}
_content.UpdateSettings(settings);
}
}

2
src/cascadia/TerminalApp/Pane.h
View File

@@ -108,7 +108,7 @@ public:
BuildStartupState BuildStartupActions(uint32_t currentId, uint32_t nextId, winrt::TerminalApp::BuildStartupKind kind);
winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetTerminalArgsForPane(winrt::TerminalApp::BuildStartupKind kind) const;
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const winrt::TerminalApp::TerminalSettingsCache& cache);
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
std::shared_ptr<Pane> NavigateDirection(const std::shared_ptr<Pane> sourcePane,
const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,

17
src/cascadia/TerminalApp/TerminalPage.cpp
View File

@@ -65,7 +65,6 @@ namespace winrt::TerminalApp::implementation
_WindowProperties{ std::move(properties) }
{
InitializeComponent();
_WindowProperties.PropertyChanged({ get_weak(), &TerminalPage::_windowPropertyChanged });
}
@@ -110,7 +109,11 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI)
{
assert(Dispatcher().HasThreadAccess());
if (_settings == nullptr)
{
// Create this only on the first time we load the settings.
_terminalSettingsCache = TerminalApp::TerminalSettingsCache{ settings, *_bindings };
}
_settings = settings;
// Make sure to call SetCommands before _RefreshUIForSettingsReload.
@@ -3104,7 +3107,7 @@ namespace winrt::TerminalApp::implementation
// serialize the actual profile's GUID along with the content guid.
const auto& profile = _settings.GetProfileForArgs(newTerminalArgs);
const auto control = _AttachControlToContent(newTerminalArgs.ContentId());
auto paneContent{ winrt::make<TerminalPaneContent>(profile, control) };
auto paneContent{ winrt::make<TerminalPaneContent>(profile, _terminalSettingsCache, control) };
return std::make_shared<Pane>(paneContent);
}
@@ -3161,14 +3164,14 @@ namespace winrt::TerminalApp::implementation
const auto control = _CreateNewControlAndContent(controlSettings, connection);
auto paneContent{ winrt::make<TerminalPaneContent>(profile, control) };
auto paneContent{ winrt::make<TerminalPaneContent>(profile, _terminalSettingsCache, control) };
auto resultPane = std::make_shared<Pane>(paneContent);
if (debugConnection) // this will only be set if global debugging is on and tap is active
{
auto newControl = _CreateNewControlAndContent(controlSettings, debugConnection);
// Split (auto) with the debug tap.
auto debugContent{ winrt::make<TerminalPaneContent>(profile, newControl) };
auto debugContent{ winrt::make<TerminalPaneContent>(profile, _terminalSettingsCache, newControl) };
auto debugPane = std::make_shared<Pane>(debugContent);
// Since we're doing this split directly on the pane (instead of going through TerminalTab,
@@ -3281,14 +3284,14 @@ namespace winrt::TerminalApp::implementation
// updating terminal panes, so that we don't have to build a _new_
// TerminalSettings for every profile we update - we can just look them
// up the previous ones we built.
_terminalSettingsCache = TerminalApp::TerminalSettingsCache{ _settings, *_bindings };
_terminalSettingsCache.Reset(_settings, *_bindings);
for (const auto& tab : _tabs)
{
if (auto terminalTab{ _GetTerminalTabImpl(tab) })
{
// Let the tab know that there are new settings. It's up to each content to decide what to do with them.
terminalTab->UpdateSettings(_settings, _terminalSettingsCache);
terminalTab->UpdateSettings(_settings);
// Update the icon of the tab for the currently focused profile in that tab.
// Only do this for TerminalTabs. Other types of tabs won't have multiple panes

12
src/cascadia/TerminalApp/TerminalPaneContent.cpp
View File

@@ -20,8 +20,10 @@ using namespace winrt::Microsoft::Terminal::TerminalConnection;
namespace winrt::TerminalApp::implementation
{
TerminalPaneContent::TerminalPaneContent(const winrt::Microsoft::Terminal::Settings::Model::Profile& profile,
const TerminalApp::TerminalSettingsCache& cache,
const winrt::Microsoft::Terminal::Control::TermControl& control) :
_control{ control },
_cache{ cache },
_profile{ profile }
{
_setupControlEvents();
@@ -340,15 +342,7 @@ namespace winrt::TerminalApp::implementation
void TerminalPaneContent::UpdateSettings(const CascadiaSettings& /*settings*/)
{
// Do nothing. We'll later be updated manually by
// UpdateTerminalSettings, which we need for profile and
// focused/unfocused settings.
assert(false); // If you hit this, you done goofed.
}
void TerminalPaneContent::UpdateTerminalSettings(const TerminalApp::TerminalSettingsCache& cache)
{
if (const auto& settings{ cache.TryLookup(_profile) })
if (const auto& settings{ _cache.TryLookup(_profile) })
{
_control.UpdateControlSettings(settings.DefaultSettings(), settings.UnfocusedSettings());
}

3
src/cascadia/TerminalApp/TerminalPaneContent.h
View File

@@ -19,6 +19,7 @@ namespace winrt::TerminalApp::implementation
struct TerminalPaneContent : TerminalPaneContentT<TerminalPaneContent>
{
TerminalPaneContent(const winrt::Microsoft::Terminal::Settings::Model::Profile& profile,
const TerminalApp::TerminalSettingsCache& cache,
const winrt::Microsoft::Terminal::Control::TermControl& control);
winrt::Windows::UI::Xaml::FrameworkElement GetRoot();
@@ -30,7 +31,6 @@ namespace winrt::TerminalApp::implementation
winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetNewTerminalArgs(BuildStartupKind kind) const;
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
void UpdateTerminalSettings(const TerminalApp::TerminalSettingsCache& cache);
void MarkAsDefterm();
@@ -64,6 +64,7 @@ namespace winrt::TerminalApp::implementation
winrt::Microsoft::Terminal::Control::TermControl _control{ nullptr };
winrt::Microsoft::Terminal::TerminalConnection::ConnectionState _connectionState{ winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::NotConnected };
winrt::Microsoft::Terminal::Settings::Model::Profile _profile{ nullptr };
TerminalApp::TerminalSettingsCache _cache{ nullptr };
bool _isDefTermSession{ false };
winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr };

2
src/cascadia/TerminalApp/TerminalPaneContent.idl
View File

@@ -10,8 +10,6 @@ namespace TerminalApp
{
Microsoft.Terminal.Control.TermControl GetTermControl();
void UpdateTerminalSettings(TerminalSettingsCache cache);
void MarkAsDefterm();
Microsoft.Terminal.Settings.Model.Profile GetProfile();

42
src/cascadia/TerminalApp/TerminalSettingsCache.cpp
View File

@@ -14,25 +14,9 @@ namespace winrt
namespace winrt::TerminalApp::implementation
{
TerminalSettingsCache::TerminalSettingsCache(const MTSM::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings) :
_settings{ settings },
_bindings{ bindings }
TerminalSettingsCache::TerminalSettingsCache(const MTSM::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings)
{
// Mapping by GUID isn't _excellent_ because the defaults profile doesn't have a stable GUID; however,
// when we stabilize its guid this will become fully safe.
const auto profileDefaults{ _settings.ProfileDefaults() };
const auto allProfiles{ _settings.AllProfiles() };
profileGuidSettingsMap.reserve(allProfiles.Size() + 1);
// Include the Defaults profile for consideration
profileGuidSettingsMap.insert_or_assign(profileDefaults.Guid(), std::pair{ profileDefaults, nullptr });
for (const auto& newProfile : allProfiles)
{
// Avoid creating a TerminalSettings right now. They're not totally cheap, and we suspect that users with many
// panes may not be using all of their profiles at the same time. Lazy evaluation is king!
profileGuidSettingsMap.insert_or_assign(newProfile.Guid(), std::pair{ newProfile, nullptr });
}
Reset(settings, bindings);
}
MTSM::TerminalSettingsCreateResult TerminalSettingsCache::TryLookup(const MTSM::Profile& profile)
@@ -54,4 +38,26 @@ namespace winrt::TerminalApp::implementation
return nullptr;
}
void TerminalSettingsCache::Reset(const MTSM::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings)
{
_settings = settings;
_bindings = bindings;
// Mapping by GUID isn't _excellent_ because the defaults profile doesn't have a stable GUID; however,
// when we stabilize its guid this will become fully safe.
const auto profileDefaults{ _settings.ProfileDefaults() };
const auto allProfiles{ _settings.AllProfiles() };
profileGuidSettingsMap.clear();
profileGuidSettingsMap.reserve(allProfiles.Size() + 1);
// Include the Defaults profile for consideration
profileGuidSettingsMap.insert_or_assign(profileDefaults.Guid(), std::pair{ profileDefaults, nullptr });
for (const auto& newProfile : allProfiles)
{
// Avoid creating a TerminalSettings right now. They're not totally cheap, and we suspect that users with many
// panes may not be using all of their profiles at the same time. Lazy evaluation is king!
profileGuidSettingsMap.insert_or_assign(newProfile.Guid(), std::pair{ newProfile, nullptr });
}
}
}

1
src/cascadia/TerminalApp/TerminalSettingsCache.h
View File

@@ -23,6 +23,7 @@ namespace winrt::TerminalApp::implementation
public:
TerminalSettingsCache(const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings);
Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult TryLookup(const Microsoft::Terminal::Settings::Model::Profile& profile);
void Reset(const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings);
private:
Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };

1
src/cascadia/TerminalApp/TerminalSettingsCache.idl
View File

@@ -8,5 +8,6 @@ namespace TerminalApp
{
TerminalSettingsCache(Microsoft.Terminal.Settings.Model.CascadiaSettings settings, AppKeyBindings bindings);
Microsoft.Terminal.Settings.Model.TerminalSettingsCreateResult TryLookup(Microsoft.Terminal.Settings.Model.Profile profile);
void Reset(Microsoft.Terminal.Settings.Model.CascadiaSettings settings, AppKeyBindings bindings);
}
}

8
src/cascadia/TerminalApp/TerminalTab.cpp
View File

@@ -267,7 +267,7 @@ namespace winrt::TerminalApp::implementation
// of the settings that apply to all tabs.
// Return Value:
// - <none>
void TerminalTab::UpdateSettings(const CascadiaSettings& settings, const TerminalApp::TerminalSettingsCache& cache)
void TerminalTab::UpdateSettings(const CascadiaSettings& settings)
{
ASSERT_UI_THREAD();
@@ -276,7 +276,7 @@ namespace winrt::TerminalApp::implementation
// Update the settings on all our panes.
_rootPane->WalkTree([&](auto pane) {
pane->UpdateSettings(settings, cache);
pane->UpdateSettings(settings);
return false;
});
}
@@ -390,7 +390,7 @@ namespace winrt::TerminalApp::implementation
return RS_(L"MultiplePanes");
}
const auto activeContent = GetActiveContent();
return activeContent ? activeContent.Title() : winrt::hstring{ L"" };
return activeContent ? activeContent.Title() : winrt::hstring{};
}
// Method Description:
@@ -450,7 +450,7 @@ namespace winrt::TerminalApp::implementation
// HORRIBLE
//
// Workaround till we know how we actually want to handle state
// restoring other kinda of panes. If this is a settings tab, just
// restoring other kinds of panes. If this is a settings tab, just
// restore it as a settings tab. Don't bother recreating terminal args
// for every pane.
//

2
src/cascadia/TerminalApp/TerminalTab.h
View File

@@ -58,7 +58,7 @@ namespace winrt::TerminalApp::implementation
bool SwapPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);
bool FocusPane(const uint32_t id);
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const TerminalApp::TerminalSettingsCache& cache);
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
void UpdateTitle();
void Shutdown() override;