PREREQ: Fix default terminal setting dropdown (#11430)
WinUI/XAML requires the `SelectedItem` to be member of the list of
`ItemsSource`. `CascadiaSettings::DefaultTerminals()` is such an `ItemsSource`
and is called every time the launch settings page is visited.
It calls `DefaultTerminal::Available()` which in turn calls `Refresh()`.
While the `SelectedItem` was cached in `CascadiaSettings`, the value of
`DefaultTerminals()` wasn't. Thus every time the page was visited, it refreshed
the `ItemsSource` list without invalidating the current `SelectedItem`.
This commit prevents such accidental mishaps from occurring in the future,
by moving the responsibility of caching solely to the `CascadiaSettings` class.
* [x] Closes #11424
* [x] I work here
* [x] Tests added/passed
* Navigating between SUI pages maintains the current dropdown selection ✔️
* Saving the settings saves the correct terminal at `HKCU:\Console\%%Startup` ✔️
(cherry picked from commit 35ce8cc858
)
This commit is contained in:
parent
603122f247
commit
39d16ba94c
3
.github/actions/spelling/expect/expect.txt
vendored
3
.github/actions/spelling/expect/expect.txt
vendored
|
@ -189,6 +189,7 @@ cacafire
|
|||
callee
|
||||
capslock
|
||||
CARETBLINKINGENABLED
|
||||
carlos
|
||||
CARRIAGERETURN
|
||||
cascadia
|
||||
cassert
|
||||
|
@ -2716,6 +2717,7 @@ wixproj
|
|||
wline
|
||||
wlinestream
|
||||
wmain
|
||||
wmemory
|
||||
WMSZ
|
||||
wnd
|
||||
WNDALLOC
|
||||
|
@ -2842,6 +2844,7 @@ YSize
|
|||
YSubstantial
|
||||
YVIRTUALSCREEN
|
||||
YWalk
|
||||
zamora
|
||||
ZCmd
|
||||
ZCtrl
|
||||
zsh
|
||||
|
|
|
@ -5,6 +5,8 @@
|
|||
#include "CascadiaSettings.h"
|
||||
#include "CascadiaSettings.g.cpp"
|
||||
|
||||
#include "DefaultTerminal.h"
|
||||
|
||||
#include <LibraryResources.h>
|
||||
|
||||
#include "AzureCloudShellGenerator.h"
|
||||
|
@ -1239,24 +1241,21 @@ bool CascadiaSettings::IsDefaultTerminalAvailable() noexcept
|
|||
// - <none>
|
||||
// Return Value:
|
||||
// - an iterable collection of all available terminals that could be the default.
|
||||
IObservableVector<Settings::Model::DefaultTerminal> CascadiaSettings::DefaultTerminals() const noexcept
|
||||
IObservableVector<Settings::Model::DefaultTerminal> CascadiaSettings::DefaultTerminals() noexcept
|
||||
{
|
||||
_refreshDefaultTerminals();
|
||||
return _defaultTerminals;
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - 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
|
||||
Settings::Model::DefaultTerminal CascadiaSettings::CurrentDefaultTerminal() const noexcept
|
||||
{
|
||||
_refreshDefaultTerminals();
|
||||
return _currentDefaultTerminal;
|
||||
}
|
||||
|
||||
|
@ -1270,3 +1269,19 @@ void CascadiaSettings::CurrentDefaultTerminal(Settings::Model::DefaultTerminal t
|
|||
{
|
||||
_currentDefaultTerminal = terminal;
|
||||
}
|
||||
|
||||
// This function is implicitly called by DefaultTerminals/CurrentDefaultTerminal().
|
||||
// It reloads the selection of available, installed terminals and caches them.
|
||||
// WinUI requires us that the `SelectedItem` of a collection is member of the list given to `ItemsSource`.
|
||||
// It's thus important that _currentDefaultTerminal is a member of _defaultTerminals.
|
||||
// Right now this is implicitly the case thanks to DefaultTerminal::Available(),
|
||||
// but in the future it might be worthwhile to change the code to use list indices instead.
|
||||
void CascadiaSettings::_refreshDefaultTerminals()
|
||||
{
|
||||
if (!_defaultTerminals)
|
||||
{
|
||||
auto [defaultTerminals, defaultTerminal] = DefaultTerminal::Available();
|
||||
_defaultTerminals = winrt::single_threaded_observable_vector(std::move(defaultTerminals));
|
||||
_currentDefaultTerminal = std::move(defaultTerminal);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -102,9 +102,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
|||
void RefreshDefaultTerminals();
|
||||
|
||||
static bool IsDefaultTerminalAvailable() noexcept;
|
||||
Windows::Foundation::Collections::IObservableVector<Model::DefaultTerminal> DefaultTerminals() const noexcept;
|
||||
Model::DefaultTerminal CurrentDefaultTerminal() const noexcept;
|
||||
void CurrentDefaultTerminal(Model::DefaultTerminal terminal);
|
||||
winrt::Windows::Foundation::Collections::IObservableVector<Model::DefaultTerminal> DefaultTerminals() noexcept;
|
||||
Model::DefaultTerminal CurrentDefaultTerminal() noexcept;
|
||||
void CurrentDefaultTerminal(const Model::DefaultTerminal& terminal);
|
||||
|
||||
private:
|
||||
com_ptr<GlobalAppSettings> _globals;
|
||||
|
@ -114,8 +114,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
|||
Windows::Foundation::IReference<SettingsLoadErrors> _loadError;
|
||||
hstring _deserializationErrorMessage;
|
||||
|
||||
Windows::Foundation::Collections::IObservableVector<Model::DefaultTerminal> _defaultTerminals;
|
||||
Model::DefaultTerminal _currentDefaultTerminal;
|
||||
void _refreshDefaultTerminals();
|
||||
|
||||
std::vector<std::unique_ptr<::Microsoft::Terminal::Settings::Model::IDynamicProfileGenerator>> _profileGenerators;
|
||||
|
||||
|
@ -177,6 +176,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
|||
friend class SettingsModelLocalTests::KeyBindingsTests;
|
||||
friend class TerminalAppUnitTests::DynamicProfileTests;
|
||||
friend class TerminalAppUnitTests::JsonTests;
|
||||
|
||||
// defterm
|
||||
winrt::Windows::Foundation::Collections::IObservableVector<Model::DefaultTerminal> _defaultTerminals{ nullptr };
|
||||
Model::DefaultTerminal _currentDefaultTerminal{ nullptr };
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
@ -16,6 +16,7 @@
|
|||
// "Generated Files" directory.
|
||||
|
||||
#include "ApplicationState.h"
|
||||
#include "DefaultTerminal.h"
|
||||
#include "FileUtils.h"
|
||||
|
||||
using namespace winrt::Microsoft::Terminal::Settings::Model::implementation;
|
||||
|
|
|
@ -10,85 +10,70 @@
|
|||
using namespace winrt::Microsoft::Terminal::Settings;
|
||||
using namespace winrt::Microsoft::Terminal::Settings::Model::implementation;
|
||||
|
||||
winrt::Windows::Foundation::Collections::IVector<Model::DefaultTerminal> DefaultTerminal::_available = winrt::single_threaded_vector<Model::DefaultTerminal>();
|
||||
Model::DefaultTerminal DefaultTerminal::_current = nullptr;
|
||||
|
||||
DefaultTerminal::DefaultTerminal(DelegationConfig::DelegationPackage pkg) :
|
||||
_pkg(pkg)
|
||||
DefaultTerminal::DefaultTerminal(DelegationConfig::DelegationPackage&& pkg) :
|
||||
_pkg{ pkg }
|
||||
{
|
||||
}
|
||||
|
||||
winrt::hstring DefaultTerminal::Name() const
|
||||
{
|
||||
static const std::wstring def{ RS_(L"InboxWindowsConsoleName") };
|
||||
return _pkg.terminal.name.empty() ? winrt::hstring{ def } : winrt::hstring{ _pkg.terminal.name };
|
||||
return _pkg.terminal.name.empty() ? winrt::hstring{ RS_(L"InboxWindowsConsoleName") } : winrt::hstring{ _pkg.terminal.name };
|
||||
}
|
||||
|
||||
winrt::hstring DefaultTerminal::Version() const
|
||||
{
|
||||
// If there's no version information... return empty string instead.
|
||||
if (DelegationConfig::PkgVersion{} == _pkg.terminal.version)
|
||||
const auto& version = _pkg.terminal.version;
|
||||
if (DelegationConfig::PkgVersion{} == version)
|
||||
{
|
||||
return winrt::hstring{};
|
||||
}
|
||||
|
||||
const auto name = fmt::format(L"{}.{}.{}.{}", _pkg.terminal.version.major, _pkg.terminal.version.minor, _pkg.terminal.version.build, _pkg.terminal.version.revision);
|
||||
return winrt::hstring{ name };
|
||||
fmt::wmemory_buffer buffer;
|
||||
fmt::format_to(buffer, L"{}.{}.{}.{}", version.major, version.minor, version.build, version.revision);
|
||||
return winrt::hstring{ buffer.data(), gsl::narrow_cast<winrt::hstring::size_type>(buffer.size()) };
|
||||
}
|
||||
|
||||
winrt::hstring DefaultTerminal::Author() const
|
||||
{
|
||||
static const std::wstring def{ RS_(L"InboxWindowsConsoleAuthor") };
|
||||
return _pkg.terminal.author.empty() ? winrt::hstring{ def } : winrt::hstring{ _pkg.terminal.author };
|
||||
return _pkg.terminal.author.empty() ? winrt::hstring{ RS_(L"InboxWindowsConsoleAuthor") } : winrt::hstring{ _pkg.terminal.author };
|
||||
}
|
||||
|
||||
winrt::hstring DefaultTerminal::Icon() const
|
||||
{
|
||||
static const std::wstring_view def{ L"\uE756" };
|
||||
return _pkg.terminal.logo.empty() ? winrt::hstring{ def } : winrt::hstring{ _pkg.terminal.logo };
|
||||
return _pkg.terminal.logo.empty() ? winrt::hstring{ L"\uE756" } : winrt::hstring{ _pkg.terminal.logo };
|
||||
}
|
||||
|
||||
void DefaultTerminal::Refresh()
|
||||
std::pair<std::vector<Model::DefaultTerminal>, Model::DefaultTerminal> DefaultTerminal::Available()
|
||||
{
|
||||
// The potential of returning nullptr for defaultTerminal feels weird, but XAML can
|
||||
// handle that appropriately and will select nothing as current in the dropdown.
|
||||
std::vector<Model::DefaultTerminal> defaultTerminals;
|
||||
Model::DefaultTerminal defaultTerminal{ nullptr };
|
||||
|
||||
std::vector<DelegationConfig::DelegationPackage> allPackages;
|
||||
DelegationConfig::DelegationPackage currentPackage;
|
||||
|
||||
LOG_IF_FAILED(DelegationConfig::s_GetAvailablePackages(allPackages, currentPackage));
|
||||
|
||||
_available.Clear();
|
||||
for (const auto& pkg : allPackages)
|
||||
for (auto& pkg : allPackages)
|
||||
{
|
||||
auto p = winrt::make<DefaultTerminal>(pkg);
|
||||
// Be a bit careful here: We're moving pkg into the constructor.
|
||||
// Afterwards it'll be invalid, so we need to cache isCurrent.
|
||||
const auto isCurrent = pkg == currentPackage;
|
||||
auto p = winrt::make<DefaultTerminal>(std::move(pkg));
|
||||
|
||||
_available.Append(p);
|
||||
|
||||
if (pkg == currentPackage)
|
||||
if (isCurrent)
|
||||
{
|
||||
_current = p;
|
||||
defaultTerminal = p;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
winrt::Windows::Foundation::Collections::IVectorView<Model::DefaultTerminal> DefaultTerminal::Available()
|
||||
{
|
||||
Refresh();
|
||||
return _available.GetView();
|
||||
}
|
||||
|
||||
Model::DefaultTerminal DefaultTerminal::Current()
|
||||
{
|
||||
if (!_current)
|
||||
{
|
||||
Refresh();
|
||||
defaultTerminals.emplace_back(std::move(p));
|
||||
}
|
||||
|
||||
// The potential of returning nullptr feels weird, but XAML can handle that appropriately
|
||||
// and will select nothing as current in the dropdown.
|
||||
return _current;
|
||||
return { std::move(defaultTerminals), std::move(defaultTerminal) };
|
||||
}
|
||||
|
||||
void DefaultTerminal::Current(const Model::DefaultTerminal& term)
|
||||
{
|
||||
THROW_IF_FAILED(DelegationConfig::s_SetDefaultByPackage(winrt::get_self<DefaultTerminal>(term)->_pkg, true));
|
||||
_current = term;
|
||||
}
|
||||
|
|
|
@ -19,7 +19,6 @@ Author(s):
|
|||
#pragma once
|
||||
|
||||
#include "DefaultTerminal.g.h"
|
||||
#include "../inc/cppwinrt_utils.h"
|
||||
|
||||
#include "../../propslib/DelegationConfig.hpp"
|
||||
|
||||
|
@ -27,26 +26,17 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
|||
{
|
||||
struct DefaultTerminal : public DefaultTerminalT<DefaultTerminal>
|
||||
{
|
||||
DefaultTerminal(DelegationConfig::DelegationPackage pkg);
|
||||
explicit DefaultTerminal(DelegationConfig::DelegationPackage&& pkg);
|
||||
|
||||
hstring Name() const;
|
||||
hstring Author() const;
|
||||
hstring Version() const;
|
||||
hstring Icon() const;
|
||||
|
||||
static void Refresh();
|
||||
static Windows::Foundation::Collections::IVectorView<Model::DefaultTerminal> Available();
|
||||
static Model::DefaultTerminal Current();
|
||||
static std::pair<std::vector<Model::DefaultTerminal>, Model::DefaultTerminal> Available();
|
||||
static void Current(const Model::DefaultTerminal& term);
|
||||
|
||||
private:
|
||||
DelegationConfig::DelegationPackage _pkg;
|
||||
static Windows::Foundation::Collections::IVector<Model::DefaultTerminal> _available;
|
||||
static Model::DefaultTerminal _current;
|
||||
};
|
||||
}
|
||||
|
||||
namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
|
||||
{
|
||||
BASIC_FACTORY(DefaultTerminal);
|
||||
}
|
||||
|
|
|
@ -9,8 +9,5 @@ namespace Microsoft.Terminal.Settings.Model
|
|||
String Author { get; };
|
||||
String Version { get; };
|
||||
String Icon { get; };
|
||||
|
||||
static DefaultTerminal Current;
|
||||
static Windows.Foundation.Collections.IVectorView<DefaultTerminal> Available { get; };
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue