Proto extensions spec #7584
Proto extensions spec #7584
Conversation
|
|
||
| Opening a profile causes its commandline argument to be automatically run. Thus, if malicious modifications are made | ||
| to existing profiles or new profiles with malicious commandline arguments are added, users could be tricked into running | ||
| things they do not want to. |
There was a problem hiding this comment.
This could already happen even now though (someone could replace the settings file), so I don't think this adds that much risk
ghost
added
the
Needs-Author-Feedback
ghost
removed
the
Needs-Author-Feedback
| } | ||
| ``` | ||
|
|
||
| This stub will *not* show up in the users settings file, similar to the way our default colour schemes do not show up. |
There was a problem hiding this comment.
(thinking out loud, not necessarily for the spec) This'll be interesting for the Settings UI, because it's valid to reference this color scheme, but not modify it. The Color Schemes page may have to either...
- remove ColorSchemes from stubs
- present them, but put a note that it's imported and not modifiable
- present them, but when the user modifies one, the modified version gets copied to the settings.json
There was a problem hiding this comment.
None of the current colour schemes we have show up in the settings file though, how does the settings UI deal with those?
There was a problem hiding this comment.
Ah. So GlobalAppSettings currently has a reference to all of the color schemes. This can't be changed because that's how we look up what a color scheme name actually maps to. The Settings UI reads/edits that instead of the actual JSON directly.
So instead, we need a way to mark where each color scheme came from (like source in Profile). That way, when the Settings UI presents the Color Schemes page, we handle stub-generated color schemes differently (the list of ideas above).
We could also introduce another top-level navigation item that lets users browse/copy/modify anything generated from stubs. But I'm less of a fan of that idea because it breaks up where all of the color schemes can be found.
ghost
added
Needs-Author-Feedback
|
@sirredbeard hey! this is related to the discussion we had about multipass being able to produce Windows Terminal profiles at runtime. Do you want to loop a couple people in to see if it fits their needs? review link here |
|
Hey I know I already signed off on this, however, what if an application wanted to be able to add actions? Could we also allow applications to add commands? |
Implementation wise it'll probably just be one more list in the json file we will have to layer, but I'm a little uneasy about allowing commands because that is directly workflow-related. Maybe we treat commands similar to colour schemes? I.e. we allow additions of new ones but no modifications to the existing ones. |
That seems reasonable to me. I suppose this was more of a showerthought - we should probably combine that showerthought into the design for "global action IDs" #6899/#7175. Maybe they can create new actions with new unique IDs, but not update existing IDs? Then, users will be able to bind those actions to keybindings using the IDs the extension app set. Feel free to ship this spec without it and follow up separately |
Yeah as its not much of a design change with regards to proto extensions, I'd much rather ship this and #7632 first, work the kinks out, and then add actions as an additional feature |
|
@malxau as somebody who offers a 3p shell that might want to carry some Terminal configuration (optionally) does this read as reasonable to you? The long/short of it is that a non-Terminal party could drop a JSON fragment expressing profile details (commandline, color scheme, etc.) that'll get picked up by Terminal on next launch. review link here |
|
@DHowett I'll try to read this in more detail tomorrow, but at least superficially, this looks wonderful. (One of the problems I've had is the color palette. The colors I used looked fine in the traditional palette, but don't look so good on newer defaults. So being able to drop a JSON file that adds a profile entry, points to wherever binaries are, and defaults to a palette that works with the colors in use makes a lot of sense. Obviously the user has the last say in all of these things in the end, it's just providing straightforward defaults.) |
|
I am catching up on this now and circulating among our teams internally. |
PankajBhojwani
added
the
AutoMerge
|
Hello @PankajBhojwani! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
|
@msftbot merge this in 27 hours |
|
Hello @PankajBhojwani! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
|
OOP |
Support for fragment extensions, according to the implementation outlined in #7584 (which calls them proto extensions.) See #7584 for more information. ## Validation Steps Performed Self-testing by creating the folder `%LOCALAPPDATA%\Microsoft\Windows Terminal\Fragments` and adding a json file into it to modify and add profiles Also self-tested with an app extension Closes #1690
Summary of the Pull Request
Proto-extensions spec
PR Checklist