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.

Sign up for GitHub

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

Jump to bottom

Fix crash on saving the settings twice #10148

Merged
2 commits merged into from
May 21, 2021
Merged

Fix crash on saving the settings twice #10148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed2b112
Fixes #10003
zadjii-msft May 21, 2021
66df3d9
Update src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
zadjii-msft May 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "CascadiaSettings.g.cpp"

using namespace ::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::Terminal;
using namespace winrt::Microsoft::Terminal::Control;
using namespace winrt::Microsoft::Terminal::Settings::Model::implementation;
using namespace winrt::Windows::Foundation::Collections;
Expand Down Expand Up @@ -1137,7 +1138,8 @@ winrt::hstring CascadiaSettings::ApplicationVersion()
}

// Method Description:
// - Forces a refresh of all default terminal state
// - Forces a refresh of all default terminal state. This hits the registry to
// read off the disk, so best to not do it on the UI thread.
// Arguments:
// - <none>
// Return Value:
Expand Down Expand Up @@ -1187,18 +1189,23 @@ bool CascadiaSettings::IsDefaultTerminalAvailable() noexcept
// - <none>
// Return Value:
// - an iterable collection of all available terminals that could be the default.
IObservableVector<winrt::Microsoft::Terminal::Settings::Model::DefaultTerminal> CascadiaSettings::DefaultTerminals() const noexcept
IObservableVector<Settings::Model::DefaultTerminal> CascadiaSettings::DefaultTerminals() const noexcept
{
return _defaultTerminals;
}

// Method Description:
// - Returns the currently selected default terminal application
// - Returns the currently selected default terminal application.
// - DANGER! This will be null unless you've called
// CascadiaSettings::RefreshDefaultTerminals. At the time of this comment (May

// 2021), only the Launch page in the settings UI calls that method, so this
// value is unset unless you've navigated to that page.
// Arguments:
// - <none>
// Return Value:
// - the selected default terminal application
winrt::Microsoft::Terminal::Settings::Model::DefaultTerminal CascadiaSettings::CurrentDefaultTerminal() const noexcept
Settings::Model::DefaultTerminal CascadiaSettings::CurrentDefaultTerminal() const noexcept
{
return _currentDefaultTerminal;
}
Expand All @@ -1209,7 +1216,7 @@ winrt::Microsoft::Terminal::Settings::Model::DefaultTerminal CascadiaSettings::C
// - terminal - Terminal from `DefaultTerminals` list to set as default
// Return Value:
// - <none>
void CascadiaSettings::CurrentDefaultTerminal(winrt::Microsoft::Terminal::Settings::Model::DefaultTerminal terminal)
void CascadiaSettings::CurrentDefaultTerminal(Settings::Model::DefaultTerminal terminal)
{
_currentDefaultTerminal = terminal;
}
10 changes: 9 additions & 1 deletion src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,15 @@ void CascadiaSettings::WriteSettingsToDisk() const
_WriteSettings(styledString, settingsPath);

// Persists the default terminal choice
Model::DefaultTerminal::Current(_currentDefaultTerminal);
//
// GH#10003 - Only do this if _currentDefaultTerminal was actually
// initialized. It's only initialized when Launch.cpp calls
// `CascadiaSettings::RefreshDefaultTerminals`. We really don't need it
// otherwise.
if (_currentDefaultTerminal)
{
Model::DefaultTerminal::Current(_currentDefaultTerminal);
}
}

// Method Description:
Expand Down