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

Fix a missing entry for intenseTextStyle in the Profile schema #14210

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

neersighted
Copy link
Contributor

@neersighted neersighted commented Oct 13, 2022

Fix a missing entry in the JSON schema for intenseTextStyle.

The JSON schema was missing an entry in the Profile section for
intenseTextStyle. I have added it as it appears in AppearanceConfig.
Note that this is currently duplicated in the schema -- however, this is
the pattern used already in Profile as AppearanceConfig entries have
alternate descriptions (and I have updated the description in that
section to make it clear it applies to unfocused terminals).
Longer-term, it likely makes sense to consolidate all entires into
ApperanceConfig and rely on the description for the unfocusedApperance
object/the name of the object to make the limited scope of those keys
clear, so that Profile can simply extend ApperanceConfig and the
duplication in the schema can be reduced.

Validation Steps Performed

Validation with schema verifying tools including VS Code.

Partially addresses #13387

@ghost ghost added Area-Schema Things that have to do with the json schema. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Oct 13, 2022
@DHowett
Copy link
Member

DHowett commented Oct 14, 2022

(I changed the wording from resolves to addresses, as the former would cause GitHub to automatically close the issue.)

Thanks for doing this!

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably go over the schema with a fine-toothed comb and perhaps some found expertise; it's grown ungainly as you've pointed out. Thanks again!

@DHowett DHowett merged commit 33cb0eb into microsoft:main Oct 14, 2022
@neersighted neersighted deleted the intensetextstyle_schema branch October 14, 2022 19:32
DHowett pushed a commit that referenced this pull request Oct 14, 2022
Fix a missing entry in the JSON schema for `intenseTextStyle`.

The JSON schema was missing an entry in the Profile section for
`intenseTextStyle`. I have added it as it appears in AppearanceConfig.
Note that this is currently duplicated in the schema -- however, this is
the pattern used already in Profile as AppearanceConfig entries have
alternate descriptions (and I have updated the description in that
section to make it clear it applies to unfocused terminals).
Longer-term, it likely makes sense to consolidate all entires into
ApperanceConfig and rely on the description for the `unfocusedApperance`
object/the name of the object to make the limited scope of those keys
clear, so that Profile can simply extend ApperanceConfig and the
duplication in the schema can be reduced.

## Validation Steps Performed
Validation with schema verifying tools including VS Code.

Partially addresses #13387

(cherry picked from commit 33cb0eb)
Service-Card-Id: 86228742
Service-Version: 1.15
DHowett pushed a commit that referenced this pull request Oct 14, 2022
Fix a missing entry in the JSON schema for `intenseTextStyle`.

The JSON schema was missing an entry in the Profile section for
`intenseTextStyle`. I have added it as it appears in AppearanceConfig.
Note that this is currently duplicated in the schema -- however, this is
the pattern used already in Profile as AppearanceConfig entries have
alternate descriptions (and I have updated the description in that
section to make it clear it applies to unfocused terminals).
Longer-term, it likely makes sense to consolidate all entires into
ApperanceConfig and rely on the description for the `unfocusedApperance`
object/the name of the object to make the limited scope of those keys
clear, so that Profile can simply extend ApperanceConfig and the
duplication in the schema can be reduced.

## Validation Steps Performed
Validation with schema verifying tools including VS Code.

Partially addresses #13387

(cherry picked from commit 33cb0eb)
Service-Card-Id: 86228743
Service-Version: 1.16
@ghost
Copy link

ghost commented Oct 18, 2022

🎉Windows Terminal v1.15.2874 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Dec 14, 2022

🎉Windows Terminal Preview v1.16.3463.0 and v1.16.3464.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Schema Things that have to do with the json schema. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants