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

Fragments should be able to add actions #16063

Closed
Tracked by #15845 ...
zadjii-msft opened this issue Sep 29, 2023 · 1 comment · Fixed by #16185
Closed
Tracked by #15845 ...

Fragments should be able to add actions #16063

zadjii-msft opened this issue Sep 29, 2023 · 1 comment · Fixed by #16185
Assignees
Labels
Area-Settings Issues related to settings and customizability, for console or terminal In-PR This issue has a related PR 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.

Comments

@microsoft-github-policy-service microsoft-github-policy-service bot 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 Sep 29, 2023
@zadjii-msft zadjii-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. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 29, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 29, 2023
@zadjii-msft zadjii-msft added this to the Up Next milestone Sep 29, 2023
@zadjii-msft
Copy link
Member Author

zadjii-msft commented Sep 29, 2023

I think we'd need to:

  • Add an internal / private source to actions, to indicate where we got the action from (ala profiles)
  • in SettingsLoader::_parseFragment move them all over into the user settings
  • don't serialize ones back that weren't from the user settings

NOPE not that

  • GlobalAppSettings::_FinalizeInheritance sets the LeastImportantParent for the user's action map on each parent global settings. Well, easy enough.
  • Create a "blank" GAS for each fragment
  • don't parse keys
  • add as a parent of the user's GAS

@zadjii-msft zadjii-msft modified the milestones: Up Next, Terminal v1.20 Oct 17, 2023
@zadjii-msft zadjii-msft self-assigned this Oct 17, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Oct 17, 2023
zadjii-msft added a commit that referenced this issue Feb 26, 2024
Surprisingly easier than I thought this would be. ActionMap already
supports layering (from defaults.json), so this basically re-uses a lot
of that for fun and profit.

The trickiest bits:
* In `SettingsLoader::_parseFragment`, I'm constructing a fake, empty
JSON object, and taking _only_ the actions out from the fragment, and
stuffing them into this temp json. Then, I parse that as a globals
object, and set _that_ as the parent to the user settings file. That
results in _only_ the actions from the fragment being parsed before the
user's actions.
* In that same method, I'm also explicitly preventing the ActionMap (et
al.) from parsing `keys` from these actions. We don't want fragments to
be able to say "ctrl+f is clear buffer" or something like that. This
required a bit of annoying plumbing.


Closes #16063 
Tests added.
Docs need to be updated.
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal In-PR This issue has a related PR 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
Development

Successfully merging a pull request may close this issue.

1 participant