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

"Magenta" is called "Purple" in WT #11456

Closed
Diablo-D3 opened this issue Oct 7, 2021 · 8 comments · Fixed by #13261
Closed

"Magenta" is called "Purple" in WT #11456

Diablo-D3 opened this issue Oct 7, 2021 · 8 comments · Fixed by #13261
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Backlog

Comments

@Diablo-D3
Copy link

Windows Terminal version (or Windows build number)

All versions

Other Software

No response

Steps to reproduce

Terminal calls "Magenta" by the wrong name, instead it is called "Purple".

Expected Behavior

The expected behavior is calling it "Magenta" instead of "Purple".

Actual Behavior

Terminal calls the color in question "Purple", instead of "Magenta".
@ghost ghost 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 Oct 7, 2021
@Diablo-D3
Copy link
Author

Diablo-D3 commented Oct 7, 2021 •
edited
Loading

I apologize for the simplicity of this report, but the form submission really doesn't cover trivial issues.

I also understand that it is possible nobody on the team wants to handle the GUI implications of handling non-English translations, but I would accept a proposed fix of the settings.json parser also accepting "Magenta" as a string, since it will now always have to also accept "Purple" to not break legacy configurations.

@Diablo-D3 Diablo-D3 mentioned this issue Oct 7, 2021
Open
@Diablo-D3
Copy link
Author

Incorrect. It is called magenta in the 8 and 16 color systems, and these are industry standard technical terms. Similarly, you do not say a printer uses cyan, purple, yellow, and black.

@zadjii-msft
Copy link
Member

Huh. No one else has complained about this since we shipped the Terminal more than 2 years ago now. I suppose we could also accept magenta as a key. Probably no harm in that.

@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Oct 8, 2021
@zadjii-msft zadjii-msft added this to the Icebox ❄ milestone Oct 8, 2021
@Diablo-D3
Copy link
Author

@zadjii-msft I'm actually surprised no one has hit this particular issue. In my case, along with #11457, it may mean nobody has tried to automate porting color schemes to WT. Or, alternatively, they have, and none of their schemes worked, and they just gave up instead of figuring out why they didn't work.

@j4james
Copy link
Collaborator

j4james commented Oct 8, 2021

No one else has complained about this since we shipped the Terminal more than 2 years ago now

We have actually discussed this before, but didn't really commit to fixing it: #2641 (comment)

@DHowett
Copy link
Member

DHowett commented Oct 8, 2021

/me goes off to register @MikeVsMike

Fair enough.

@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 15, 2021
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 15, 2021
@zadjii-msft zadjii-msft added the good first issue This is a fix that might be easier for someone to do as a first contribution label Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
matthewd673 added a commit to matthewd673/terminal that referenced this issue Jun 10, 2022
@matthewd673
Check alt color names in Json (microsoft#11456)
8f799b6
@matthewd673 matthewd673 mentioned this issue Jun 10, 2022
Merged
@ghost ghost added the In-PR This issue has a related PR label Jun 10, 2022
@ghost ghost closed this as completed in #13261 Jun 10, 2022
ghost pushed a commit that referenced this issue Jun 10, 2022
@matthewd673
Accept color name "magenta" instead of "purple" (#13261)
1b630ab 
When loading color schemes from Json, check if GetValueForKey failed. If
it did, and we were searching for the colors "purple"/"brightPurple",
swap out the color name with "magenta"/"brightMagenta" and try again.
This was tested manually by creating a new color scheme using the colors
"magenta"/"brightMagenta" and loading it, then printing colored text in
the terminal to assure that the colors were correctly assigned as
purple.

This changes the color loader to use an index pair table to add support
for alternate color names.

## Validation Steps Performed
- Manually edit settings.json, creating a new color scheme with colors
  "magenta" and "brightMagenta"
- View the color scheme in settings - the colors will appear as "purple"
  and "brightPurple" in the UI
- Run a program that prints colored text, purple and brightPurple text
  will appear in the colors defined as magenta and brightMagenta
- Modify the color scheme in the settings UI and save it, the colors
  will be saved as "purple" and "brightPurple" *(I think this is the
  right behavior, since calling them "magenta" is technically a
  mistake)*
- Repeat the steps above with a normal color scheme - colors named
  "purple" and "brightPurple" still load properly

Closes #11456
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jun 10, 2022
@Diablo-D3
Copy link
Author

@matthewd673 Thanks for the fix.

@ghost
Copy link

ghost commented Jul 6, 2022

🎉This issue was addressed in #13261, which has now been successfully released as Windows Terminal Preview v1.15.186.:tada:

Handy links:

This issue was closed.
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 good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Accept color name "magenta" instead of "purple" matthewd673/terminal
4 participants
@DHowett @Diablo-D3 @j4james @zadjii-msft