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

Saving in the Settings UI breaks "iterateOn" #10365

Closed
ffes opened this issue Jun 8, 2021 · 4 comments · Fixed by #10373
Closed

Saving in the Settings UI breaks "iterateOn" #10365

ffes opened this issue Jun 8, 2021 · 4 comments · Fixed by #10373
Assignees
@carlos-zamora
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. Priority-0 Bugs that we consider release-blocking/recall-class (P0) 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 v1.10

Comments

@ffes
Copy link

ffes commented Jun 8, 2021

Windows Terminal version (or Windows build number)

1.9.1523.0

Other Software

No response

Steps to reproduce

I have written a blogpost on how to add the profiles to the command palette.

When I copy-paste the code mentioned on that page directly in settings.json everything works as expected.

When I open the Settings UI and press save the key iterateOn is deleted, even without changing anything.

For reference, this is a snippet:

{
	"iterateOn": "profiles",
	"icon": "${profile.icon}",
	"name": "${profile.name}: New tab",
	"command": {
		"action": "newTab",
		"profile": "${profile.name}"
	}
}

Expected Behavior

Saving from the Settings UI doesn't remove anything.

Actual Behavior

Saving from the UI removes the iterateOn key-values pair from settings.json.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jun 8, 2021
@DHowett
Copy link
Member

DHowett commented Jun 8, 2021

/cc @carlos-zamora this will stop us from shipping action serialization out of preview.

@DHowett
Copy link
Member

DHowett commented Jun 8, 2021

(user settings loss bugs are no joke.)

@DHowett DHowett added 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. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. labels Jun 8, 2021
@DHowett DHowett added this to the Terminal v1.10 milestone Jun 8, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 8, 2021
@carlos-zamora carlos-zamora self-assigned this Jun 8, 2021
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 8, 2021
@carlos-zamora carlos-zamora mentioned this issue Jun 9, 2021
Merged
2 tasks
@ghost ghost added the In-PR This issue has a related PR label Jun 9, 2021
@ghost ghost closed this as completed in #10373 Jun 10, 2021
@ghost ghost removed the In-PR This issue has a related PR label Jun 10, 2021
ghost pushed a commit that referenced this issue Jun 10, 2021
@carlos-zamora
Bugfix: serialize iterable commands (#10373)
9294ecc 
## Summary of the Pull Request
Fixes a bug where top-level iterable commands were not serialized.

## PR Checklist
* [X] Closes #10365 
* [X] Tests added/passed

## Detailed Description of the Pull Request / Additional comments
- `Command::ToJson`:
   - iterable commands deserve the same treatment as nested commands
- `ActionMap`:
   - Similar to how we store nested commands, iterable commands need to be handled separately from standard commands. Then, when generating the name map, we make sure we export the iterable commands at the same time we export the nested commands.
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jun 10, 2021
@carlos-zamora carlos-zamora mentioned this issue Jun 11, 2021
Open
12 tasks
DHowett pushed a commit that referenced this issue Jul 7, 2021
@carlos-zamora @DHowett
Bugfix: serialize iterable commands (#10373)
658e6e6 
## Summary of the Pull Request
Fixes a bug where top-level iterable commands were not serialized.

## PR Checklist
* [X] Closes #10365
* [X] Tests added/passed

## Detailed Description of the Pull Request / Additional comments
- `Command::ToJson`:
   - iterable commands deserve the same treatment as nested commands
- `ActionMap`:
   - Similar to how we store nested commands, iterable commands need to be handled separately from standard commands. Then, when generating the name map, we make sure we export the iterable commands at the same time we export the nested commands.

(cherry picked from commit 9294ecc)
@ghost
Copy link

ghost commented Jul 14, 2021

🎉This issue was addressed in #10373, which has now been successfully released as Windows Terminal v1.9.1942.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 14, 2021

🎉This issue was addressed in #10373, which has now been successfully released as Windows Terminal Preview v1.10.1933.0.:tada:

Handy links:

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

@carlos-zamora carlos-zamora

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. Priority-0 Bugs that we consider release-blocking/recall-class (P0) 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 v1.10
Development

Successfully merging a pull request may close this issue.

Bugfix: serialize iterable commands microsoft/terminal
3 participants
@DHowett @ffes @carlos-zamora