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

Autoinsertion of profiles causes exponential explosion when you add a {name:}-only profile #2782

Closed
DHowett-MSFT opened this issue Sep 16, 2019 · 1 comment · Fixed by #2789
Closed

Autoinsertion of profiles causes exponential explosion when you add a {name:}-only profile #2782

DHowett-MSFT opened this issue Sep 16, 2019 · 1 comment · Fixed by #2789
Assignees
@zadjii-msft
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Milestone
Terminal-1909

Comments

@DHowett-MSFT
Copy link
Contributor

image
@DHowett-MSFT DHowett-MSFT added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Sep 16, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal-1909 milestone Sep 16, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 16, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 16, 2019
@zadjii-msft zadjii-msft added the Severity-Blocking We won't ship a release like this! No-siree. label Sep 17, 2019
zadjii-msft added a commit that referenced this issue Sep 17, 2019
@zadjii-msft
Add a test for #2782
907ebcd
@zadjii-msft zadjii-msft mentioned this issue Sep 17, 2019
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Sep 17, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements and removed In-PR This issue has a related PR labels Sep 17, 2019
zadjii-msft added a commit that referenced this issue Sep 17, 2019
@zadjii-msft @DHowett
Don't explode name-only profiles (#2789)
c30ef6d 
* Add a test for #2782

* Attempt to do something weird with _GenerateStub

  I was thinking maybe we have the stubs have a GUID included. I like that less though I think. That would mean that DPGs would always have the GUID generated for them, even if the DPG doesn't specify a GUID. I guess that's fine though. No DPG's _aren't_ generating names now so this shouldn't change anything.

* Add some more notes on why this was a bad idea

* Actually fix the issue at hand

  If the profile doesn't have a guid, it's a name-only profile.
  During validation, we'll generate a GUID for the profile, but
  validation occurs after this. We should ignore these types of
  profiles.
  If a dynamic profile was generated _without_ a GUID, we also
  don't want it serialized here. The first check in
  Profile::ShouldBeLayered checks that the profile hasa guid. For a
  dynamic profile without a GUID, that'll _never_ be true, so it
  would be impossible to be layered.

* Revert "Add some more notes on why this was a bad idea"

This reverts commit 85b8b8a.

* Revert "Attempt to do something weird with _GenerateStub"

This reverts commit f204b98.

* Little test fixes

* Update src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp

Co-Authored-By: Dustin L. Howett (MSFT) <[email protected]>
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Sep 17, 2019
@ghost
Copy link

ghost commented Sep 24, 2019

🎉This issue was addressed in #2789, which has now been successfully released as Windows Terminal Preview v0.5.2661.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Milestone
Terminal-1909
Development

Successfully merging a pull request may close this issue.

Don't explode name-only profiles microsoft/terminal
2 participants
@DHowett-MSFT @zadjii-msft