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

--tabColor and setTabColor should accept color names #8893

Open
carlos-zamora opened this issue Jan 26, 2021 · 18 comments
Open

--tabColor and setTabColor should accept color names #8893

carlos-zamora opened this issue Jan 26, 2021 · 18 comments
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Milestone

Comments

@carlos-zamora
Copy link
Member

Description of the new feature/enhancement

It'd be cool if --tabColor and setTabColor should accept color names in addition to hex-codes. Specifically, something like this would be accepted:

{ "command": { "action": "setTabColor", "color": "red" }, "keys": "" }

Proposed technical implementation details (optional)

Detecting if it's a name vs hex-code should be pretty straightforward: check if the leading character is a #.

#7578 may help detect if the color name actually exists?

@carlos-zamora carlos-zamora added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Jan 26, 2021
@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 Jan 26, 2021
@DHowett
Copy link
Member

DHowett commented Jan 26, 2021

If we’re going to do that, we should converge all color converters on the VT color parser. All of them. This will change the semantics of our color parser slightly. It’s the only right thing to do.

@skyline75489
Copy link
Collaborator

Back then Dustin brought up the idea of "all the color we know about" in #7578. The XParseColor spec includes the # format which I think generally surpasses the plain hex format. So yeah this is currently possible using the functions added in #7578 .

You can just feed ColorFromXTermColor with a string and you'll have the color. See

void UtilsTests::TestColorFromXTermColor()

@carlos-zamora
Copy link
Member Author

carlos-zamora commented Jan 26, 2021

Yeah, it'd also be nice if we could allow for things like "foreground": "red" in the JSON too.

@DHowett
Copy link
Member

DHowett commented Jan 26, 2021

The only downside is that #eee will turn into #e0e0e0 (X11 rules) instead of #eeeeee (CSS rules, current WT)

@DHowett
Copy link
Member

DHowett commented Jan 26, 2021

Well, also that the color table will be copied into a third binary because of static linking but hey, we have a huge Utils problem already.

@skyline75489
Copy link
Collaborator

I think we can prioritize CSS rules if the input is in the form of #eee in this new ColorFromString function. On the VT side, we keep the old ColorFromXTermColor. This way we'll have the best of the both worlds. (anyone noticed that this is actually Hannah Montana reference, anyone?...)

@j4james
Copy link
Collaborator

j4james commented Jan 26, 2021

If you're going to prioritize CSS rules for the hex formats, then you also need to consider whether you want to prioritize CSS rules for the color names too (for example, CSS green and X11 green are very different colors). Sooner or later someone is going to ask why a particular color is what it is. That's easy enough to explain if we can just say we're using CSS, or we're using X11. It's harder to justify if we've got a weird mix of the two.

@skyline75489
Copy link
Collaborator

skyline75489 commented Jan 26, 2021 via email

@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Jan 26, 2021
@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 Jan 26, 2021
@DHowett
Copy link
Member

DHowett commented Jan 26, 2021

I think that we should converge on the X11 color names and accept the breaking change (the loss of 4 bits of depth on the short-form hex code colors). @zadjii-msft what do you think?

@zadjii-msft
Copy link
Member

I think if you're using #abc now as an alias for #aabbcc, and we break this by evaluating that now as #a0b0c0, and the workaround is "change the value to #aabbcc", then I think that's a perfectly reasonable story. People can deal with that for all of the 4 people using #abc style colors and who can tell the difference between #ffffff and #f0f0f0.

@zadjii-msft
Copy link
Member

Just so long as no one's expecting "red" to mean "the value of red from the color scheme". That's right out. Not doing that 😛

@skyline75489
Copy link
Collaborator

no one's expecting "red" to mean "the value of red from the color scheme".

I'm not sure, man. Our users have proven to be...imaginative .

@skyline75489
Copy link
Collaborator

I'm fine with converging on the X11 color names. I do believe most people are using #aabbcc instead of #abc (see, Github isn't picking this as a valid color). Let's face it, people who bother to use a customized hex code would probably be unsatisfied with simple colors like #abc.

Also I'm proposing to add support for rgb(111, 111, 111) format in addition to the X11 formats. This format is used widely on the web as we know it.

@zadjii-msft
Copy link
Member

Yea I'm cool with the rgb(12, 34, 56) version too

@j4james
Copy link
Collaborator

j4james commented Jan 27, 2021

Also I'm proposing to add support for rgb(111, 111, 111) format in addition to the X11 formats.

Can we please make sure that's not exposed in the OSC palette sequences though, unless it's one of the standard formats supported by other terminals.

@skyline75489
Copy link
Collaborator

skyline75489 commented Jan 27, 2021 via email

@DHowett
Copy link
Member

DHowett commented Jan 27, 2021

I don't necessarily see the value in rgb(x,y,z). If we are working to reduce the number of individual color parsers that we have to support . . . we shouldn't do so and then turn around and add another. As the settings UI gets used more and more, I expect that the incidence of folks using the uncommon syntaxes will evaporate until it reaches 0.

@skyline75489
Copy link
Collaborator

skyline75489 commented Jan 28, 2021 via email

@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

5 participants