mirror of
https://github.com/microsoft/terminal.git
synced 2026-07-08 17:46:05 +00:00
Iterate on JsonManager (needs review)
This commit is contained in:
@@ -1318,7 +1318,7 @@ namespace winrt::TerminalApp::implementation
|
||||
keyChord = KeyChordSerialization::FromString(winrt::to_hstring(realArgs.KeyChord()));
|
||||
}
|
||||
_settings.GlobalSettings().ActionMap().AddSendInputAction(realArgs.Name(), commandLine, keyChord);
|
||||
_settings.WriteSettingsToDisk();
|
||||
JsonManager::WriteSettings(_settings);
|
||||
ActionSaved(commandLine, realArgs.Name(), realArgs.KeyChord());
|
||||
}
|
||||
catch (const winrt::hresult_error& ex)
|
||||
|
||||
@@ -951,7 +951,9 @@ namespace winrt::TerminalApp::implementation
|
||||
if (result == ContentDialogResult::Primary && checkbox.IsChecked().Value())
|
||||
{
|
||||
_settings.GlobalSettings().ConfirmOnClose(ConfirmOnClose::Never);
|
||||
_settings.WriteSettingsToDisk();
|
||||
// Persisted via auto-save: the setter fires the write sink, which
|
||||
// updates the JsonManager baseline and avoids a redundant write +
|
||||
// spurious reload (GH#12424). A pending write is flushed on exit.
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -45,7 +45,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
|
||||
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
|
||||
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
|
||||
|
||||
_settings.ResetToDefaultSettings();
|
||||
JsonManager::ResetToDefaultSettings();
|
||||
}
|
||||
|
||||
Compatibility::Compatibility()
|
||||
|
||||
@@ -861,9 +861,18 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
|
||||
void MainPage::SaveButton_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/)
|
||||
{
|
||||
_settingsClone.LogSettingChanges(false);
|
||||
if (!_settingsClone.WriteSettingsToDisk())
|
||||
if (JsonManager::WriteSettings(_settingsClone).empty())
|
||||
{
|
||||
ShowLoadWarningsDialog.raise(*this, _settingsClone.Warnings());
|
||||
// The write failed. Surface the clone's existing warnings plus an
|
||||
// explicit "failed to write" warning (the old WriteSettingsToDisk
|
||||
// appended this internally).
|
||||
auto warnings{ winrt::single_threaded_vector<SettingsLoadWarnings>() };
|
||||
for (auto&& w : _settingsClone.Warnings())
|
||||
{
|
||||
warnings.Append(w);
|
||||
}
|
||||
warnings.Append(SettingsLoadWarnings::FailedToWriteToSettings);
|
||||
ShowLoadWarningsDialog.raise(*this, warnings.GetView());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -143,6 +143,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
|
||||
struct CascadiaSettings : CascadiaSettingsT<CascadiaSettings>
|
||||
{
|
||||
// JsonManager performs settings.json persistence (GH#12424) and reads the
|
||||
// cached _currentDefaultTerminal directly to persist the default-terminal
|
||||
// choice without triggering an expensive defterm refresh.
|
||||
friend struct JsonManager;
|
||||
|
||||
public:
|
||||
static Model::CascadiaSettings LoadDefaults();
|
||||
static Model::CascadiaSettings LoadAll();
|
||||
@@ -168,8 +173,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
Model::ActionMap ActionMap() const noexcept;
|
||||
winrt::Windows::Foundation::Collections::IVectorView<Model::ExtensionPackage> Extensions();
|
||||
void ResetApplicationState() const;
|
||||
void ResetToDefaultSettings();
|
||||
bool WriteSettingsToDisk();
|
||||
void SetWriteHandler(std::function<void()> handler);
|
||||
|
||||
Json::Value ToJson() const;
|
||||
@@ -203,16 +206,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
private:
|
||||
static const std::filesystem::path& _settingsPath();
|
||||
static const std::filesystem::path& _releaseSettingsPath();
|
||||
static winrt::hstring _calculateHash(std::string_view settings, const FILETIME& lastWriteTime);
|
||||
|
||||
winrt::com_ptr<implementation::Profile> _createNewProfile(const std::wstring_view& name) const;
|
||||
Model::Profile _getProfileForCommandLine(const winrt::hstring& commandLine) const;
|
||||
void _refreshDefaultTerminals();
|
||||
void _writeSettingsToDisk(std::string_view contents);
|
||||
|
||||
// Auto-save (GH#12424): the shared "request auto-save" sink installed on
|
||||
// every inheritable object in this live tree. Empty until SetWriteHandler.
|
||||
std::shared_ptr<std::function<void()>> _writeSink;
|
||||
// Auto-save (GH#12424): the shared "request auto-save" notifier installed
|
||||
// on every inheritable object in this live tree. Empty until SetWriteHandler.
|
||||
std::shared_ptr<implementation::SettingsWriteNotifier> _writeSink;
|
||||
void _installWriteSink();
|
||||
|
||||
void _resolveDefaultProfile() const;
|
||||
|
||||
@@ -30,8 +30,6 @@ namespace Microsoft.Terminal.Settings.Model
|
||||
|
||||
CascadiaSettings Copy();
|
||||
void ResetApplicationState();
|
||||
void ResetToDefaultSettings();
|
||||
Boolean WriteSettingsToDisk();
|
||||
void LogSettingChanges(Boolean isJsonLoad);
|
||||
|
||||
String Hash { get; };
|
||||
|
||||
@@ -20,6 +20,7 @@
|
||||
#include "ApplicationState.h"
|
||||
#include "DefaultTerminal.h"
|
||||
#include "FileUtils.h"
|
||||
#include "JsonManager.h"
|
||||
|
||||
#include "ProfileEntry.h"
|
||||
#include "FolderEntry.h"
|
||||
@@ -1284,13 +1285,23 @@ try
|
||||
// settings string back to the file.
|
||||
if (mustWriteToDisk)
|
||||
{
|
||||
settings->WriteSettingsToDisk();
|
||||
// JsonManager::WriteSettings (GH#12424) force-writes and returns the new
|
||||
// hash (empty on failure); capture it so reload-detection has an accurate
|
||||
// baseline.
|
||||
const auto hash = JsonManager::WriteSettings(settings.as<Model::CascadiaSettings>());
|
||||
if (hash.empty())
|
||||
{
|
||||
settings->_warnings.Append(SettingsLoadWarnings::FailedToWriteToSettings);
|
||||
}
|
||||
else
|
||||
{
|
||||
settings->_hash = hash;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
// lastWriteTime is only valid if mustWriteToDisk is false.
|
||||
// Additionally WriteSettingsToDisk() updates the _hash for us already.
|
||||
settings->_hash = _calculateHash(settingsString, lastWriteTime);
|
||||
settings->_hash = CalculateSettingsHash(settingsString, lastWriteTime);
|
||||
}
|
||||
|
||||
settings->_researchOnLoad();
|
||||
@@ -1496,7 +1507,7 @@ CascadiaSettings::CascadiaSettings(SettingsLoader&& loader) :
|
||||
// and the defaults fallback never auto-save.
|
||||
void CascadiaSettings::_installWriteSink()
|
||||
{
|
||||
_writeSink = std::make_shared<std::function<void()>>();
|
||||
_writeSink = std::make_shared<implementation::SettingsWriteNotifier>();
|
||||
|
||||
const auto installForProfile = [&](implementation::Profile* prof) {
|
||||
if (!prof)
|
||||
@@ -1532,13 +1543,22 @@ void CascadiaSettings::_installWriteSink()
|
||||
{
|
||||
installForProfile(winrt::get_self<implementation::Profile>(p));
|
||||
}
|
||||
|
||||
// TODO CARLOS: Known gaps (GH#12424): ColorScheme and Theme are not IInheritable / not
|
||||
// JSON-backed, so per-property in-place edits (e.g. SetColorTableEntry,
|
||||
// individual theme colors) do NOT trigger auto-save. ActionMap is IInheritable
|
||||
// but not JSON-backed, so individual action edits don't either. Add/remove of
|
||||
// color schemes and themes IS covered via the GlobalAppSettings collection
|
||||
// mutators (AddColorScheme/RemoveColorScheme/DuplicateColorScheme/AddTheme).
|
||||
// Full per-property coverage waits for those types becoming JSON-backed and is
|
||||
// only required once the settings editor relies on auto-save.
|
||||
}
|
||||
|
||||
void CascadiaSettings::SetWriteHandler(std::function<void()> handler)
|
||||
{
|
||||
if (_writeSink)
|
||||
{
|
||||
*_writeSink = std::move(handler);
|
||||
_writeSink->SetHandler(std::move(handler));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1566,12 +1586,6 @@ const std::filesystem::path& CascadiaSettings::_releaseSettingsPath()
|
||||
return path;
|
||||
}
|
||||
|
||||
// Returns a has (approximately) uniquely identifying the settings.json contents on disk.
|
||||
winrt::hstring CascadiaSettings::_calculateHash(std::string_view settings, const FILETIME& lastWriteTime)
|
||||
{
|
||||
return CalculateSettingsHash(settings, lastWriteTime);
|
||||
}
|
||||
|
||||
// This returns something akin to %LOCALAPPDATA%\Packages\WindowsTerminalDev_8wekyb3d8bbwe\LocalState
|
||||
// just like SettingsPath(), but without the trailing \settings.json.
|
||||
winrt::hstring CascadiaSettings::SettingsDirectory()
|
||||
@@ -1628,58 +1642,6 @@ void CascadiaSettings::ResetApplicationState() const
|
||||
state.Flush();
|
||||
}
|
||||
|
||||
void CascadiaSettings::ResetToDefaultSettings()
|
||||
{
|
||||
ApplicationState::SharedInstance().Reset();
|
||||
_writeSettingsToDisk(LoadStringResource(IDR_USER_DEFAULTS));
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - Write the current state of CascadiaSettings to our settings file
|
||||
// - Create a backup file with the current contents, if one does not exist
|
||||
// - Persists the default terminal handler choice to the registry
|
||||
// Arguments:
|
||||
// - <none>
|
||||
// Return Value:
|
||||
// - <none>
|
||||
bool CascadiaSettings::WriteSettingsToDisk()
|
||||
{
|
||||
// write current settings to current settings file
|
||||
Json::StreamWriterBuilder wbuilder;
|
||||
wbuilder.settings_["enableYAMLCompatibility"] = true; // suppress spaces around colons
|
||||
wbuilder.settings_["indentation"] = " ";
|
||||
wbuilder.settings_["precision"] = 6; // prevent values like 1.1000000000000001
|
||||
|
||||
try
|
||||
{
|
||||
_writeSettingsToDisk(Json::writeString(wbuilder, ToJson()));
|
||||
}
|
||||
catch (...)
|
||||
{
|
||||
LOG_CAUGHT_EXCEPTION();
|
||||
_warnings.Append(SettingsLoadWarnings::FailedToWriteToSettings);
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
void CascadiaSettings::_writeSettingsToDisk(std::string_view contents)
|
||||
{
|
||||
FILETIME lastWriteTime{};
|
||||
// Auto-save (GH#12424): write atomically via a process-unique temp file and
|
||||
// keep a best-effort "settings.json.bak" backup of the prior contents.
|
||||
WriteSettingsFile(_settingsPath(), contents, /*makeBackup*/ true, &lastWriteTime);
|
||||
|
||||
_hash = _calculateHash(contents, lastWriteTime);
|
||||
|
||||
// Persists the default terminal choice
|
||||
// GH#10003 - Only do this if _currentDefaultTerminal was actually initialized.
|
||||
if (_currentDefaultTerminal)
|
||||
{
|
||||
DefaultTerminal::Current(_currentDefaultTerminal);
|
||||
}
|
||||
}
|
||||
|
||||
#ifndef NDEBUG
|
||||
[[maybe_unused]] static std::string _getDevPathToSchema()
|
||||
{
|
||||
|
||||
@@ -277,11 +277,16 @@ void GlobalAppSettings::LayerActionsFrom(const Json::Value& json, const OriginTa
|
||||
void GlobalAppSettings::AddColorScheme(const Model::ColorScheme& scheme)
|
||||
{
|
||||
_colorSchemes.Insert(scheme.Name(), scheme);
|
||||
// Coarse-grained auto-save coverage (GH#12424): color schemes aren't
|
||||
// IInheritable, so add/remove is notified here at the collection level. (No-op
|
||||
// until this tree is the live, bound tree.)
|
||||
_NotifyWriteSettings();
|
||||
}
|
||||
|
||||
void GlobalAppSettings::RemoveColorScheme(hstring schemeName)
|
||||
{
|
||||
_colorSchemes.TryRemove(schemeName);
|
||||
_NotifyWriteSettings();
|
||||
}
|
||||
|
||||
winrt::Microsoft::Terminal::Settings::Model::ColorScheme GlobalAppSettings::DuplicateColorScheme(const Model::ColorScheme& source)
|
||||
@@ -455,6 +460,9 @@ winrt::Microsoft::Terminal::Settings::Model::Theme GlobalAppSettings::CurrentThe
|
||||
void GlobalAppSettings::AddTheme(const Model::Theme& theme)
|
||||
{
|
||||
_themes.Insert(theme.Name(), theme);
|
||||
// Coarse-grained auto-save coverage (GH#12424): themes aren't IInheritable,
|
||||
// so add is notified here at the collection level. (No-op until bound.)
|
||||
_NotifyWriteSettings();
|
||||
}
|
||||
|
||||
winrt::Windows::Foundation::Collections::IMapView<winrt::hstring, winrt::Microsoft::Terminal::Settings::Model::Theme> GlobalAppSettings::Themes() noexcept
|
||||
|
||||
@@ -22,6 +22,33 @@ Author(s):
|
||||
|
||||
namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
{
|
||||
// A late-bound, shared "request auto-save" notifier (GH#12424). One instance
|
||||
// is created by CascadiaSettings and installed (by shared_ptr) on every
|
||||
// object in the *live* settings tree. When a setting is mutated, the object
|
||||
// calls Notify(); CascadiaSettings binds the actual handler after a
|
||||
// successful load via SetHandler(). Until a handler is bound, Notify() is a
|
||||
// no-op. The notifier is deliberately NOT propagated by Copy()/clone paths,
|
||||
// so editor clones (and the defaults fallback) never auto-save the user's
|
||||
// settings.json.
|
||||
struct SettingsWriteNotifier
|
||||
{
|
||||
void Notify() const
|
||||
{
|
||||
if (_handler)
|
||||
{
|
||||
_handler();
|
||||
}
|
||||
}
|
||||
|
||||
void SetHandler(std::function<void()> handler)
|
||||
{
|
||||
_handler = std::move(handler);
|
||||
}
|
||||
|
||||
private:
|
||||
std::function<void()> _handler;
|
||||
};
|
||||
|
||||
template<typename T>
|
||||
struct IInheritable
|
||||
{
|
||||
@@ -68,12 +95,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
}
|
||||
|
||||
// A shared "request auto-save" sink. The owning CascadiaSettings
|
||||
// creates one sink and installs it on every object in the *live* tree.
|
||||
// When a setting on this layer is mutated, _NotifyWriteSettings() invokes
|
||||
// the sink, which requests a debounced auto-save. The sink is intentionally
|
||||
// NOT copied by Copy()/clone paths, so editor clones (and the defaults
|
||||
// fallback) never auto-save the user's settings.json.
|
||||
using WriteSettingsSink = std::shared_ptr<std::function<void()>>;
|
||||
// creates one notifier and installs it on every object in the *live*
|
||||
// tree. When a setting on this layer is mutated, _NotifyWriteSettings()
|
||||
// invokes the notifier, which requests a debounced auto-save. The notifier
|
||||
// is intentionally NOT copied by Copy()/clone paths, so editor clones (and
|
||||
// the defaults fallback) never auto-save the user's settings.json.
|
||||
using WriteSettingsSink = std::shared_ptr<SettingsWriteNotifier>;
|
||||
void SetWriteSettingsSink(const WriteSettingsSink& sink)
|
||||
{
|
||||
_writeSettingsSink = sink;
|
||||
@@ -88,9 +115,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
// No-op until a sink is installed (i.e. only on the committed live tree).
|
||||
void _NotifyWriteSettings() const
|
||||
{
|
||||
if (_writeSettingsSink && *_writeSettingsSink)
|
||||
if (_writeSettingsSink)
|
||||
{
|
||||
(*_writeSettingsSink)();
|
||||
_writeSettingsSink->Notify();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -5,6 +5,9 @@
|
||||
#include "JsonManager.h"
|
||||
#include "CascadiaSettings.h"
|
||||
#include "FileUtils.h"
|
||||
#include "ApplicationState.h"
|
||||
#include "DefaultTerminal.h"
|
||||
#include "resource.h"
|
||||
|
||||
#include <til/io.h>
|
||||
|
||||
@@ -31,10 +34,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
|
||||
JsonManager::~JsonManager()
|
||||
{
|
||||
// Tear down the watcher before the rest of *this is destroyed.
|
||||
// _writeThrottler (last-declared member) is destroyed first, cancelling
|
||||
// any pending callback.
|
||||
// Stop watching first so the flush's write below can't trigger a
|
||||
// self-write callback during teardown.
|
||||
Stop();
|
||||
|
||||
// Flush any pending auto-save so a debounced write isn't lost on shutdown
|
||||
// (e.g. a setting changed moments before the app exits). The flush runs
|
||||
// _write(), which only touches the captured byte snapshot -- not the tree.
|
||||
_writeThrottler.flush();
|
||||
}
|
||||
|
||||
// Begins watching the settings directory for external changes.
|
||||
@@ -70,18 +77,24 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
// loaded tree (never a failed load or the defaults fallback).
|
||||
void JsonManager::SetLiveSettings(const Model::CascadiaSettings& settings)
|
||||
{
|
||||
// Detach the previous tree's write handler so a stale tree still retained
|
||||
// elsewhere can no longer schedule saves against us.
|
||||
if (auto old = _liveSettings.get())
|
||||
{
|
||||
winrt::get_self<implementation::CascadiaSettings>(old)->SetWriteHandler(nullptr);
|
||||
}
|
||||
|
||||
{
|
||||
std::scoped_lock lock{ _stateMutex };
|
||||
++_generation;
|
||||
_generation.bump();
|
||||
_pendingSnapshot.clear();
|
||||
_autoSaveEnabled = static_cast<bool>(settings);
|
||||
_expectedDiskHash = settings ? settings.Hash() : winrt::hstring{};
|
||||
}
|
||||
|
||||
_liveSettings = settings ? winrt::make_weak(settings) : winrt::weak_ref<Model::CascadiaSettings>{ nullptr };
|
||||
|
||||
if (settings)
|
||||
{
|
||||
_liveSettings = winrt::make_weak(settings);
|
||||
|
||||
auto self = winrt::get_self<implementation::CascadiaSettings>(settings);
|
||||
auto weakThis = get_weak();
|
||||
self->SetWriteHandler([weakThis]() {
|
||||
@@ -91,20 +104,18 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
}
|
||||
});
|
||||
}
|
||||
else
|
||||
{
|
||||
_liveSettings = nullptr;
|
||||
}
|
||||
}
|
||||
|
||||
// Unbinds the live tree and disables auto-save (e.g. on load failure or
|
||||
// when falling back to the defaults settings object).
|
||||
// when falling back to the defaults settings object). Identical to binding a
|
||||
// null tree.
|
||||
void JsonManager::ClearLiveSettings()
|
||||
{
|
||||
{
|
||||
std::scoped_lock lock{ _stateMutex };
|
||||
++_generation;
|
||||
_pendingSnapshot.clear();
|
||||
_autoSaveEnabled = false;
|
||||
_expectedDiskHash = {};
|
||||
}
|
||||
_liveSettings = winrt::weak_ref<Model::CascadiaSettings>{ nullptr };
|
||||
SetLiveSettings(nullptr);
|
||||
}
|
||||
|
||||
// Captures a snapshot of the bound tree (on the calling/owner thread) and
|
||||
@@ -112,22 +123,19 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
// thread, keeps all model-tree traversal on the owner thread.
|
||||
void JsonManager::RequestSave()
|
||||
{
|
||||
uint64_t generation{};
|
||||
{
|
||||
std::scoped_lock lock{ _stateMutex };
|
||||
if (!_autoSaveEnabled)
|
||||
{
|
||||
return;
|
||||
}
|
||||
generation = _generation;
|
||||
}
|
||||
|
||||
// A bound (non-null) tree is exactly what enables auto-save.
|
||||
const auto live = _liveSettings.get();
|
||||
if (!live)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
til::generation_t generation;
|
||||
{
|
||||
std::scoped_lock lock{ _stateMutex };
|
||||
generation = _generation;
|
||||
}
|
||||
|
||||
std::string snapshot;
|
||||
try
|
||||
{
|
||||
@@ -153,37 +161,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
_writeThrottler();
|
||||
}
|
||||
|
||||
// Synchronously writes the bound tree to disk. Used by callers that need the
|
||||
// write to complete before returning (rather than the debounced path).
|
||||
bool JsonManager::Save()
|
||||
{
|
||||
{
|
||||
std::scoped_lock lock{ _stateMutex };
|
||||
if (!_autoSaveEnabled)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
const auto live = _liveSettings.get();
|
||||
if (!live)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
std::string snapshot;
|
||||
try
|
||||
{
|
||||
snapshot = _serialize(live);
|
||||
}
|
||||
catch (...)
|
||||
{
|
||||
LOG_CAUGHT_EXCEPTION();
|
||||
return false;
|
||||
}
|
||||
return _persistSnapshot(snapshot);
|
||||
}
|
||||
|
||||
// Debounce callback (timer thread): writes the captured snapshot.
|
||||
void JsonManager::_write() noexcept
|
||||
{
|
||||
@@ -227,22 +204,24 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
|
||||
FILETIME diskTime{};
|
||||
const auto diskContent = til::io::read_file_as_utf8_string_if_exists(path, false, &diskTime);
|
||||
if (!diskContent.empty())
|
||||
if (diskContent.empty())
|
||||
{
|
||||
// The file was deleted/emptied externally. Treat as a conflict so
|
||||
// we don't silently recreate it, unless we never had a baseline
|
||||
// (nothing to lose).
|
||||
conflict = !_expectedDiskHash.empty();
|
||||
}
|
||||
else
|
||||
{
|
||||
const auto diskHash = CalculateSettingsHash(diskContent, diskTime);
|
||||
if (!_expectedDiskHash.empty() && diskHash != _expectedDiskHash)
|
||||
{
|
||||
// Disk diverged from our baseline, likely due to an external edit. Disk is
|
||||
// the source of truth, so drop our pending change.
|
||||
conflict = true;
|
||||
}
|
||||
// Disk diverged from our baseline, likely due to an external edit.
|
||||
// Disk is the source of truth, so drop our pending change.
|
||||
conflict = !_expectedDiskHash.empty() && diskHash != _expectedDiskHash;
|
||||
}
|
||||
|
||||
if (!conflict)
|
||||
{
|
||||
FILETIME newTime{};
|
||||
WriteSettingsFile(path, snapshot, /*makeBackup*/ true, &newTime);
|
||||
_expectedDiskHash = CalculateSettingsHash(snapshot, newTime);
|
||||
_expectedDiskHash = _writeContent(snapshot);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -296,8 +275,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
LOG_CAUGHT_EXCEPTION();
|
||||
}
|
||||
|
||||
// Serializes a settings tree to JSON using the same writer settings as
|
||||
// CascadiaSettings::WriteSettingsToDisk. Must run on the owner thread.
|
||||
// Serializes a settings tree to JSON. Walks the model tree (ToJson), so it
|
||||
// must run on the owner thread.
|
||||
std::string JsonManager::_serialize(const Model::CascadiaSettings& settings)
|
||||
{
|
||||
auto self = winrt::get_self<implementation::CascadiaSettings>(settings);
|
||||
@@ -309,4 +288,57 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
|
||||
return Json::writeString(wbuilder, self->ToJson());
|
||||
}
|
||||
|
||||
// Low-level write: atomically writes content to settings.json (with a .bak
|
||||
// backup) and returns the resulting content/time hash. Pure I/O -- it never
|
||||
// touches the model tree, so it is safe to call from the background timer
|
||||
// thread (the auto-save path) as well as from the owner thread.
|
||||
winrt::hstring JsonManager::_writeContent(std::string_view content)
|
||||
{
|
||||
const std::filesystem::path path{ std::wstring_view{ CascadiaSettings::SettingsPath() } };
|
||||
FILETIME newTime{};
|
||||
WriteSettingsFile(path, content, /*makeBackup*/ true, &newTime);
|
||||
return CalculateSettingsHash(content, newTime);
|
||||
}
|
||||
|
||||
// Force-writes a tree to disk: serialize + atomic write + .bak + persist the
|
||||
// default-terminal choice. Returns the new hash (empty on failure). Static --
|
||||
// no instance and no auto-save baseline check. Used by load-time fixups, the
|
||||
// settings-editor clone save, and other explicit savers. Must run on the
|
||||
// thread that owns the tree (it reads tree state for the defterm persist).
|
||||
winrt::hstring JsonManager::WriteSettings(const Model::CascadiaSettings& settings)
|
||||
try
|
||||
{
|
||||
if (!settings)
|
||||
{
|
||||
return {};
|
||||
}
|
||||
|
||||
const auto content = _serialize(settings);
|
||||
const auto hash = _writeContent(content);
|
||||
|
||||
// Persist the default terminal choice (GH#10003: only when actually
|
||||
// initialized). Read the cached member directly -- the public getter
|
||||
// would trigger an expensive defterm refresh.
|
||||
auto self = winrt::get_self<implementation::CascadiaSettings>(settings);
|
||||
if (self->_currentDefaultTerminal)
|
||||
{
|
||||
DefaultTerminal::Current(self->_currentDefaultTerminal);
|
||||
}
|
||||
|
||||
return hash;
|
||||
}
|
||||
catch (...)
|
||||
{
|
||||
LOG_CAUGHT_EXCEPTION();
|
||||
return {};
|
||||
}
|
||||
|
||||
// Resets ApplicationState and writes the user-defaults template to disk.
|
||||
void JsonManager::ResetToDefaultSettings()
|
||||
{
|
||||
ApplicationState::SharedInstance().Reset();
|
||||
const std::filesystem::path path{ std::wstring_view{ CascadiaSettings::SettingsPath() } };
|
||||
WriteSettingsFile(path, LoadStringResource(IDR_USER_DEFAULTS), /*makeBackup*/ true);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,12 +8,23 @@ Module Name:
|
||||
Abstract:
|
||||
- Owns settings.json persistence and file-change watching, split out of
|
||||
CascadiaSettings as part of the Auto-Save effort.
|
||||
- An instance is owned by the app (AppLogic), not a singleton. AppLogic binds
|
||||
the live settings tree via SetLiveSettings() ONLY after a successful load,
|
||||
and disables auto-save on load failure / defaults fallback so we never
|
||||
overwrite a user's broken settings.json with defaults.
|
||||
- Ownership: an instance is owned by the app (AppLogic), NOT a process singleton.
|
||||
The decision of "when NOT to save" lives with AppLogic — it binds the live tree
|
||||
via SetLiveSettings() ONLY after a successful load, and never binds the
|
||||
LoadDefaults() fallback, so we never overwrite a user's broken settings.json
|
||||
with defaults. There is one AppLogic per process, so an instance is already
|
||||
process-scoped; instance ownership also gives clean lifetime + testability that
|
||||
a singleton holding the live model tree would not.
|
||||
- The bound tree is held weakly and guarded by a generation token so a
|
||||
scheduled write can never run against a superseded tree.
|
||||
- Threading: the agile weak_ref is touched only on the owner (UI) thread, because
|
||||
the model *tree* it points at is not thread-safe. Serialization is snapshotted
|
||||
on the owner thread (RequestSave) and only the resulting immutable byte snapshot
|
||||
crosses to the background debounce-timer / file-I/O thread.
|
||||
- The static WriteSettings()/ResetToDefaultSettings() are force-writes callable
|
||||
without an instance (used by load-time fixups, the editor clone save, and other
|
||||
explicit savers). The instance auto-save path is the only debounced one and is
|
||||
the only path that performs the baseline lost-update conflict check.
|
||||
|
||||
Author(s):
|
||||
- Carlos Zamora - June 2026
|
||||
@@ -25,6 +36,7 @@ Author(s):
|
||||
|
||||
#include <inc/cppwinrt_utils.h>
|
||||
#include <til/throttled_func.h>
|
||||
#include <til/generational.h>
|
||||
#include <wil/filesystem.h>
|
||||
#include <mutex>
|
||||
|
||||
@@ -42,19 +54,29 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
void ClearLiveSettings();
|
||||
|
||||
void RequestSave();
|
||||
bool Save();
|
||||
|
||||
// Force-write the given tree to disk (serialize + atomic write + .bak +
|
||||
// persist default-terminal choice). Returns the new content/time hash, or
|
||||
// an empty string on failure. Static so it works without an instance:
|
||||
// used by load-time fixups, the settings-editor clone save, and other
|
||||
// explicit savers. Does NOT perform the auto-save baseline conflict check.
|
||||
static winrt::hstring WriteSettings(const Model::CascadiaSettings& settings);
|
||||
|
||||
// Reset ApplicationState and write the user-defaults template to disk.
|
||||
static void ResetToDefaultSettings();
|
||||
|
||||
TYPED_EVENT(SettingsChangedExternally, Model::JsonManager, Windows::Foundation::IInspectable);
|
||||
|
||||
private:
|
||||
// The live settings tree, bound only after a successful load. Held
|
||||
// weakly so JsonManager never keeps a superseded/failed tree alive.
|
||||
// Only touched on the owner (UI) thread.
|
||||
// Only touched on the owner (UI) thread (see Threading note above). A
|
||||
// bound (non-null) tree is exactly what enables auto-save.
|
||||
winrt::weak_ref<Model::CascadiaSettings> _liveSettings{ nullptr };
|
||||
|
||||
// Bumped on every (Set|Clear)LiveSettings so a write scheduled against
|
||||
// a previous tree can detect that it is stale and bail out.
|
||||
uint64_t _generation{ 0 };
|
||||
til::generation_t _generation;
|
||||
|
||||
// The folder watcher over the settings directory. Created in Start(),
|
||||
// torn down in Stop(). Its callback runs on a background thread.
|
||||
@@ -63,12 +85,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
// State shared between the owner thread, the debounce timer thread, and
|
||||
// the watcher thread. Guarded by _stateMutex.
|
||||
std::mutex _stateMutex;
|
||||
bool _autoSaveEnabled{ false };
|
||||
std::string _pendingSnapshot;
|
||||
// The generation the pending snapshot was captured under. If the live
|
||||
// tree is swapped (reload) before the debounced write fires, the
|
||||
// generation no longer matches and the stale snapshot is dropped.
|
||||
uint64_t _pendingSnapshotGeneration{ 0 };
|
||||
til::generation_t _pendingSnapshotGeneration;
|
||||
// The hash we expect settings.json to currently have on disk. Used to
|
||||
// distinguish our own writes from external edits, and as the baseline
|
||||
// for the lost-update conflict check.
|
||||
@@ -84,6 +105,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||
void _onFileChanged() noexcept;
|
||||
bool _persistSnapshot(const std::string& snapshot) noexcept;
|
||||
static std::string _serialize(const Model::CascadiaSettings& settings);
|
||||
static winrt::hstring _writeContent(std::string_view content);
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -13,6 +13,15 @@ namespace Microsoft.Terminal.Settings.Model
|
||||
{
|
||||
JsonManager();
|
||||
|
||||
// Force-writes the given tree to disk (serialize + atomic write + .bak).
|
||||
// Returns the new content/time hash, or an empty string on failure.
|
||||
// Static so it works without an instance: used by load-time fixups, the
|
||||
// settings-editor clone save, and other explicit savers.
|
||||
static String WriteSettings(CascadiaSettings settings);
|
||||
|
||||
// Resets ApplicationState and writes the user-defaults template to disk.
|
||||
static void ResetToDefaultSettings();
|
||||
|
||||
// Lifecycle. The folder watcher is owned here but explicitly started
|
||||
// and stopped by the app. JsonManager never reloads or touches
|
||||
// UI-bound state itself; it only raises SettingsChangedExternally.
|
||||
@@ -28,11 +37,6 @@ namespace Microsoft.Terminal.Settings.Model
|
||||
// Schedules a debounced auto-save of the bound tree. No-op if unbound.
|
||||
void RequestSave();
|
||||
|
||||
// Synchronously writes the bound tree to disk. Used by load-time
|
||||
// fixups, which must not route through the debounce. Returns true on
|
||||
// success.
|
||||
Boolean Save();
|
||||
|
||||
// Raised when an external (non-self) edit to settings.json is detected
|
||||
// on disk. The app handles marshaling any reload onto the UI thread.
|
||||
event Windows.Foundation.TypedEventHandler<JsonManager, Object> SettingsChangedExternally;
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
#include "pch.h"
|
||||
|
||||
#include "../TerminalSettingsModel/CascadiaSettings.h"
|
||||
#include "../TerminalSettingsModel/ColorScheme.h"
|
||||
#include "../TerminalSettingsModel/FileUtils.h"
|
||||
#include "JsonTestClass.h"
|
||||
|
||||
@@ -26,6 +27,7 @@ namespace SettingsModelUnitTests
|
||||
TEST_METHOD(WriteSettingsFileCreatesBackup);
|
||||
TEST_METHOD(WriteHandlerFiresOnSetterAndClear);
|
||||
TEST_METHOD(ClonedSettingsDoNotFireWriteHandler);
|
||||
TEST_METHOD(CollectionEditsFireWriteHandler);
|
||||
|
||||
private:
|
||||
static std::filesystem::path _tempFile(const wchar_t* name)
|
||||
@@ -156,4 +158,33 @@ namespace SettingsModelUnitTests
|
||||
settings->AllProfiles().GetAt(0).HistorySize(7000);
|
||||
VERIFY_ARE_EQUAL(1, writeRequests);
|
||||
}
|
||||
|
||||
void JsonManagerTests::CollectionEditsFireWriteHandler()
|
||||
{
|
||||
// Color schemes / themes aren't IInheritable, so per-property edits aren't
|
||||
// covered, but add/remove is wired at the GlobalAppSettings collection
|
||||
// level. Verify that coarse-grained coverage requests a save.
|
||||
static constexpr std::string_view settingsJson{ R"({
|
||||
"profiles": {
|
||||
"list": [
|
||||
{
|
||||
"name": "profile0",
|
||||
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}"
|
||||
}
|
||||
]
|
||||
}
|
||||
})" };
|
||||
|
||||
const auto settings = winrt::make_self<implementation::CascadiaSettings>(settingsJson);
|
||||
|
||||
auto writeRequests = 0;
|
||||
settings->SetWriteHandler([&]() { ++writeRequests; });
|
||||
|
||||
const auto scheme = winrt::make<implementation::ColorScheme>(winrt::hstring{ L"My Scheme" });
|
||||
settings->GlobalSettings().AddColorScheme(scheme);
|
||||
VERIFY_ARE_EQUAL(1, writeRequests);
|
||||
|
||||
settings->GlobalSettings().RemoveColorScheme(L"My Scheme");
|
||||
VERIFY_ARE_EQUAL(2, writeRequests);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user