Skip to content

Conversation

@lhecker
Copy link
Member

@lhecker lhecker commented Jul 10, 2025

If an IME provider sets both crText and crBk we should respect this,
but the previous logic would incorrectly assert for crLine != TF_CT_NONE.

Validation Steps Performed

❌ I'm not aware which TSF even sets these colors in a
way that's compatible with us in the first place...

@lhecker lhecker added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Priority-3 A description (P3) Area-i18n Internationalization issues (e.g.: non-US input handling doesn't work) labels Jul 10, 2025
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Looks good to me. The way he handle underline seems right too, since we're currently treating this block as "apply styling if foreground and background styling type matches", so just extending that reasoning to underline makes sense to me.

@DHowett DHowett merged commit f2b30b4 into main Jul 14, 2025
19 checks passed
@DHowett DHowett deleted the dev/lhecker/improve-tsf-color-filtering branch July 14, 2025 22:19
@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.22 Servicing Pipeline Jul 14, 2025
@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.23 Servicing Pipeline Jul 14, 2025
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.22 Servicing Pipeline Jul 29, 2025
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.23 Servicing Pipeline Jul 29, 2025
DHowett pushed a commit that referenced this pull request Jul 29, 2025
If an IME provider sets both `crText` and `crBk` we should respect this,
but the previous logic would incorrectly assert for `crLine !=
TF_CT_NONE`.

## Validation Steps Performed
❌ I'm not aware which TSF even sets these colors in a
way that's compatible with us in the first place...

(cherry picked from commit f2b30b4)
Service-Card-Id: PVTI_lADOAF3p4s4Axadtzgck4T0
Service-Version: 1.23
DHowett pushed a commit that referenced this pull request Jul 29, 2025
If an IME provider sets both `crText` and `crBk` we should respect this,
but the previous logic would incorrectly assert for `crLine !=
TF_CT_NONE`.

## Validation Steps Performed
❌ I'm not aware which TSF even sets these colors in a
way that's compatible with us in the first place...

(cherry picked from commit f2b30b4)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgck4T8
Service-Version: 1.22
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.23 Servicing Pipeline Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-i18n Internationalization issues (e.g.: non-US input handling doesn't work) 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

Status: Cherry Picked

Development

Successfully merging this pull request may close these issues.

4 participants