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

Implement inheritance/layering behavior for settings lists/maps #8100

Open
carlos-zamora opened this issue Oct 29, 2020 · 4 comments
Open

Implement inheritance/layering behavior for settings lists/maps #8100

carlos-zamora opened this issue Oct 29, 2020 · 4 comments
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone
Backlog

Comments

@carlos-zamora
Copy link
Member

Description of the new feature/enhancement

TSM Inheritance (Spec #7876 + Impl #7923 ) enabled an object model representation of layering JSON. Some settings that are saved to arrays/maps (i.e. actions, color schemes) still experience a special form of layering that is not represented in the object model.

Today, the object model std::moves the list of actions/schemes down child-by-child, and directly modifies those values. We need to restructure this system to...

  • track where an action/color scheme came from
  • layer actions over other actions

This blocks the following work items:

  • serialization for actions
  • Settings UI implementation for actions
  • serializing only new color schemes (ones not from defaults.json)

This also is partially related to...
@carlos-zamora carlos-zamora added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Oct 29, 2020
@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 Oct 29, 2020
@carlos-zamora carlos-zamora mentioned this issue Oct 29, 2020
Merged
1 task
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 2, 2020
@carlos-zamora carlos-zamora added this to the Terminal v1.7 milestone Jan 31, 2021
@carlos-zamora carlos-zamora mentioned this issue Feb 11, 2021
Open
12 tasks
@carlos-zamora
Copy link
Member Author

Moved this to backlog because this is more related to #8991 and #6900. So it's not extremely urgent right now.

@carlos-zamora carlos-zamora mentioned this issue Apr 22, 2021
Merged
@carlos-zamora
Copy link
Member Author

#9621 solves most of the problem here.

The remaining work to be done is for color schemes.

ghost pushed a commit that referenced this issue May 20, 2021
@carlos-zamora
Introduce serialization for actions (#9926)
ff8fdbd 
## Summary of the Pull Request

This PR builds on the `ActionMap` PR (#6900) by leveraging `ActionMap` to serialize actions. From the top down, the process is now as follows:
- `CascadiaSettings`: remove the hack of copying whatever we had for actions before.
- `GlobalAppSettings`: serialize the `ActionMap` to `"actions": []`
- `ActionMap`: iterate over the internal `_ActionMap` (list of actions) and serialize each `Command`
- `Command`: **THIS IS WHERE THE MAGIC HAPPENS!** For _each_ key mapping, serialize an action. Only the first one needs to include the name and icon.
- `ActionAndArgs`: Find the relevant `IActionArgs` parser and serialize the `ActionAndArgs`.
- `ActionArgs`: **ANNOYING CHANGE** Serialize any args that are set. We _need_ each setting to be saved as a `std::optional`. As with inheritance, this allows us to distinguish an explicit setting to the default value (sometimes `null`) vs an implicit "give me the default value". This allows us to serialize only the relevant details of each action, rather than writing _all_ of the args.

## References
- #8100: Inheritance/Layering for lists
   - This tracks layering and better serialization for color schemes _and_ actions. This PR resolves half of that issue. The next step is to apply the concepts used in this PR (and #9621) to solve the similar problem for color schemes.
- #6900: Actions page

## Validation Steps Performed
Tests added!
@zadjii-msft zadjii-msft mentioned this issue Nov 16, 2021
Closed
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@zadjii-msft
Copy link
Member

@DHowett So, I think #12800 closes out the last bit of this? This issue was written long before Leonard re-wrote the settings parser, so I don't think it's relevant anymore?

@DHowett
Copy link
Member

DHowett commented Feb 27, 2024

I'm still somewhat uncomfortable with this -- we are still doing flat mapping of things that come from different layers, we're just doing it more cleverly with 12800. Themes aren't touched at all, so they are still flat-mapped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@DHowett @carlos-zamora @zadjii-msft