Add support for "User Default" settings #3369
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are settings that apply to _every_ profile, before user customizations. This can be placed in the globals under the "defaultSettings" key. It's a blob of profile settings that will be parsed just like a profile, and applied to profiles after default&dynamic profiles, but before user profiles.
zadjii-msft
added
Area-Settings
DHowett-MSFT
approved these changes
Oct 30, 2019
ghost
requested review from
miniksa
approved these changes
Oct 31, 2019
doc/cascadia/SettingsSchema.md
Outdated
| @@ -8,6 +8,7 @@ Properties listed below affect the entire window, regardless of the profile sett | |||
| | `alwaysShowTabs` | _Required_ | Boolean | `true` | When set to `true`, tabs are always displayed. When set to `false` and `showTabsInTitlebar` is set to `false`, tabs only appear after typing <kbd>Ctrl</kbd> + <kbd>T</kbd>. | | |||
| | `copyOnSelect` | Optional | Boolean | `false` | When set to `true`, a selection is immediately copied to your clipboard upon creation. When set to `false`, the selection persists and awaits further action. | | |||
| | `defaultProfile` | _Required_ | String | PowerShell guid | Sets the default profile. Opens by typing <kbd>Ctrl</kbd> + <kbd>T</kbd> or by clicking the '+' icon. The guid of the desired default profile is used as the value. | | |||
| | `defaultSettings` | Optional | Object (Profile) | | Default settings to use with each profile. Settings in each profile in profiles will override these values. | | |||
There was a problem hiding this comment.
This description is clear as mud.
I think you mean "This set of settings is at the bottom of the layer cake, if defined, and those in the individual profiles will cover these up, if specified."
We're starting to need some sort of diagram to explain settings here. Perhaps a cake-based one.
carlos-zamora
approved these changes
Oct 31, 2019
DHowett-MSFT
suggested changes
Oct 31, 2019
ghost
added
the
Needs-Author-Feedback
ghost
removed
the
Needs-Author-Feedback
|
There's been quite the discussion on the team regarding this issue. There doesn't seem to be any sort of consensus on naming or how exactly this should work, so I'm closing this in favor of having a proper full spec review. Functionally, the code change is simple, as seen here. Naming, however, is a Hard problem. |
4 tasks
ghost
pushed a commit
that referenced
this pull request
Dec 11, 2019
## 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 a commit
that referenced
this pull request
Dec 11, 2019
Refer to the original issue: **Default Profile for Common Profile Settings** #2325 So this is my summary of everything we discussed regarding "default profile settings". The original PR was #3369, but we were _not_ in agreement on the UX, so this PR is for discussion about that. I put forth 4 proposals that were mentioned in the discussion. In the discussion that followed, we decided the 3rd proposal was the best. The doc reflects that choice.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
These are settings that apply to every profile, before user customizations. This can be placed in the globals under the "defaultSettings" key. It's a blob of profile settings that will be parsed just like a profile, and applied to profiles after default&dynamic profiles, but before user profiles.
"defaultSettings" is kinda a placeholder key ATM - @cinnamon-msft do we have concerns with this name? Does it make enough sense?
References
#2515, #2603
PR Checklist
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.
Validation Steps Performed
ran the tests