Add Dynamic Profile Generators #2603

zadjii-msft · 2019-08-29T15:17:55Z

This PR targets the #2515 PR. It does that for the sake of diffing. When this PR and #2515 are both ready, I'll merge #2515 first, then change the target of this branch, and merge this one.

Summary of the Pull Request

This PR adds support for "dynamic profiles", in accordance with the Cascading Settings Spec. Currently, we have three types of default profiles that fit the category of dynamic profile generators. These are profiles that we want to create on behalf of the user, but require runtime information to be able to create correctly. Because they require runtime information, we can't ship a static version of these profiles as a part of defaults.json. These three profile generators are:

The Powershell Core generator
The WSL Distro generator
The Azure Cloud Shell generator

References

PR Checklist

Closes Feature Request: Cascading settings (with a hardcoded default.json?) #754
I work here
look at all these Tests
Requires documentation to be updated - This is done as part of the parent PR

Detailed Description of the Pull Request / Additional comments

We want to be able to enable the user to edit dynamic profiles that are generated from DPGs. When dynamic profiles are added, we'll add entries for them to the user's profiles.json. We do this without re-serializing the settings. Instead, we insert a partial serialization for the profile into the user's settings.

Remaining TODOs:

Make sure that dynamic profiles appear in the right place in the order of profiles -> Ensure that dynamic profiles have a consistent place in the profiles list #2722
don't serialize the colorTable key for dynamic profiles.
re-parse the user settings string if we've changed it.
Handle changing the default profile to pwsh if it exists on first launch, or file a follow-up issue -> On first launch, if Powershell Core is installed, set it as the default profile #2721

Validation Steps Performed

* Moves tests into individual classes * adds support for layering a colorscheme on top of another

…yer-settings

must have committed without staging this change, uh oh. Not like those tests actually work so nbd

This is like 80% of #754. Needs tests.

* add support to unbind a key with `null` or `"unbound"` or `"garbage"`

In the end, I think we need to ask _was this worth it_

Add a MsBuild target to auto-generate a header with the defaults.json as a string in the file. That way, we can _always_ load the defaults. Literally impossible to not.

…yer-settings

src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp

…mic-profiles

carlos-zamora

Mostly found some minor spelling errors. Two minor questions on the tests.

src/cascadia/ut_app/DynamicProfileTests.cpp

carlos-zamora · 2019-09-13T16:19:26Z

src/cascadia/ut_app/DynamicProfileTests.cpp

+        VERIFY_IS_TRUE(settings._profiles.at(3)._guid.has_value());
+        VERIFY_IS_TRUE(settings._profiles.at(4)._guid.has_value());
+
+        VERIFY_ARE_NOT_EQUAL(settings._profiles.at(0)._guid.value(),


Here we're verifying that all of the GUIDs are unique right? Why not loop through every possible combination to ensure that? (or, better yet, push all of the guids into a set and check if the GUID actually got inserted)

src/cascadia/ut_app/DynamicProfileTests.cpp

src/cascadia/TerminalApp/Profile.cpp

src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp

Co-Authored-By: Carlos Zamora <[email protected]>

…crosoft/Terminal into dev/migrie/f/dynamic-profiles

…mic-profiles

DHowett-MSFT · 2019-09-16T19:45:59Z

src/cascadia/TerminalApp/PowershellCoreProfileGenerator.cpp

+        pwshProfile.SetColorScheme({ L"Campbell" });
+
+        // If powershell core is installed, we'll use that as the default.
+        // Otherwise, we'll use normal Windows Powershell as the default.


comment is dead

This PR represents the start of the work on Cascading User + default settings, #754. Cascading settings will be done in two parts: * [ ] Layered Default+User settings (this PR) * [ ] Dynamic Profile Generation (#2603). Until _both_ are done, _neither are going in. The dynamic profiles PR will target this PR when it's ready, but will go in as a separate commit into master. This PR covers adding one primary feature: the settings are now in two separate files: * a static `defaults.json` that ships with the package (the "default settings") * a `profiles.json` with the user's customizations (the "user settings) User settings are _layered_ upon the settings in the defaults settings. ## References Other things that might be related here: * #1378 - This seems like it's definitely fixed. The default keybindings are _much_ cleaner, and without the save-on-load behavior, the user's keybindings will be left in a good state * #1398 - This might have honestly been solved by #2475 ## PR Checklist * [x] Closes #754 * [x] Closes #1378 * [x] Closes #2566 * [x] I work here * [x] Tests added/passed * [x] Requires documentation to be updated - it **ABSOLUTELY DOES** ## Detailed Description of the Pull Request / Additional comments 1. We start by taking all of the `FromJson` functions in Profile, ColorScheme, Globals, etc, and converting them to `LayerJson` methods. These are effectively the same, with the change that instead of building a new object, they are simply layering the values on top of `this` object. 2. Next, we add tests for layering properties like that. 3. Now, we add a `defaults.json` to the package. This is the file the users can refer to as our default settings. 4. We then take that `defaults.json` and stamp it into an auto generated `.h` file, so we can use it's data without having to worry about reading it from disk. 5. We then change the `LoadAll` function in `CascadiaSettings`. Now, the function does two loads - one from the defaults, and then a second load from the `profiles.json` file, layering the settings from each source upon the previous values. 6. If the `profiles.json` file doesn't exist, we'll create it from a hardcoded `userDefaults.json`, which is stamped in similar to how `defaults.json` is. 7. We also add support for _unbinding_ keybindings that might exist in the `defaults.json`, but the user doesn't want to be bound to anything. 8. We add support for _hiding_ a profile, which is useful if a user doesn't want one of the default profiles to appear in the list of profiles. ## TODO: * [x] Still need to make Alt+Click work on the settings button * [x] Need to write some user documentation on how the new settings model works * [x] Fix the pair of tests I broke (re: Duplicate profiles) <hr> * Create profiles by layering them * Update test to layer multiple times on the same profile * Add support for layering an array of profiles, but break a couple tests * Add a defaults.json to the package * Layer colorschemes * Moves tests into individual classes * adds support for layering a colorscheme on top of another * Layer an array of color schemes * oh no, this was missed with #2481 must have committed without staging this change, uh oh. Not like those tests actually work so nbd * Layer keybindings * Read settings from defaults.json + profiles.json, layer appropriately This is like 80% of #754. Needs tests. * Add tests for keybindings * add support to unbind a key with `null` or `"unbound"` or `"garbage"` * Layer or clear optional properties * Add a helper to get an optional variable for a bunch of different types In the end, I think we need to ask _was this worth it_ * Do this with the stretch mode too * Add back in the GUID check for profiles * Add some tests for global settings layering * M A D W I T H P O W E R Add a MsBuild target to auto-generate a header with the defaults.json as a string in the file. That way, we can _always_ load the defaults. Literally impossible to not. * When the user's profile.json doesn't exist, create it from a template * Re-order profiles to match the order set in the user's profiles.json * Add tests for re-ordering profiles to match user ordering * Add support for hiding profiles using `"hidden": true` * Use the hardcoded defaults.json for the exception->"use defaults" case * Somehow I messed up the git submodules? * woo documentation * Fix a Terminal.App.Unit.Tests failure * signed/unsigned is hard * Use Alt+Settings button to open the default settings * Missed a signed/unsigned * Some very preliminary PR feedback * More PR feedback Use the wil helper for the exe path Move jsonutils into their own file kill some dead code * Add templates to these bois * remove some code for generating defaults, reorder defaults.json a tad * Make guid a std::optional * Large block of PR feedback * Remove some dead code * add some comments * tag some todos * stl is love, stl is life * add `-noprofile` * Fix the crash that dustin found * -Encoding ASCII * Set a profile's default scheme to Campbell * Fix the tests I regressed * Update UsingJsonSetting.md to reflect that changes from these PRs * Change how GenerateGuidForProfile works * Make AppKeyBindings do its own serialization * Remove leftover dead code from the previous commit * Fix up an enormous number of PR nits * Fix a typo; Update the defaults to match #2378 * Tiny nits * Some typos, PR nits * Fix this broken defaults case

…c-profiles # Conflicts: # src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp # src/cascadia/TerminalApp/CascadiaSettings.cpp # src/cascadia/TerminalApp/CascadiaSettings.h # src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp # src/cascadia/TerminalApp/Profile.cpp # src/cascadia/TerminalApp/Profile.h # src/cascadia/ut_app/JsonTests.cpp

ghost · 2019-09-24T16:27:17Z

🎉Windows Terminal Preview v0.5.2661.0 has been released which incorporates this pull request.:tada:

Handy links:

## Summary of the Pull Request _This is attempt 2 at this feature_. The original PR can be found at #3369. These are settings that apply to _every_ profile, before user customizations. If the user wants to add "default profile settings", they can make the `"profiles"` property an _object_, instead of a list, and add `"defaults"` key underneath that object. The users list of profiles should then be under the `list` property of the `profiles` object. ## References #2515, #2603, #3369, #3569 ## PR Checklist * [x] Closes #2325 * [x] I work here * [x] Tests added/passed * [x] schema, docs updated ## Detailed Description of the Pull Request / Additional comments ~~Discussion in #2325 itself serves as the "spec" for this task. I thought we'd need more discussion on the topic, but it ended up being pretty straightforward.~~ I should not have said that in the original PR. We've had a better spec review now that I think we're happier with. ## Validation Steps Performed _ran the tests_

zadjii-msft added 30 commits August 20, 2019 09:51

Create profiles by layering them

858341f

Update test to layer multiple times on the same profile

2074ac2

Add support for layering an array of profiles, but break a couple tests

deb135f

Add a defaults.json to the package

e524ea7

Layer colorschemes

8157fd6

* Moves tests into individual classes * adds support for layering a colorscheme on top of another

Layer an array of color schemes

16336bb

Merge remote-tracking branch 'origin/master' into dev/migrie/f/754-la…

870fed3

…yer-settings

oh no, this was missed with #2481

22e375d

must have committed without staging this change, uh oh. Not like those tests actually work so nbd

Layer keybindings

4e3c3c3

Read settings from defaults.json + profiles.json, layer appropriately

2c2fa14

This is like 80% of #754. Needs tests.

Add tests for keybindings

9bcf0c5

* add support to unbind a key with `null` or `"unbound"` or `"garbage"`

Layer or clear optional properties

c6b80b9

Add a helper to get an optional variable for a bunch of different types

c040356

In the end, I think we need to ask _was this worth it_

Do this with the stretch mode too

51bcf11

Add back in the GUID check for profiles

8b938ff

Add some tests for global settings layering

aec4c51

M A D W I T H P O W E R

1d662ca

Add a MsBuild target to auto-generate a header with the defaults.json as a string in the file. That way, we can _always_ load the defaults. Literally impossible to not.

When the user's profile.json doesn't exist, create it from a template

9a14dab

Re-order profiles to match the order set in the user's profiles.json

82b57f8

Add tests for re-ordering profiles to match user ordering

2084f89

Add support for hiding profiles using "hidden": true

e07c1ba

Use the hardcoded defaults.json for the exception->"use defaults" case

3ab0f25

Merge remote-tracking branch 'origin/master' into dev/migrie/f/754-la…

39b7fe1

…yer-settings

Somehow I messed up the git submodules?

9b6a2ee

woo documentation

be8e858

Fix a Terminal.App.Unit.Tests failure

7dcb179

signed/unsigned is hard

7953e8c

Use Alt+Settings button to open the default settings

2032b96

Missed a signed/unsigned

bf792e8

Start dynamically creating profiles

caeaa34

zadjii-msft requested review from carlos-zamora and DHowett-MSFT September 11, 2019 22:44

DHowett-MSFT reviewed Sep 11, 2019

View reviewed changes

src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp Outdated Show resolved Hide resolved

zadjii-msft added 3 commits September 11, 2019 16:09

Tiny nits

fed6492

Merge branch 'dev/migrie/f/754-layer-settings' into dev/migrie/f/dyna…

4ce2733

…mic-profiles

In-den-taition!

ad335ef

zadjii-msft requested a review from DHowett-MSFT September 12, 2019 16:59

zadjii-msft added 2 commits September 13, 2019 09:14

Some typos, PR nits

d1e98b6

Merge branch 'dev/migrie/f/754-layer-settings' into dev/migrie/f/dyna…

220ed31

…mic-profiles

carlos-zamora approved these changes Sep 13, 2019

View reviewed changes

DHowett-MSFT suggested changes Sep 13, 2019

View reviewed changes

src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp Show resolved Hide resolved

src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp Show resolved Hide resolved

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 13, 2019

zadjii-msft and others added 2 commits September 16, 2019 08:06

Fix this broken defaults case

e454753

Apply suggestions from code review

8bdf405

Co-Authored-By: Carlos Zamora <[email protected]>

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 16, 2019

zadjii-msft added 3 commits September 16, 2019 08:14

Merge branch 'dev/migrie/f/dynamic-profiles' of https://github.com/Mi…

11051fd

…crosoft/Terminal into dev/migrie/f/dynamic-profiles

Merge branch 'dev/migrie/f/754-layer-settings' into dev/migrie/f/dyna…

12ad666

…mic-profiles

PR nits

00efd9f

DHowett-MSFT approved these changes Sep 16, 2019

View reviewed changes

zadjii-msft changed the base branch from dev/migrie/f/754-layer-settings to master September 16, 2019 19:57

zadjii-msft merged commit 0df0234 into master Sep 16, 2019

zadjii-msft mentioned this pull request Sep 17, 2019

Don't explode name-only profiles #2789

Merged

3 tasks

ghost mentioned this pull request Sep 24, 2019

Feature Request: Cascading settings (with a hardcoded default.json?) #754

Closed

42 tasks

zadjii-msft mentioned this pull request Oct 29, 2019

Add support for "User Default" settings #3369

Closed

4 tasks

zadjii-msft mentioned this pull request Dec 9, 2019

Add support for "User Default" settings II #3892

Merged

4 tasks

zadjii-msft deleted the dev/migrie/f/dynamic-profiles branch January 31, 2020 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Dynamic Profile Generators #2603

Add Dynamic Profile Generators #2603

zadjii-msft commented Aug 29, 2019 •

edited

Loading

carlos-zamora left a comment

carlos-zamora Sep 13, 2019

DHowett-MSFT Sep 16, 2019

ghost commented Sep 24, 2019

Add Dynamic Profile Generators #2603

Add Dynamic Profile Generators #2603

Conversation

zadjii-msft commented Aug 29, 2019 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Remaining TODOs:

Validation Steps Performed

carlos-zamora left a comment

Choose a reason for hiding this comment

carlos-zamora Sep 13, 2019

Choose a reason for hiding this comment

DHowett-MSFT Sep 16, 2019

Choose a reason for hiding this comment

ghost commented Sep 24, 2019

zadjii-msft commented Aug 29, 2019 •

edited

Loading