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.

Sign up for GitHub

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

Jump to bottom

Feature Request: stable UUIDs for default profiles #870

Closed
DHowett-MSFT opened this issue May 16, 2019 · 3 comments · Fixed by #913
Closed

Feature Request: stable UUIDs for default profiles #870

DHowett-MSFT opened this issue May 16, 2019 · 3 comments · Fixed by #913
Assignees
@DHowett-MSFT
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Milestone
Terminal v0.2 - P...

Comments

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented May 16, 2019 •
edited
Loading

Summary of the new feature/enhancement

This feature will ensure that the default profiles created by Windows Terminal always have the same GUIDs. This is necessary for us to move to cascading settings (#754), as it will allow the user to override some of the default profiles and for us to re-map their overrides back to the original profile.

Proposed technical implementation details

I propose that we use a version 5 UUID as outlined in RFC 4122, section 4.3: Algorithm for Creating a Name-Based UUID.

Namespace: {2BDE4A90-D05F-401C-9492-E40884EAD1D8} (voted most random at local county fair)
Canonical Form: Name of profile encoded in BOM-less UTF-8 BOM-less UTF-16LE
@DHowett-MSFT DHowett-MSFT added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels May 16, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal v0.2 milestone May 16, 2019
@DHowett-MSFT DHowett-MSFT self-assigned this May 16, 2019
@DHowett-MSFT
Copy link
Contributor Author

Justification for 0.2 (@cinnamon-msft, @bitcrazed)

This is required for 0.2 because it will be our first out-in-the-wild release. If we change the GUIDs after that point, we risk damaging user settings.

@DHowett-MSFT
Copy link
Contributor Author

DHowett-MSFT commented May 17, 2019 •
edited
Loading

(This is assigned to me because I am working on it. It's not up for grabs!)

DHowett-MSFT pushed a commit that referenced this issue May 17, 2019
@DHowett
Generate UUIDv5s for default profiles
226f6bb 
Closes #870.
@DHowett-MSFT DHowett-MSFT mentioned this issue May 21, 2019
Merged
5 tasks
@DHowett-MSFT
Copy link
Contributor Author

I've switched the suggested name format from BOMless UTF-8 to BOMless UTF-16LE because I wanted to have fewer string conversions back and forth at runtime.

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 21, 2019
DHowett-MSFT pushed a commit that referenced this issue May 21, 2019
@DHowett
Switch to v5 UUIDs as profile GUIDs for the default profiles (#913)
8da6737 
This commit switches the GUIDs for default profiles from being randomly generated to being version 5 UUIDs. More info in #870.

## PR Checklist
* [x] Closes #870
* [x] CLA signed
* [x] Tests added/passed
* [x] Requires documentation to be updated (#883)
* [x] I've discussed this with core contributors already.

## Detailed Description of the Pull Request / Additional comments
This commit has a number of changes that seem ancillary, but they're general goodness. Let me explain:

* I've added a whole new Types test library with only two tests in
* Since UUIDv5 generation requires SHA1, we needed to take a dependency on bcrypt
* I honestly don't think we should have to link bcrypt in conhost, but LTO should take care of that
  * I considered adding a new Terminal-specific Utils/Types library, but that seemed like a waste
* The best way to link bcrypt turned out to be in line with a discussion @miniksa and I had, where we decided we both love APISets and think that the console should link against them exclusively... so I've added `onecore_apiset.lib` to the front of the link line, where it will deflect the linker away from most of the other libs automagically.

```
StartGroup: UuidTests::TestV5UuidU8String
Verify: AreEqual(uuidExpected, uuidActual)
EndGroup: UuidTests::TestV5UuidU8String [Passed]

StartGroup: UuidTests::TestV5UuidU16String
Verify: AreEqual(uuidExpected, uuidActual)
EndGroup: UuidTests::TestV5UuidU16String [Passed]
```
@jeremyd2019 jeremyd2019 mentioned this issue Jun 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DHowett-MSFT DHowett-MSFT

Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Terminal v0.2 - Preview 1
Development

Successfully merging a pull request may close this issue.

Switch to v5 UUIDs as profile GUIDs for the default profiles microsoft/terminal
2 participants
@DHowett-MSFT @miniksa