Skip to content

Make all the ICoreSettings / IControlSettings properties getters-only #7219

Closed

Description

We've discussed this for a while now, but it's possible now this the other work Carlos is doing with settings.

I've got a WIP branch here: dev/migrie/f/settings-getters-only, with only a few errors.

I think for each of these, the TermControl is going to need to cache a local copy of the value, to represent it's actual runtime value, rather than use the IControlSettings as a buffer.

I'm filing this so someone else can pick that up where I left off.

Activity

  1. added
    Help WantedWe encourage anyone to jump in on these.
    Area-SettingsIssues related to settings and customizability, for console or terminal
    Issue-TaskIt's a feature request, but it doesn't really need a major design.
    Area-CodeHealthIssues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.
    good first issueThis is a fix that might be easier for someone to do as a first contribution
    on Aug 7, 2020
  2. ghost added
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Aug 7, 2020
  3. removed
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Aug 7, 2020
  4. MichelleTanPY commented on Aug 26, 2020

    @MichelleTanPY
    Contributor

    Hi, I thought of picking this up, and made the changes according to the WIP dev branch there. But I got a lot of errors from multiple files complaining that the argument takes 1 parameters. Any pointers or examples ?
    Thanks

  5. zadjii-msft commented on Aug 26, 2020

    @zadjii-msft
    MemberAuthor

    Michelle Tan (@MichelleTanPY) That's basically the whole task here actually. Currently, the TermControl stashes a bunch of run time changes to the settings in it's IControlSettings object. In that WIP branch, I made all the properties getters-only, but I haven't yet changed TermControl to do the right thing here. For any of the properties where there's a compile error, you'll need to change TermControl to have some member variable that it uses to store the runtime value instead.

  6. ghost added
    In-PRThis issue has a related PR
    on Oct 26, 2021
  7. ghost closed this as completedin #11619on Dec 1, 2021
  8. ghost added a commit that references this issue on Dec 1, 2021
    094273b
  9. ghost removed
    In-PRThis issue has a related PR
    on Dec 1, 2021
  10. ghost added
    Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.
    on Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Area-CodeHealthIssues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.Area-SettingsIssues related to settings and customizability, for console or terminalHelp WantedWe encourage anyone to jump in on these.Issue-TaskIt's a feature request, but it doesn't really need a major design.Product-TerminalThe new Windows Terminal.Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions