-
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
Default Profile for Common Profile Settings #2325
Comments
Sounds like subset of #1258 (comment) |
Tagging @zadjii-msft for ideas regarding Cascading Settings. Personally, I think that the current spec covers this functionality in a cleaner way, but @zadjii-msft might catch something I'm missing here. :) |
So I'm gonna leave this open (unless there's another issue we've got already tracking this bit). This is one of the things I refer to in the #1258 spec, but I specifically don't cover to try and keep that spec as atomic as possible. I absolutely want to be able to include a This is probably something that'll be spec'd up once #754 is more ironed out. |
@zadjii-msft this probably only needs a very light spec, if one at all. Proposal to change type to |
Done. I might just skip the spec tbh. I'll prototype and feel out if it really needs the spec, I think we all understand what we want from this feature. |
Let's have a discussion: Where should these settings be layered? For defaults, dynamic profiles, should these "global default" settings be applied:
@microsoft/windows-console-team thoughts? Hypothetical settings: {
"$schema": "https://aka.ms/terminal-profiles-schema",
"defaultProfile": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
"defaultSettings":
{
"padding" : "2",
"useAcrylic" : true,
"fontFace" : "Cascadia Code",
"icon" : "myIcon.png"
},
"profiles":
[
{
// Make changes here to the powershell.exe profile
"guid": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
"name": "Windows PowerShell",
"commandline": "powershell.exe",
"hidden": false
},
{
// Make changes here to the cmd.exe profile
"guid": "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}",
"name": "cmd",
"commandline": "cmd.exe",
"hidden": false
},
{
"guid": "{c6eaf9f4-32a7-5fdc-b5cf-066e8a4b1e40}",
"hidden": false,
"name": "Ubuntu-18.04",
// recall that the WSL DPG sets the icon to the tux png.
"source": "Windows.Terminal.Wsl"
},
],
"schemes": [],
"keybindings": []
} If we do 1, then If we do 2, then the If we do 3, then the |
2 feels right to me- how would the user say “I want Cascadia Code everywhere” if the Profile constructor or the Default profiles set it to Consolas? The user’s preferred defaults IMO trump any app generated data, so if they want to set a default icon it’s on them? I can see why that would not be ideal. To properly handle option 1 we would need to remove everything from defaults.json that is also matched by the constructor, and that reduces the absolute value of defaults.json. |
If you do 3, is there any way to override the icon for the DPG or is it now toast? I think it's either 2 or 3. |
With both 2 and 3, we can still override the icon from a DPG by setting the icon within the profile manually. The other profiles in |
In that case, I think if you expressly set an icon at this user-set-defaults level, it should kill the DPG icons unless you again go further and override one of the DPGs to have an icon. So that's a 2, I believe. |
2 it is then :) Thanks all for the pseudo-spec review. We can debate naming in the PR, and if I run into any other issues I'll ping this thread. Though, that won't be till at least the end of the month :P |
I don't know if this is related for sure, but build 0.5.2762.0 from the store on a fresh profile resulted in a near empty settings json with only profiles information. The settings for everything else is seemingly gone. |
@jwhipp That's the point! You should give a closer read to the settings file -- it explains what to do and what you should be looking for. 😄 |
I see... my bad. My eyes immediately started scanning for the portions I wanted to edit and I didn't even notice that comment line. |
## 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_
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.
🎉This issue was addressed in #3892, which has now been successfully released as Handy links: |
Default Profile for Common Profile Settings
Allow settings which the user would like applied to all profiles to be specified in a profile with a well-known name, such as
"name" : "_default"
. All settings in this profile will be applied before they are overridden from individual profiles. This would be helpful to allow all profiles to have acrylic applied with a certain opacity where those settings are not specified.The text was updated successfully, but these errors were encountered: