Skip to content
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

Proto extensions spec #7584

Merged
29 commits merged into from
Nov 5, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9345e28
Proto extensions spec (draft)
PankajBhojwani Sep 9, 2020
667ee27
edits
PankajBhojwani Sep 9, 2020
969593e
examples
PankajBhojwani Sep 10, 2020
56d6892
good bot
PankajBhojwani Sep 10, 2020
f56a0b0
additional potential issue
PankajBhojwani Sep 10, 2020
1c196db
more details and clarity
PankajBhojwani Sep 11, 2020
6024b44
spell
PankajBhojwani Sep 11, 2020
e406c98
small fix
PankajBhojwani Sep 11, 2020
56d351d
forgot to update this
PankajBhojwani Sep 11, 2020
e388c07
small clarification
PankajBhojwani Sep 14, 2020
7d3af2f
don't need full profiles to provide a GUID
PankajBhojwani Sep 14, 2020
b864074
stubs must be put into lists
PankajBhojwani Sep 16, 2020
54981db
example of a full file
PankajBhojwani Sep 16, 2020
08d7731
formatting
PankajBhojwani Sep 16, 2020
4ccee5c
update folders
PankajBhojwani Sep 16, 2020
32569c7
specifics for app extensions
PankajBhojwani Sep 18, 2020
a34f7d0
no need for properties, change last updated date
PankajBhojwani Sep 18, 2020
95f2dc7
don't require displayname and description
PankajBhojwani Sep 18, 2020
202951f
updates from discussion
PankajBhojwani Sep 21, 2020
4c28d99
clarifications
PankajBhojwani Oct 2, 2020
4c81790
fragments subdirectory for app extensions
PankajBhojwani Oct 5, 2020
5472357
one more future consideration
PankajBhojwani Oct 5, 2020
1a4f739
change extension host name
PankajBhojwani Oct 7, 2020
01c3263
edits based on discussion
PankajBhojwani Oct 9, 2020
985a63d
naming fix
PankajBhojwani Oct 15, 2020
46c0dc3
Merge branch 'master' of https://github.com/microsoft/terminal into d…
PankajBhojwani Oct 15, 2020
1395539
dictionary, merge master
PankajBhojwani Oct 15, 2020
5660873
another naming fix
PankajBhojwani Oct 15, 2020
64666b2
linter.
PankajBhojwani Oct 15, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spell-check/dictionary/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ llabs
LCID
lround
LSHIFT
msappx
NCHITTEST
NCLBUTTONDBLCLK
NCRBUTTONDBLCLK
Expand Down
1 change: 1 addition & 0 deletions .github/actions/spell-check/dictionary/dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -313153,6 +313153,7 @@ post-Mishnical
postmistress
postmistresses
postmistress-ship
postmodern
postmortal
post-mortem
postmortem
Expand Down
251 changes: 251 additions & 0 deletions doc/specs/Proto extensions-spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
---
author: <Pankaj> <Bhojwani> <@PankajBhojwani>
created on: <2020-9-9>
last updated: <2020-9-9>
---

# Proto extensions

## Abstract

This spec outlines adding support for proto extensions. This would allow other apps/programs
to add json snippets to our json files, and will be used when we generate settings for our various profiles.

## Inspiration

### Goal: Allow programs to have a say in the settings for their profiles

Currently, Ubuntu/WSL/Powershell etc are unable to provide any specifications for how they want
their profiles in Terminal to look - only we and the users can modify the profiles. We want to provide
these installations with the functionality to have a say in this, allowing them to specify things like
their icon, their font and so on. However, we want to maintain that the user has final say over all of
these settings.

## Solution Design

Currently, when we load the settings we perform the following steps (this is a simplified description,
for the full version see the [spec for cascading default + user settings](https://github.com/microsoft/terminal/blob/master/doc/specs/%23754%20-%20Cascading%20Default%20Settings.md)):

1. Generate profiles from the defaults file
2. Generate profiles from the dynamic profile generators
3. Layer the user settings on top of all the profiles created in steps 1 and 2
4. Validate the settings

To allow for installations to add in their snippets of json, I propose the addition of a new step
in between 2 and 3:

1. Generate profiles from the defaults file
2. Generate profiles from the dynamic profile generators
3. **Incorporate the additional provided json stubs** - these stubs could be:
1. modifications to existing profiles
2. additions of full profiles
3. additions of colour schemes
4. Layer the user settings on top of all the profiles created in steps 1 through 3
5. Validate the settings

### Specifications of the json stubs

As written above, the json stubs could be modifications to existing profiles, additions to full profiles, or additions of colour schemes.

#### Modifications to existing profiles
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

The main thing to note for modification of existing profiles is that this will only be used for modifying the
default profiles (cmd/powershell) or the dynamically generated profiles.

For modifications to existing profiles, the json stub would need to indicate which profile it wishes to modify.
Note that currently, we generate a GUID for dynamic profiles using the "initial" name of the profile (i.e. before
any user changes are applied). For example, the "initial" name of a WSL profile is the \<name\> argument to
"wsl.exe -d \<name\>", and the "initial" name of a Powershell profile is something like "Powershell (ARM)" - depending
on the version. Thus, the stub creator could simply use the same uuidv5 GUID generator we do on that name to obtain the
GUID. Then, in the stub they provide the GUID can be used to identify which profile to modify.

Naturally, we will have to provide documentation on how they could generate the desired GUID themselves. In that documentation,
we will also provide examples showing how the current GUIDs are generated for clarity's sake.

We might run into the case where multiple json stubs modify the same profile and so they override each other. For the initial implementation, we
are simply going to apply _all_ the changes. Eventually, we will probably want some sort of hierarchy to determine
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
an order to which changes are applied.
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

Here is an example of a json stub that modifies an existing profile (specifically the Azure cloud shell profile):

```js
{
"guid": "{b453ae62-4e3d-5e58-b989-0a998ec441b8}",
"fontSize": 16,
"fontWeight": "thin"
}
```

**NOTE**: This will *not* change the way the profile looks in the user's settings file. The Azure cloud shell profile
in their settings file (assuming they have made no changes) will still be as below.

```js
{
"guid": "{b453ae62-4e3d-5e58-b989-0a998ec441b8}",
"hidden": false,
"name": "Azure Cloud Shell",
"source": "Windows.Terminal.Azure"
}
```
However, the user is free to make changes to it as usual and those changes will have the 'final say'.
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

#### Full profile stubs
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

Technically, full profile stubs do not need to contain anything (they could just be '\{\}'). However we should
have some qualifying minimum criteria before we accept a stub as a full profile. I suggest that we only create
new profile objects from stubs that contain at least the following

* A name
* A commandline argument
* A unique GUID - this is so we avoid conflicts if several different creators name their profile the same thing

As in the case of the dynamic profile generator, if we create a profile that did not exist before (i.e. does not
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
exist in the user settings), we need to add the profile to the user settings file and re-save that file.

Furthermore, we will add a source field to profiles created this way (again, similar to what we do for dynamic profiles).
The source field value is dependent on how we obtained this json file, and will be touched on in more detail later in the
spec (see "Creation and location(s) of the json files" - "The source field").
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

Here is an example of a json stub that contains a full profile:

```js
{
"guid": "{a821ae62-9d4a-3e34-b989-0a998ec283e6}",
"name": "Cool Profile",
"commandline": "powershell.exe",
"antialiasingMode": "aliased",
"fontWeight": "bold",
"scrollbarState": "hidden"
}
```

When this profile gets added back to the user's settings file, it will look similar to the way we currently add
new dynamic profiles to the user's settings file. Going along with the example above, the corresponding
json stub in the user's settings file will be:

```js
{
"guid": "{a821ae62-9d4a-3e34-b989-0a998ec283e6}",
"name": "Cool Profile",
"hidden": "false",
"source": "local"
}
```
Again, the user will be able to make changes to it as they do for all other profiles.

#### Creation of colour schemes

As with full profiles, we will have some qualifying criteria for what we accept as full colour schemes: color schemes
must have
* A name
* A background
* A foreground
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

This will cause the issue of colour schemes being overridden if there are many creations of a colour scheme with the
same name. Since for now all we have to uniquely identify colour schemes *is* the name, we will just let this be.

Here is an example of a json stub that contains a colour scheme:

```js
{
"name": "Postmodern Tango Light",
"background": '#61D6D6',
"foreground": '#E74856'
}
```

This stub will *not* show up in the users settings file, similar to the way our default colour schemes do not show up.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the current colour schemes we have show up in the settings file though, how does the settings UI deal with those?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


### Creation and location(s) of the json files

#### For apps installed through Microsoft store (or similar)

For apps that are installed through something like the Microsoft Store, they should add their json file(s) to
an app extension. During our profile generation, we will probe the OS for app extensions of this type that it
knows about and obtain the json files or objects. These apps should should use msappx for us to know about
their extensions.
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

#### For apps installed 'traditionally' and third parties/independent users

For apps that are installed 'traditionally', there are 2 cases. The first is that this installation is for all
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
the users of the system - in this case, the installer should add their json files to the global folder:

```
C:\ProgramData\Microsoft\Windows\Terminal
```

Note: `C:\ProgramData` is a [known folder](https://docs.microsoft.com/en-us/previous-versions/windows/desktop/legacy/bb776911(v=vs.85))
that apps should be able to access. It will be on us to create the folder `Terminal` in `C:\ProgramData\Microsoft\Windows` for this to happen.

In the second case, the installation is only for the current user. For this case, the installer should add the
json files to the local folder:

```C:\Users\<user>\AppData\Local\Microsoft\Windows\Terminal```

This is also where independent users will add their own json files for Terminal to generate/modify profiles.

We will look through both folders mentioned above during profile generation.
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

#### The source field

Currently, we allow users an easy way to disable/enable profiles that are generated from certain sources. For
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
example, a user can easily hide all dynamic profiles with the source `"Windows.Terminal.Wsl"` if they wish to.
To retain this functionality, we will add source fields to profiles we create through proto-extensions.

For full profiles that came from the *global* folder `C:\ProgramData\Microsoft\Windows\Terminal`,
we will give the value `global` to the source field.

For full profiles that came from the *local* folder `C:\Users\<user>\AppData\Local\Microsoft\Windows\Terminal`,
we will give the value `local` to the source field.

For full profiles that came from app extensions, we will give the value `app` to the source field.



## UI/UX Design
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

This feature will allow other installations a level of control over how their profiles look in Terminal. For example,
if Ubuntu gets a new icon or a new font they can have those changes be reflected in Terminal users' Ubuntu profiles.

Furthermore, this allows users an easy way to share profiles they have created - instead of needing to modify their
settings file directly they could simply download a json file into the folder
`C:\Users\<user>\AppData\Local\Microsoft\Windows\Terminal`.

## Capabilities

### Accessibility

This change should not affect accessibility.

### Security

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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could already happen even now though (someone could replace the settings file), so I don't think this adds that much risk


### Reliability

This should not affect reliability - most of what its doing is simply layering json which we already do.

### Compatibility

Again, there should not be any issues here - the user settings can still be layered after this layering for the user
to have the final say.

### Performance, Power, and Efficiency

Looking through the additional json files could negatively impact startup time.

## Potential Issues

Cases which would likely be frustrating:

* An installer dumps a _lot_ of json files into the folder which we need to look through.

## Future considerations
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

How will this affect the settings UI?

PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
## Resources

N/A