From 707429ef06d8305c83b51b5e3d8a9c4233f33b6e Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 13 Jul 2020 13:56:40 -0700 Subject: [PATCH 01/12] Add Spec for winrt TerminalSettings --- doc/specs/#885 - winrt Terminal Settings.md | 133 ++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 doc/specs/#885 - winrt Terminal Settings.md diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md new file mode 100644 index 00000000000..39549d2d384 --- /dev/null +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -0,0 +1,133 @@ +--- +author: Carlos Zamora @carlos-zamora +created on: 2020-07-10 +last updated: 2020-07-10 +issue id: [#885] +--- + +# Spec Title + +## Abstract + +This spec proposes a major refactor and repurposing of the TerminalSettings project. TerminalSettings would + be responsible for exposing, serializing, and deserializing settings as WinRT objects for Windows Terminal. In + doing so, Terminal's settings model is accessible as WinRT objects to existing components like TerminalApp, + TerminalControl, and TerminalCore. Additionally, Terminal Settings can be used by the Settings UI or + Shell Extensions to modify or reference Terminal's settings respectively. + +## Inspiration + +The main driver for this change is the Settings UI. The Settings UI will need to read and modify Terminal Settings. + At the time of writing this spec, the Terminal Settings are serialized as objects in the TerminalApp project. + To access these objects, the Settings UI would need to be a part of TerminalApp too, making it more bloated. + +## Solution Design + +### Terminal Settings Model: Objects and Projections + +The objects/interfaces introduced in this section will be exposed as WinRT objects/interfaces + within the `Microsoft.Terminal.Settings` namespace. This allows them to be interacted with across + different project layers. + +As before, a terminal instance will need a `Profile` (formerly `TerminalSettings`) to be able to update its settings. +`Profile` is a composition of... +- `ICore`: settings used to customize TerminalCore +- `IControl`: settings used to customize TerminalControl + +Additionally, the top-level object holding all of the settings related to Windows Terminal will be +`Settings`. This will be a composition of... +- `AppSettings app`: global settings that generally affect TerminalApp +- `IObservableVector profiles`: a list of `Profile` objects +- `IObservableMap keybindings`: a list of key bindings indexed by their unique key chord +- `IObservableMap commands`: a list of command palette commands indexed by their (autogenerated) name +- `IObservableMap schemes`: a list of ColorScheme objects indexed by their unique name +- `IVector warnings`: a list of warnings that occurred during serialization + +Introducing a number of observable WinRT types allows for other components to subscribe to changes + to these collections. A particular example of this being used can be seen in the (Settings UI: Preview section)[#settings-ui:-preview]. + + + +### Terminal Settings Model: Serialization and Deserialization + +Introducing these `Microsoft.Terminal.Settings` WinRT objects also allow the serialization and deserialization + logic from TerminalApp to be moved to TerminalSettings. `JsonUtils` introduces several quick and easy methods + for setting serialization. This will be moved into the `Microsoft.Terminal.Settings` namespace too. + +Deserialization will be an extension of the existing `JsonUtils` `ConversionTrait` struct template. `ConversionTrait` + already includes `FromJson` and `CanConvert`. Deserialization would be handled by a `ToJson` function. + + +### Terminal Settings Model: Consumption + +Separate projects can consume the Terminal Settings objects as needed. + +TerminalApp would use... +- `profiles` to populate the dropdown menu +- `keybindings` to detect active key bindings and which actions they run +- `commands` to populate the Command Palette +- `warnings` to show a serialization warning, if any + +TermControl would be given a `Profile` object, but as an `IControl` object. Whereas, TerminalCore + would be passed the same `Profile` object, but as an `ICore` object. This is similar as is done today + with `TerminalSettings` being split into `IControlSettings`, and `ICoreSettings`. + + +## UI/UX Design + +N/A + +## Capabilities + +### Accessibility + +N/A + +### Security + +N/A + +### Reliability + +N/A + +### Compatibility + +N/A + +### Performance, Power, and Efficiency + +## Potential Issues + +### Deserialization + +After deserializing the settings, injecting the new json into settings.json + should not remove the existing comments or formatting. + +## Future considerations + +### Theming + +When XAML Theming gets introduced, `TerminalSettings` will need to provide + serialization and deserialization logic for the new JSON. + + +### Dropdown Customization + +Profiles are proposed to be represented in an `IObservableVector`. They could be represented as an + `IObservableMap` with the guid or profile name used as the key value. The main concern with representing + profiles as a map is that this loses the order for the dropdown menu. + +Once Dropdown Customization is introduced, `Settings` is expected to serialize the relevant JSON and expose + it to TerminalApp (probably as an `IObservableVector`). Since this new object would + handle the ordering of profiles (if any), the `IObservableVector` could be replaced with + an observable map indexed by the profile name, thus even potentially providing an opportunity to + remove guid as an identifier entirely. + + +## Resources + +- [Spec: XAML Theming](https://github.com/microsoft/terminal/pull/5772) +- [Spec: Dropdown Customization](https://github.com/microsoft/terminal/pull/5888) +- [New JSON Utils](https://github.com/microsoft/terminal/pull/6590) +- [Spec: Settings UI](https://github.com/microsoft/terminal/pull/6720) From 8be5ef5688342e153ac37e74f8ee6a698b1ebf59 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 14 Jul 2020 16:33:59 -0700 Subject: [PATCH 02/12] Fix TerminalSettings object purpose and definition --- doc/specs/#885 - winrt Terminal Settings.md | 73 ++++++++++++--------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index 39549d2d384..9765594ed40 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -2,7 +2,7 @@ author: Carlos Zamora @carlos-zamora created on: 2020-07-10 last updated: 2020-07-10 -issue id: [#885] +issue id: /#885 --- # Spec Title @@ -26,26 +26,41 @@ The main driver for this change is the Settings UI. The Settings UI will need to ### Terminal Settings Model: Objects and Projections The objects/interfaces introduced in this section will be exposed as WinRT objects/interfaces - within the `Microsoft.Terminal.Settings` namespace. This allows them to be interacted with across + within the `Microsoft.Terminal.Settings.Model` namespace. This allows them to be interacted with across different project layers. -As before, a terminal instance will need a `Profile` (formerly `TerminalSettings`) to be able to update its settings. -`Profile` is a composition of... -- `ICore`: settings used to customize TerminalCore -- `IControl`: settings used to customize TerminalControl - Additionally, the top-level object holding all of the settings related to Windows Terminal will be -`Settings`. This will be a composition of... -- `AppSettings app`: global settings that generally affect TerminalApp -- `IObservableVector profiles`: a list of `Profile` objects -- `IObservableMap keybindings`: a list of key bindings indexed by their unique key chord -- `IObservableMap commands`: a list of command palette commands indexed by their (autogenerated) name -- `IObservableMap schemes`: a list of ColorScheme objects indexed by their unique name -- `IVector warnings`: a list of warnings that occurred during serialization +`AppSettings`. The runtime class will look something like this: +```c++ +runtimeclass AppSettings +{ + // global settings that generally only affect TerminalApp + GlobalSettings Globals { get; }; + + // the list of profiles available to Windows Terminal (custom and dynamically generated) + IObservableVector Profiles { get; }; + + // the list of key bindings indexed by their unique key chord + IObservableMap Keybindings { get; }; + + // the list of command palette commands indexed by their name (custom or auto-generated) + IObservableMap Commands { get; }; + + // the list of color schemes indexed by their unique names + IObservableMap Schemes { get; }; + + // a list of warnings encountered during the serialization of the JSON + IVector Warnings { get; }; +} +``` Introducing a number of observable WinRT types allows for other components to subscribe to changes - to these collections. A particular example of this being used can be seen in the (Settings UI: Preview section)[#settings-ui:-preview]. + to these collections. A particular example of this being used can be a preview of the TermControl in + the upcoming Settings UI. +Adjacent to the introduction of `AppSettings`, `IControlSettings` and `ICoreSettings` will be moved + to the `Microsoft.Terminal.TerminalControl` namespace. This allows for a better consumption of the + settings model that is covered later in the (Consumption section)[#terminal-settings-model:-consumption]. ### Terminal Settings Model: Serialization and Deserialization @@ -62,15 +77,20 @@ Deserialization will be an extension of the existing `JsonUtils` `ConversionTrai Separate projects can consume the Terminal Settings objects as needed. -TerminalApp would use... -- `profiles` to populate the dropdown menu -- `keybindings` to detect active key bindings and which actions they run -- `commands` to populate the Command Palette -- `warnings` to show a serialization warning, if any +TerminalApp would reference `AppSettings` for the following functionality... +- `Profiles` to populate the dropdown menu +- `Keybindings` to detect active key bindings and which actions they run +- `Commands` to populate the Command Palette +- `Warnings` to show a serialization warning, if any + +At the time of writing this spec, TerminalApp constructs `TerminalControl.TerminalSettings` WinRT objects + to expose `IControlSettings` and `ICoreSettings` to any hosted terminals. In moving `IControlSettings` + and `ICoreSettings` down to the TerminalControl layer, TerminalApp can now have better control over + how to expose relevant settings to a TerminalControl instance. -TermControl would be given a `Profile` object, but as an `IControl` object. Whereas, TerminalCore - would be passed the same `Profile` object, but as an `ICore` object. This is similar as is done today - with `TerminalSettings` being split into `IControlSettings`, and `ICoreSettings`. +`TerminalSettings` will be moved to TerminalApp and act as a bridge that queries `AppSettings`. Thus, + when TermControl requests `IControlSettings` data, `TerminalSettings` responds by extracting the + relevant information from `AppSettings`. TerminalCore operates the same way with `ICoreSettings`. ## UI/UX Design @@ -106,12 +126,6 @@ After deserializing the settings, injecting the new json into settings.json ## Future considerations -### Theming - -When XAML Theming gets introduced, `TerminalSettings` will need to provide - serialization and deserialization logic for the new JSON. - - ### Dropdown Customization Profiles are proposed to be represented in an `IObservableVector`. They could be represented as an @@ -127,7 +141,6 @@ Once Dropdown Customization is introduced, `Settings` is expected to serialize t ## Resources -- [Spec: XAML Theming](https://github.com/microsoft/terminal/pull/5772) - [Spec: Dropdown Customization](https://github.com/microsoft/terminal/pull/5888) - [New JSON Utils](https://github.com/microsoft/terminal/pull/6590) - [Spec: Settings UI](https://github.com/microsoft/terminal/pull/6720) From a99f459128a7ce70cb824c061da5a7bf336dd678 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 16 Jul 2020 11:32:26 -0700 Subject: [PATCH 03/12] Correctly link issue id --- doc/specs/#885 - winrt Terminal Settings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index 9765594ed40..4882ba9031f 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -2,7 +2,7 @@ author: Carlos Zamora @carlos-zamora created on: 2020-07-10 last updated: 2020-07-10 -issue id: /#885 +issue id: [#885](https://github.com/microsoft/terminal/issues/885) --- # Spec Title From 5cb8f6e664e9ed05cedd9ce6edf9a291b98e31c8 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 17 Jul 2020 19:24:37 -0700 Subject: [PATCH 04/12] add more details on interacting with settings model --- doc/specs/#885 - winrt Terminal Settings.md | 57 ++++++++++++++++++--- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index 4882ba9031f..f2f94096cd1 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -65,33 +65,78 @@ Adjacent to the introduction of `AppSettings`, `IControlSettings` and `ICoreSett ### Terminal Settings Model: Serialization and Deserialization -Introducing these `Microsoft.Terminal.Settings` WinRT objects also allow the serialization and deserialization +Introducing these `Microsoft.Terminal.Settings.Model` WinRT objects also allow the serialization and deserialization logic from TerminalApp to be moved to TerminalSettings. `JsonUtils` introduces several quick and easy methods - for setting serialization. This will be moved into the `Microsoft.Terminal.Settings` namespace too. + for setting serialization. This will be moved into the `Microsoft.Terminal.Settings.Model` namespace too. Deserialization will be an extension of the existing `JsonUtils` `ConversionTrait` struct template. `ConversionTrait` already includes `FromJson` and `CanConvert`. Deserialization would be handled by a `ToJson` function. -### Terminal Settings Model: Consumption +### Interacting with the Terminal Settings Model -Separate projects can consume the Terminal Settings objects as needed. +Separate projects can interact with `AppSettings` to extract data, subscribe to changes, and commit changes. +This API will look something like this: +```c++ +runtimeclass AppSettings +{ + AppSettings(); + + // Load a settings file, and layer those changes on top of the existing AppSettings + void LayerSettings(String path); + + // Create a copy of the existing AppSettings + AppSettings Clone(); + + // Deserialize the existing AppSettings into a settings file + void Export(String path); +} +``` -TerminalApp would reference `AppSettings` for the following functionality... +#### TerminalApp: Loading and Reloading Changes + +TerminalApp will construct and reference an `AppSettings settings` using a shared smart pointer as follows: +- TerminalApp will have a global reference to "defaults.json" and "settings.json" filepaths as `constexpr` +- construct an `AppSettings` using the default constructor +- `settings.LayerSettings("defaults.json")` +- `settings.LayerSettings("settings.json")` + +**NOTE:** This model allows us to layer even more settings files on top of the existing Terminal Settings + Model, if so desired. This could be helpful when importing additional settings files from an external location + such as a marketplace. + +When TerminalApp detects a change to "settings.json", it'll repeat the steps above. We could cache the result from + `settings.LayerSettings("defaults.json")` to improve performance. + +Additionally, TerminalApp would reference `settings` for the following functionality... - `Profiles` to populate the dropdown menu - `Keybindings` to detect active key bindings and which actions they run - `Commands` to populate the Command Palette - `Warnings` to show a serialization warning, if any +#### TerminalControl: Acquiring and Applying the Settings + At the time of writing this spec, TerminalApp constructs `TerminalControl.TerminalSettings` WinRT objects to expose `IControlSettings` and `ICoreSettings` to any hosted terminals. In moving `IControlSettings` and `ICoreSettings` down to the TerminalControl layer, TerminalApp can now have better control over how to expose relevant settings to a TerminalControl instance. -`TerminalSettings` will be moved to TerminalApp and act as a bridge that queries `AppSettings`. Thus, +`TerminalSettings` (which implements `IControlSettings` and `ICoreSettings`) will be moved to TerminalApp and act as a bridge that queries `AppSettings`. Thus, when TermControl requests `IControlSettings` data, `TerminalSettings` responds by extracting the relevant information from `AppSettings`. TerminalCore operates the same way with `ICoreSettings`. +#### Settings UI: Modifying and Applying the Settings + +The Settings UI will also have a reference to the `AppSettings settings` using a shared smart pointer. + + ## UI/UX Design From 1ef02752dab29276d415524d8e4f572fdcf244cb Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 20 Jul 2020 11:54:21 -0400 Subject: [PATCH 05/12] describe how to export/apply changes from Settings UI --- doc/specs/#885 - winrt Terminal Settings.md | 36 ++++++++++++++------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index f2f94096cd1..c3d0d366a01 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -87,9 +87,10 @@ runtimeclass AppSettings // Create a copy of the existing AppSettings AppSettings Clone(); - - // Deserialize the existing AppSettings into a settings file - void Export(String path); + + // Compares object to "source" and applies changes to + // the settings file at "outPath" + void ApplyChanges(String outPath, AppSettings source); } ``` @@ -127,16 +128,29 @@ At the time of writing this spec, TerminalApp constructs `TerminalControl.Termin #### Settings UI: Modifying and Applying the Settings -The Settings UI will also have a reference to the `AppSettings settings` using a shared smart pointer. +The Settings UI will also have a reference to the `AppSettings settings` from TerminalApp using a shared smart pointer +as `settingsSource`. When the Settings UI is opened up, the Settings UI will also have its own `AppSettings settingsClone` +that is a clone of TerminalApp's `AppSettings`. +```c++ +settingsClone = settingsSource.Clone() +``` + +As the user navigates the Settings UI, the relevant contents of `settingsSource` will be retrieved and presented. +As the user makes changes to the Settings UI, XAML will hold on to the changes that are made. +When the user saves/applies the changes in the XAML, `settingsClone` will be updated to reflect those changes, +then `settingsClone.ApplyChanges("settings.json", settingsSource)` is called. This compares the changes between +`settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. + +As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `AppSettings`. + Since the above triggers a change to `settings.json`, TerminalApp will also update itself. When + something like this occurs, `settingsSource` will automatically be updated too. - + **NOTE:** In the event that the user would want to export their current configuration, `ApplyChanges` + can be used to export the changes to a new file. ## UI/UX Design From 2a87c4e4f585dc030726b446be1e88d1603f625d Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 20 Jul 2020 16:28:48 -0400 Subject: [PATCH 06/12] address Dustin's PR comments --- doc/specs/#885 - winrt Terminal Settings.md | 23 +++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index c3d0d366a01..450243683a0 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -90,14 +90,14 @@ runtimeclass AppSettings // Compares object to "source" and applies changes to // the settings file at "outPath" - void ApplyChanges(String outPath, AppSettings source); + void Save(String outPath); } ``` #### TerminalApp: Loading and Reloading Changes -TerminalApp will construct and reference an `AppSettings settings` using a shared smart pointer as follows: -- TerminalApp will have a global reference to "defaults.json" and "settings.json" filepaths as `constexpr` +TerminalApp will construct and reference an `AppSettings settings` as follows: +- TerminalApp will have a global reference to "defaults.json" and "settings.json" filepaths - construct an `AppSettings` using the default constructor - `settings.LayerSettings("defaults.json")` - `settings.LayerSettings("settings.json")` @@ -135,11 +135,9 @@ that is a clone of TerminalApp's `AppSettings`. settingsClone = settingsSource.Clone() ``` -As the user navigates the Settings UI, the relevant contents of `settingsSource` will be retrieved and presented. -As the user makes changes to the Settings UI, XAML will hold on to the changes that are made. -When the user saves/applies the changes in the XAML, `settingsClone` will be updated to reflect those changes, -then `settingsClone.ApplyChanges("settings.json", settingsSource)` is called. This compares the changes between -`settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. +As the user navigates the Settings UI, the relevant contents of `settingsClone` will be retrieved and presented. + As the user makes changes to the Settings UI, XAML will update `settingsClone` using XAML data binding. + When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; this compares the changes between `settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `AppSettings`. Since the above triggers a change to `settings.json`, TerminalApp will also update itself. When @@ -149,7 +147,7 @@ In the case that a user is simultaneously updating the settings file directly an `settingsSource` and `settingsClone` can be compared to ensure that the Settings UI, the TerminalApp, and the settings files are all in sync. - **NOTE:** In the event that the user would want to export their current configuration, `ApplyChanges` + **NOTE:** In the event that the user would want to export their current configuration, `Save` can be used to export the changes to a new file. ## UI/UX Design @@ -183,6 +181,13 @@ N/A After deserializing the settings, injecting the new json into settings.json should not remove the existing comments or formatting. +The deserialization process takes place right after comparing the `settingsSource` and `settingsClone` objects. + For each setting found in the diff, we go to the relevant part of the JSON and see if the key is already there. + If it is, we update the value to be the one from `settingsClone`. Otherwise, we append the key/value pair + at the end of the section (much like we do with dynamic profiles in `profiles`). + + + ## Future considerations ### Dropdown Customization From 59460b802866ae6fa2c7c7b7bc6ac3d81d6fdede Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 21 Jul 2020 13:32:01 -0400 Subject: [PATCH 07/12] minor cleanup; more details on dropdown idea --- doc/specs/#885 - winrt Terminal Settings.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index 450243683a0..3f2d2de7ec7 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -128,16 +128,17 @@ At the time of writing this spec, TerminalApp constructs `TerminalControl.Termin #### Settings UI: Modifying and Applying the Settings -The Settings UI will also have a reference to the `AppSettings settings` from TerminalApp using a shared smart pointer -as `settingsSource`. When the Settings UI is opened up, the Settings UI will also have its own `AppSettings settingsClone` -that is a clone of TerminalApp's `AppSettings`. +The Settings UI will also have a reference to the `AppSettings settings` from TerminalApp + as `settingsSource`. When the Settings UI is opened up, the Settings UI will also have its own `AppSettings settingsClone` + that is a clone of TerminalApp's `AppSettings`. ```c++ settingsClone = settingsSource.Clone() ``` As the user navigates the Settings UI, the relevant contents of `settingsClone` will be retrieved and presented. As the user makes changes to the Settings UI, XAML will update `settingsClone` using XAML data binding. - When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; this compares the changes between `settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. + When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; + this compares the changes between `settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `AppSettings`. Since the above triggers a change to `settings.json`, TerminalApp will also update itself. When @@ -196,11 +197,11 @@ Profiles are proposed to be represented in an `IObservableVector`. They could be `IObservableMap` with the guid or profile name used as the key value. The main concern with representing profiles as a map is that this loses the order for the dropdown menu. -Once Dropdown Customization is introduced, `Settings` is expected to serialize the relevant JSON and expose - it to TerminalApp (probably as an `IObservableVector`). Since this new object would - handle the ordering of profiles (if any), the `IObservableVector` could be replaced with - an observable map indexed by the profile name, thus even potentially providing an opportunity to - remove guid as an identifier entirely. +Once Dropdown Customization is introduced, `AppSettings` can expose it to TerminalApp as + an `IObservableVector`. This handles the ordering of profiles and other + dropdown menu items. Additionally, `IObservableVector` can be replaced with + `IObservableMap` or `IObservableMap` (where the key is + a profile name). This would improve profile retrieval performance. ## Resources From b025ab676b797638f6520728555a9c1a4acc8e98 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 22 Jul 2020 16:37:26 -0400 Subject: [PATCH 08/12] apply feedback from brownbag --- doc/specs/#885 - winrt Terminal Settings.md | 134 ++++++++++++-------- 1 file changed, 81 insertions(+), 53 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index 3f2d2de7ec7..6e396de3495 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -80,34 +80,26 @@ This API will look something like this: ```c++ runtimeclass AppSettings { - AppSettings(); - - // Load a settings file, and layer those changes on top of the existing AppSettings - void LayerSettings(String path); - - // Create a copy of the existing AppSettings - AppSettings Clone(); - - // Compares object to "source" and applies changes to - // the settings file at "outPath" - void Save(String outPath); + // Create an AppSettings based on the defaults.json file data + // and layer the settings file at "path" onto it + AppSettings(String path); } ``` #### TerminalApp: Loading and Reloading Changes TerminalApp will construct and reference an `AppSettings settings` as follows: -- TerminalApp will have a global reference to "defaults.json" and "settings.json" filepaths -- construct an `AppSettings` using the default constructor -- `settings.LayerSettings("defaults.json")` -- `settings.LayerSettings("settings.json")` +- TerminalApp will have a global reference to the "settings.json" filepath +- construct an `AppSettings` using `AppSettings("settings.json")`. This builds an `AppSettings` + from the "defaults.json" file data (which is already compiled as a string literal) + and layers the settings.json data on top of it. **NOTE:** This model allows us to layer even more settings files on top of the existing Terminal Settings Model, if so desired. This could be helpful when importing additional settings files from an external location such as a marketplace. When TerminalApp detects a change to "settings.json", it'll repeat the steps above. We could cache the result from - `settings.LayerSettings("defaults.json")` to improve performance. + constructing an `AppSettings` from "defaults.json" data to improve performance. Additionally, TerminalApp would reference `settings` for the following functionality... - `Profiles` to populate the dropdown menu @@ -122,34 +114,12 @@ At the time of writing this spec, TerminalApp constructs `TerminalControl.Termin and `ICoreSettings` down to the TerminalControl layer, TerminalApp can now have better control over how to expose relevant settings to a TerminalControl instance. -`TerminalSettings` (which implements `IControlSettings` and `ICoreSettings`) will be moved to TerminalApp and act as a bridge that queries `AppSettings`. Thus, - when TermControl requests `IControlSettings` data, `TerminalSettings` responds by extracting the - relevant information from `AppSettings`. TerminalCore operates the same way with `ICoreSettings`. - -#### Settings UI: Modifying and Applying the Settings - -The Settings UI will also have a reference to the `AppSettings settings` from TerminalApp - as `settingsSource`. When the Settings UI is opened up, the Settings UI will also have its own `AppSettings settingsClone` - that is a clone of TerminalApp's `AppSettings`. -```c++ -settingsClone = settingsSource.Clone() -``` - -As the user navigates the Settings UI, the relevant contents of `settingsClone` will be retrieved and presented. - As the user makes changes to the Settings UI, XAML will update `settingsClone` using XAML data binding. - When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; - this compares the changes between `settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. - -As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `AppSettings`. - Since the above triggers a change to `settings.json`, TerminalApp will also update itself. When - something like this occurs, `settingsSource` will automatically be updated too. - -In the case that a user is simultaneously updating the settings file directly and the Settings UI, - `settingsSource` and `settingsClone` can be compared to ensure that the Settings UI, the TerminalApp, - and the settings files are all in sync. +`TerminalSettings` (which implements `IControlSettings` and `ICoreSettings`) will be moved to + TerminalApp and act as a bridge connecting `AppSettings` to the TermControl. It will operate + very similarly as it does today. On construction of the TermControl or hot-reload, + `TerminalSettings` will be constructed by copying the relevant values of `AppSettings`. + Then, it will be passed to TermControl (and TermCore by extension). - **NOTE:** In the event that the user would want to export their current configuration, `Save` - can be used to export the changes to a new file. ## UI/UX Design @@ -191,21 +161,79 @@ The deserialization process takes place right after comparing the `settingsSourc ## Future considerations -### Dropdown Customization +### TerminalSettings: passing by reference + +`TermApp` synthesizes a `TerminalSettings` by copying the relevant values of `AppSettings`, + then giving it to a Terminal Control. Some visual keybindings and interactions like ctrl+scroll + and ctrl+shift+scroll to change the font size and acrylic opacity operate by directly modifying + the value of the instantiated `TerminalSettings`. However, when a settings reload occurs, + these instanced changes are lost. + +`TerminalSettings` can be used as a WinRT object that references (instead of copies) the relevant + values of `AppSettings`. This would prevent those instanced changes from being lost on a settings + reload. -Profiles are proposed to be represented in an `IObservableVector`. They could be represented as an - `IObservableMap` with the guid or profile name used as the key value. The main concern with representing - profiles as a map is that this loses the order for the dropdown menu. +Since previewable commands like `setColorScheme` would require a clone of the existing `TerminalSettings`, + a `Clone` API can be added on `TerminalSettings` to accomplish that. When passing by value, + `TerminalSettings` can just overwrite the existing property (i.e.: color scheme). When passing + by reference, a slightly more complex mechanism is required to override the value. -Once Dropdown Customization is introduced, `AppSettings` can expose it to TerminalApp as - an `IObservableVector`. This handles the ordering of profiles and other - dropdown menu items. Additionally, `IObservableVector` can be replaced with - `IObservableMap` or `IObservableMap` (where the key is - a profile name). This would improve profile retrieval performance. +Now, instead of overwriting the value, we need to override the reference to a constant value +(i.e.: `snapOnInput=true`) or a referenced value (i.e.: `colorScheme`). +### Layering Additonal Settings +As we begin to introduce more sources that affect the settings (via extensions or themes), + we can introduce a `LayerSettings(String path)`. This layers the new settings file + onto the existing `AppSettings`. This is already done internally, we would just expose + it via C++/WinRT. + +```c++ +runtimeclass AppSettings +{ + // Load a settings file, and layer those changes on top of the existing AppSettings + void LayerSettings(String path); +} +``` + +### Settings UI: Modifying and Applying the Settings (DRAFT) + +```c++ +runtimeclass AppSettings +{ + // Create a copy of the existing AppSettings + AppSettings Clone(); + + // Compares object to "source" and applies changes to + // the settings file at "outPath" + void Save(String outPath); +} +``` + +The Settings UI will also have a reference to the `AppSettings settings` from TerminalApp + as `settingsSource`. When the Settings UI is opened up, the Settings UI will also have its own `AppSettings settingsClone` + that is a clone of TerminalApp's `AppSettings`. +```c++ +settingsClone = settingsSource.Clone() +``` + +As the user navigates the Settings UI, the relevant contents of `settingsClone` will be retrieved and presented. + As the user makes changes to the Settings UI, XAML will update `settingsClone` using XAML data binding. + When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; + this compares the changes between `settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. + +As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `AppSettings`. + Since the above triggers a change to `settings.json`, TerminalApp will also update itself. When + something like this occurs, `settingsSource` will automatically be updated too. + +In the case that a user is simultaneously updating the settings file directly and the Settings UI, + `settingsSource` and `settingsClone` can be compared to ensure that the Settings UI, the TerminalApp, + and the settings files are all in sync. + + **NOTE:** In the event that the user would want to export their current configuration, `Save` + can be used to export the changes to a new file. ## Resources -- [Spec: Dropdown Customization](https://github.com/microsoft/terminal/pull/5888) +- [Previewable Commands](https://github.com/microsoft/terminal/issues/6689) - [New JSON Utils](https://github.com/microsoft/terminal/pull/6590) - [Spec: Settings UI](https://github.com/microsoft/terminal/pull/6720) From d4f7b517b9f8cbd76ece2cf8cb0cd501248f3bf5 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 22 Jul 2020 16:42:26 -0400 Subject: [PATCH 09/12] good bot --- doc/specs/#885 - winrt Terminal Settings.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index 6e396de3495..f7d77f8dfc0 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -173,7 +173,7 @@ The deserialization process takes place right after comparing the `settingsSourc values of `AppSettings`. This would prevent those instanced changes from being lost on a settings reload. -Since previewable commands like `setColorScheme` would require a clone of the existing `TerminalSettings`, +Since previewing commands like `setColorScheme` would require a clone of the existing `TerminalSettings`, a `Clone` API can be added on `TerminalSettings` to accomplish that. When passing by value, `TerminalSettings` can just overwrite the existing property (i.e.: color scheme). When passing by reference, a slightly more complex mechanism is required to override the value. @@ -181,7 +181,7 @@ Since previewable commands like `setColorScheme` would require a clone of the ex Now, instead of overwriting the value, we need to override the reference to a constant value (i.e.: `snapOnInput=true`) or a referenced value (i.e.: `colorScheme`). -### Layering Additonal Settings +### Layering Additional Settings As we begin to introduce more sources that affect the settings (via extensions or themes), we can introduce a `LayerSettings(String path)`. This layers the new settings file onto the existing `AppSettings`. This is already done internally, we would just expose @@ -234,6 +234,6 @@ In the case that a user is simultaneously updating the settings file directly an ## Resources -- [Previewable Commands](https://github.com/microsoft/terminal/issues/6689) +- [Preview Commands](https://github.com/microsoft/terminal/issues/6689) - [New JSON Utils](https://github.com/microsoft/terminal/pull/6590) - [Spec: Settings UI](https://github.com/microsoft/terminal/pull/6720) From 43fa8f5e9bb67d35445f71ddf2b0911d7cc4b331 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 31 Aug 2020 14:16:30 -0700 Subject: [PATCH 10/12] polish + add error handling & key bindings --- doc/specs/#885 - winrt Terminal Settings.md | 150 +++++++++++--------- 1 file changed, 81 insertions(+), 69 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index f7d77f8dfc0..3c0d604a561 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -9,58 +9,73 @@ issue id: [#885](https://github.com/microsoft/terminal/issues/885) ## Abstract -This spec proposes a major refactor and repurposing of the TerminalSettings project. TerminalSettings would - be responsible for exposing, serializing, and deserializing settings as WinRT objects for Windows Terminal. In - doing so, Terminal's settings model is accessible as WinRT objects to existing components like TerminalApp, - TerminalControl, and TerminalCore. Additionally, Terminal Settings can be used by the Settings UI or +This spec proposes a major refactor and repurposing of the TerminalSettings project as the TerminalSettingsModel. + TerminalSettingsModel would be responsible for exposing, serializing, and deserializing settings as WinRT objects + for Windows Terminal. In doing so, Terminal's settings model is accessible as WinRT objects to existing components + like TerminalApp, TerminalControl, and TerminalCore. Additionally, Terminal Settings can be used by the Settings UI or Shell Extensions to modify or reference Terminal's settings respectively. ## Inspiration -The main driver for this change is the Settings UI. The Settings UI will need to read and modify Terminal Settings. - At the time of writing this spec, the Terminal Settings are serialized as objects in the TerminalApp project. - To access these objects, the Settings UI would need to be a part of TerminalApp too, making it more bloated. +The main driver for this change is the Settings UI. The Settings UI will need to read and modify Terminal's settings + objects. At the time of writing this spec, the Terminal's settings are serialized as objects in the TerminalApp project. + To access these objects via XAML, the Settings UI needs them to be WinRT objects. Additional features that need the + settings objects to be WinRT objects include future shell extensions, like jumplist. ## Solution Design ### Terminal Settings Model: Objects and Projections -The objects/interfaces introduced in this section will be exposed as WinRT objects/interfaces - within the `Microsoft.Terminal.Settings.Model` namespace. This allows them to be interacted with across - different project layers. +The following TerminalApp objects will become WinRT objects and will be moved to the TerminalSettingsModel project +(formerly TerminalSettings): +- ColorScheme +- Profile +- GlobalAppSettings +- CascadiaSettings -Additionally, the top-level object holding all of the settings related to Windows Terminal will be -`AppSettings`. The runtime class will look something like this: -```c++ -runtimeclass AppSettings -{ - // global settings that generally only affect TerminalApp - GlobalSettings Globals { get; }; +The TerminalSettingsModel project will have a root namespace of `Microsoft.Terminal.Settings.Model`. - // the list of profiles available to Windows Terminal (custom and dynamically generated) - IObservableVector Profiles { get; }; +Adjacent to the introduction of these settings objects, `IControlSettings` and `ICoreSettings` will be moved + to the `Microsoft.Terminal.TerminalControl` namespace. This allows for a better consumption of the + settings model that is covered later in the (Consumption section)[#terminal-settings-model:-consumption]. - // the list of key bindings indexed by their unique key chord - IObservableMap Keybindings { get; }; +#### Moving/Splitting the Action Model - // the list of command palette commands indexed by their name (custom or auto-generated) - IObservableMap Commands { get; }; +Windows Terminal represents actions via several objects: +- `AppKeyBindings`: a map of all the defined keybindings and their corresponding actions +- `ActionAndArgs`: a (de)serializable action (this holds more objects inside of it, but we won't focus on that for now) +- `ShortcutActionDispatch`: responsible for dispatching events pertinent to a given ActionAndArgs object +`TerminalApp`'s `TerminalPage` handles any events dispatched by the `ShortcutActionDispatch`. - // the list of color schemes indexed by their unique names - IObservableMap Schemes { get; }; +With the introduction of the TerminalSettingsModel, we will split `AppKeyBindings` using a `KeyMapping` class. + This separation will look something like the following: +```c++ +namespace TerminalApp +{ + [default_interface] runtimeclass AppKeyBindings : Microsoft.Terminal.TerminalControl.IKeyBindings + { + AppKeyBindings(); + + // NOTE: It may be possible to move both of these to the constructor instead + void SetDispatch(ShortcutActionDispatch dispatch); + void SetKeyMap(KeyMapping keymap); + } +} - // a list of warnings encountered during the serialization of the JSON - IVector Warnings { get; }; +namespace TerminalSettingsModel +{ + [default_interface] runtimeclass KeyMapping + { + void SetKeyBinding(ActionAndArgs actionAndArgs, Microsoft.Terminal.TerminalControl.KeyChord chord); + void ClearKeyBinding(Microsoft.Terminal.TerminalControl.KeyChord chord); + + Microsoft.Terminal.TerminalControl.KeyChord GetKeyBindingForAction(ShortcutAction action); + Microsoft.Terminal.TerminalControl.KeyChord GetKeyBindingForActionWithArgs(ActionAndArgs actionAndArgs); + } } ``` - -Introducing a number of observable WinRT types allows for other components to subscribe to changes - to these collections. A particular example of this being used can be a preview of the TermControl in - the upcoming Settings UI. - -Adjacent to the introduction of `AppSettings`, `IControlSettings` and `ICoreSettings` will be moved - to the `Microsoft.Terminal.TerminalControl` namespace. This allows for a better consumption of the - settings model that is covered later in the (Consumption section)[#terminal-settings-model:-consumption]. +This separation leaves `AppKeyBindings` with the responsibility of detecting and dispatching actions, whereas + `KeyMapping` handles the (de)serialization and navigation of the key bindings. ### Terminal Settings Model: Serialization and Deserialization @@ -73,39 +88,36 @@ Deserialization will be an extension of the existing `JsonUtils` `ConversionTrai already includes `FromJson` and `CanConvert`. Deserialization would be handled by a `ToJson` function. -### Interacting with the Terminal Settings Model +### Terminal Settings Model: Warnings and Serialization Errors + +Today, if the serialization of `CascadiaSettings` encounters any errors, an exception is thrown and caught/handled + by falling back to a simple `CascadiaSettings` object. However, WinRT does not support exceptions. + +To get around this issue, when `CascadiaSettings` encounters a serialization error, it must internally record + any pertinent information for that error, and return the simple `CascadiaSettings` as if nothing happened. + The consumer must then call `CascadiaSettings::GetErrors()` and `CascadiaSettings::GetWarnings()` to properly + understand whether an error ocurred and how to present that to the user. -Separate projects can interact with `AppSettings` to extract data, subscribe to changes, and commit changes. -This API will look something like this: -```c++ -runtimeclass AppSettings -{ - // Create an AppSettings based on the defaults.json file data - // and layer the settings file at "path" onto it - AppSettings(String path); -} -``` #### TerminalApp: Loading and Reloading Changes -TerminalApp will construct and reference an `AppSettings settings` as follows: +TerminalApp will construct and reference a `CascadiaSettings settings` as follows: - TerminalApp will have a global reference to the "settings.json" filepath -- construct an `AppSettings` using `AppSettings("settings.json")`. This builds an `AppSettings` +- construct an `CascadiaSettings` using `CascadiaSettings("settings.json")`. This builds an `CascadiaSettings` from the "defaults.json" file data (which is already compiled as a string literal) and layers the settings.json data on top of it. +- check for errors/warnings, and handle them appropriately + +This will be different from the current model which has the settings.json path hardcoded, and is simplified + to a `LoadAll()` call wrapped in error handlers. **NOTE:** This model allows us to layer even more settings files on top of the existing Terminal Settings Model, if so desired. This could be helpful when importing additional settings files from an external location such as a marketplace. -When TerminalApp detects a change to "settings.json", it'll repeat the steps above. We could cache the result from - constructing an `AppSettings` from "defaults.json" data to improve performance. +When TerminalApp detects a change to settings.json, it'll repeat the steps above. We could cache the result from + constructing an `CascadiaSettings` from "defaults.json" data to improve performance. -Additionally, TerminalApp would reference `settings` for the following functionality... -- `Profiles` to populate the dropdown menu -- `Keybindings` to detect active key bindings and which actions they run -- `Commands` to populate the Command Palette -- `Warnings` to show a serialization warning, if any #### TerminalControl: Acquiring and Applying the Settings @@ -115,9 +127,9 @@ At the time of writing this spec, TerminalApp constructs `TerminalControl.Termin how to expose relevant settings to a TerminalControl instance. `TerminalSettings` (which implements `IControlSettings` and `ICoreSettings`) will be moved to - TerminalApp and act as a bridge connecting `AppSettings` to the TermControl. It will operate + TerminalApp and act as a bridge connecting `CascadiaSettings` to the TermControl. It will operate very similarly as it does today. On construction of the TermControl or hot-reload, - `TerminalSettings` will be constructed by copying the relevant values of `AppSettings`. + `TerminalSettings` will be constructed by copying the relevant values of `CascadiaSettings`. Then, it will be passed to TermControl (and TermCore by extension). @@ -163,14 +175,14 @@ The deserialization process takes place right after comparing the `settingsSourc ### TerminalSettings: passing by reference -`TermApp` synthesizes a `TerminalSettings` by copying the relevant values of `AppSettings`, +`TermApp` synthesizes a `TerminalSettings` by copying the relevant values of `CascadiaSettings`, then giving it to a Terminal Control. Some visual keybindings and interactions like ctrl+scroll and ctrl+shift+scroll to change the font size and acrylic opacity operate by directly modifying the value of the instantiated `TerminalSettings`. However, when a settings reload occurs, these instanced changes are lost. `TerminalSettings` can be used as a WinRT object that references (instead of copies) the relevant - values of `AppSettings`. This would prevent those instanced changes from being lost on a settings + values of `CascadiaSettings`. This would prevent those instanced changes from being lost on a settings reload. Since previewing commands like `setColorScheme` would require a clone of the existing `TerminalSettings`, @@ -184,13 +196,13 @@ Now, instead of overwriting the value, we need to override the reference to a co ### Layering Additional Settings As we begin to introduce more sources that affect the settings (via extensions or themes), we can introduce a `LayerSettings(String path)`. This layers the new settings file - onto the existing `AppSettings`. This is already done internally, we would just expose + onto the existing `CascadiaSettings`. This is already done internally, we would just expose it via C++/WinRT. ```c++ -runtimeclass AppSettings +runtimeclass CascadiaSettings { - // Load a settings file, and layer those changes on top of the existing AppSettings + // Load a settings file, and layer those changes on top of the existing CascadiaSettings void LayerSettings(String path); } ``` @@ -198,10 +210,10 @@ runtimeclass AppSettings ### Settings UI: Modifying and Applying the Settings (DRAFT) ```c++ -runtimeclass AppSettings +runtimeclass CascadiaSettings { - // Create a copy of the existing AppSettings - AppSettings Clone(); + // Create a copy of the existing CascadiaSettings + CascadiaSettings Clone(); // Compares object to "source" and applies changes to // the settings file at "outPath" @@ -209,9 +221,9 @@ runtimeclass AppSettings } ``` -The Settings UI will also have a reference to the `AppSettings settings` from TerminalApp - as `settingsSource`. When the Settings UI is opened up, the Settings UI will also have its own `AppSettings settingsClone` - that is a clone of TerminalApp's `AppSettings`. +The Settings UI will also have a reference to the `CascadiaSettings settings` from TerminalApp + as `settingsSource`. When the Settings UI is opened up, the Settings UI will also have its own `CascadiaSettings settingsClone` + that is a clone of TerminalApp's `CascadiaSettings`. ```c++ settingsClone = settingsSource.Clone() ``` @@ -221,7 +233,7 @@ As the user navigates the Settings UI, the relevant contents of `settingsClone` When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; this compares the changes between `settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. -As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `AppSettings`. +As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `CascadiaSettings`. Since the above triggers a change to `settings.json`, TerminalApp will also update itself. When something like this occurs, `settingsSource` will automatically be updated too. From c6bfb66ca120bc1e77e30a996d804230ef600d84 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 5 Oct 2020 10:13:22 -0700 Subject: [PATCH 11/12] address PR comments - Mike --- doc/specs/#885 - winrt Terminal Settings.md | 38 ++++++++++----------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/doc/specs/#885 - winrt Terminal Settings.md b/doc/specs/#885 - winrt Terminal Settings.md index 3c0d604a561..8a1f4be5f96 100644 --- a/doc/specs/#885 - winrt Terminal Settings.md +++ b/doc/specs/#885 - winrt Terminal Settings.md @@ -5,7 +5,7 @@ last updated: 2020-07-10 issue id: [#885](https://github.com/microsoft/terminal/issues/885) --- -# Spec Title +# Terminal Settings Model ## Abstract @@ -59,7 +59,7 @@ namespace TerminalApp // NOTE: It may be possible to move both of these to the constructor instead void SetDispatch(ShortcutActionDispatch dispatch); void SetKeyMap(KeyMapping keymap); - } + } } namespace TerminalSettingsModel @@ -85,12 +85,12 @@ Introducing these `Microsoft.Terminal.Settings.Model` WinRT objects also allow t for setting serialization. This will be moved into the `Microsoft.Terminal.Settings.Model` namespace too. Deserialization will be an extension of the existing `JsonUtils` `ConversionTrait` struct template. `ConversionTrait` - already includes `FromJson` and `CanConvert`. Deserialization would be handled by a `ToJson` function. + already includes `FromJson` and `CanConvert`. Serialization would be handled by a `ToJson` function. ### Terminal Settings Model: Warnings and Serialization Errors -Today, if the serialization of `CascadiaSettings` encounters any errors, an exception is thrown and caught/handled +Today, if the deserialization of `CascadiaSettings` encounters any errors, an exception is thrown and caught/handled by falling back to a simple `CascadiaSettings` object. However, WinRT does not support exceptions. To get around this issue, when `CascadiaSettings` encounters a serialization error, it must internally record @@ -104,7 +104,7 @@ To get around this issue, when `CascadiaSettings` encounters a serialization err TerminalApp will construct and reference a `CascadiaSettings settings` as follows: - TerminalApp will have a global reference to the "settings.json" filepath - construct an `CascadiaSettings` using `CascadiaSettings("settings.json")`. This builds an `CascadiaSettings` - from the "defaults.json" file data (which is already compiled as a string literal) + from the "defaults.json" file data (which is already compiled as a string literal) and layers the settings.json data on top of it. - check for errors/warnings, and handle them appropriately @@ -159,17 +159,7 @@ N/A ## Potential Issues -### Deserialization - -After deserializing the settings, injecting the new json into settings.json - should not remove the existing comments or formatting. - -The deserialization process takes place right after comparing the `settingsSource` and `settingsClone` objects. - For each setting found in the diff, we go to the relevant part of the JSON and see if the key is already there. - If it is, we update the value to be the one from `settingsClone`. Otherwise, we append the key/value pair - at the end of the section (much like we do with dynamic profiles in `profiles`). - - +N/A ## Future considerations @@ -214,7 +204,7 @@ runtimeclass CascadiaSettings { // Create a copy of the existing CascadiaSettings CascadiaSettings Clone(); - + // Compares object to "source" and applies changes to // the settings file at "outPath" void Save(String outPath); @@ -230,10 +220,10 @@ settingsClone = settingsSource.Clone() As the user navigates the Settings UI, the relevant contents of `settingsClone` will be retrieved and presented. As the user makes changes to the Settings UI, XAML will update `settingsClone` using XAML data binding. - When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; + When the user saves/applies the changes in the XAML, `settingsClone.Save("settings.json")` is called; this compares the changes between `settingsClone` and `settingsSource`, then injects the changes (if any) to `settings.json`. -As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `CascadiaSettings`. +As mentioned earlier, TerminalApp detects a change to "settings.json" to update its `CascadiaSettings`. Since the above triggers a change to `settings.json`, TerminalApp will also update itself. When something like this occurs, `settingsSource` will automatically be updated too. @@ -244,6 +234,16 @@ In the case that a user is simultaneously updating the settings file directly an **NOTE:** In the event that the user would want to export their current configuration, `Save` can be used to export the changes to a new file. + ### Reserialization (DRAFT) + +After deserializing the settings, injecting the new json into settings.json + should not remove the existing comments or formatting. + +The reserialization process takes place right after comparing the `settingsSource` and `settingsClone` objects. + For each setting found in the diff, we go to the relevant part of the JSON and see if the key is already there. + If it is, we update the value to be the one from `settingsClone`. Otherwise, we append the key/value pair + at the end of the section (much like we do with dynamic profiles in `profiles`). + ## Resources - [Preview Commands](https://github.com/microsoft/terminal/issues/6689) From 0f0356931406d20e011179625c2a72d254ef5380 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 5 Oct 2020 10:43:47 -0700 Subject: [PATCH 12/12] reserialization is a word --- .github/actions/spell-check/dictionary/dictionary.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spell-check/dictionary/dictionary.txt b/.github/actions/spell-check/dictionary/dictionary.txt index c01a75b1927..fe81616c71c 100644 --- a/.github/actions/spell-check/dictionary/dictionary.txt +++ b/.github/actions/spell-check/dictionary/dictionary.txt @@ -345227,6 +345227,7 @@ resequester resequestration reserate reserene +reserialization reserpine reserpinized reservable