-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft Spec for Cascading Default + User Settings #1258
Draft Spec for Cascading Default + User Settings #1258
Conversation
…scading-settings-spec
* Remove `hiddenProfiles` in favor of `profile.hidden` * Add info on how layering will work * add more powershell core info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this. It's great.
I have a huge question:
do we need a locally cascading default profile?
Here's what I'm thinking:
I want to set 100% of my profiles to cursorShape: pie
and cursorColor: raspberry
, but if I have more and more profiles that will get increasingly onerous when I then want to change cursorColor: blueberry
...
@carlos-zamora has some requests in this area too.
Fix simple typos Co-Authored-By: Dustin L. Howett (MSFT) <[email protected]>
* Try and make dynamic settings a bit clearer * more clearly call out serializing only what's different from a default- constructed `Profile` * Add more goals * add a blurb for user-default profile objects
…thub.com/Microsoft/Terminal into dev/migrie/s/754-cascading-settings-spec
Here is my model of settings, adopted form #1513. It's main goals are:
The things I didn't mention below remain as their are now or in above spec/discussion, like default settings. I'd make this an issue or PR but I was instructed to write it here.
|
This comment has been minimized.
This comment has been minimized.
…54-cascading-settings-spec
* Add updates concerning dynamic profile generation This is based on discussion with @DHowett-MSFT we had o*line. We're trying to work through a way to prevent dynamic profiles from roaming to machines the dynamic profiles might not exist on. After writing this up, I'm not totally sure that it's a better design. * Add some initial updates from discussion * Pushing some updates here. I haven't given it a once over to ensure it's all consistent but it's worth reviewing @DHowett-MSFT * Some minor updates from Dustin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec is very detailed (which is great for what we need right now), but I definitely think we need some kind of easily accessible tutorial or user docs when we release this feature.
I'm also confused by |
@carlos-zamora you better believe there's going to be a much easier doc for this when it comes out. This is more the technical implementation details, when I make an actual PR for it, I'll make sure to include a settings-howto.md with a user-friendly explanation of how the layering works. Theoretically, everything the user currently has will still work, though some may become redundant. Future user profiles will be simpler to read, with the minimum set of key-value pairs in them. |
Just my €0.02: IMVHO this has lower priority than #1770. Of course, the full settings experience needs both. |
@miniksa @DHowett-MSFT can I get some additional passes on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with this. I just left a few nits.
Co-Authored-By: Michael Niksa <[email protected]>
Summary of the Pull Request
Adds a spec describing how a cascading settings model should work. This spec describes a model where there are two sets of settings, "Default" and "User" settings, and the two are layered to create the runtime settings.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Just read the spec.