Fix profile name generation to allocate unique name #9816
Conversation
ghost
added
Area-Settings UI
miniksa
added
the
AutoMerge
| const winrt::hstring newName{ fmt::format(L"Profile {}", _allProfiles.Size()) }; | ||
| newProfile->Name(newName); | ||
| winrt::hstring newName{}; | ||
| for (uint32_t candidateIndex = 0; candidateIndex < _allProfiles.Size() + 1; candidateIndex++) |
There was a problem hiding this comment.
nit: should we just start this at _allProfiles.Size()? I know we'll never have UINT32_MAX - Size() profiles, but won't the additional math on line 234 cause it to overflow then?
There was a problem hiding this comment.
Starting here from 0 to actually allow overflow (and to wrap around to Profile 0 and so on). This actually ensures that a name is allocated no matter what.
|
Hello @miniksa! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 7 hours 7 minutes. No worries though, I will be back when the time is right! 😉 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 (
|
There was a problem hiding this comment.
Thank you! @PankajBhojwani you may want to do something similar over in #9352 for your CascadiaSettings::DuplicateProfile.
|
@msftbot merge this in 1 minute |
|
Hello @DHowett! 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". |
ghost
merged commit 
DHowett
added
zPreview-Service-Queued-1.13
## PR Checklist * [x] Closes microsoft#9714 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. ## Detailed Description of the Pull Request / Additional comments Attempts to generate a name Profile X, where X is the index of the new profile (1-based). As long as name is already taken, generates new name by incrementing X by 1
DHowett
removed
the
zStable-Service-Queued-1.12
## PR Checklist * [x] Closes #9714 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. ## Detailed Description of the Pull Request / Additional comments Attempts to generate a name Profile X, where X is the index of the new profile (1-based). As long as name is already taken, generates new name by incrementing X by 1 (cherry picked from commit 3368e60)
## PR Checklist * [x] Closes #9714 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. ## Detailed Description of the Pull Request / Additional comments Attempts to generate a name Profile X, where X is the index of the new profile (1-based). As long as name is already taken, generates new name by incrementing X by 1 (cherry picked from commit 3368e60)
|
🎉 Handy links: |
ghost
mentioned this pull request
|
🎉 Handy links: |
DHowett
removed
the
zPreview-Service-Queued-1.13
PR Checklist
Detailed Description of the Pull Request / Additional comments
Attempts to generate a name Profile X, where X is the index of the new profile (1-based).
As long as name is already taken, generates new name by incrementing X by 1