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

integrated terminal: customizing colors through user settings is a bit weird #6766

Closed
alexdima opened this issue May 24, 2016 · 8 comments
Closed
Assignees
Labels
terminal Integrated terminal issues ux User experience issues verified Verification succeeded
Milestone

Comments

@alexdima
Copy link
Member

alexdima commented May 24, 2016

Testing #6654

Our story so far when it comes to customizing colors is through themes (fyi @aeschli ). Exposing color values in the user settings makes our story inconsistent. e.g. it would be nice to share a theme as an extension that also customizes the integrated terminal, I think the theme format is open-ended to support any amount of additional keys? This also opens the door for requests such as: "I want to change the color of keywords from my settings", which is a path we don't really want to go to

It is also needed that each color would have a light and a dark variant in the settings. if I tweak the values in the light theme, they might not look good when I switch to a dark theme, etc.

@alexdima alexdima added the ux User experience issues label May 24, 2016
@alexdima alexdima added this to the May 2016 milestone May 24, 2016
@Tyriar
Copy link
Member

Tyriar commented May 24, 2016

It wouldn't make sense to pull the values from themes. tmThemes, while not very strict, simply would not have these new values when migrating from other editors, so the majority of themes would not have the values we desire. Also, currently 2 themes cannot be applied at once.

This situation, while not ideal, allows the user to customize the colors in isolation on their theme.

Possible steps forward:

There is also more customization that may be added in the future, I believe it's possible to set a 255 color palette for the terminal as well.

@Tyriar Tyriar added the terminal Integrated terminal issues label May 24, 2016
@alexdima
Copy link
Member Author

I think #1231 is the best way forward, we need to have VSCode specific themes. I was suggesting TM themes as they have an open format, but you have a very good point: lots of people would like to have a vscode theme A (that changes file explorer colors, terminal colors (maybe even icons?), etc.) + a TM theme B (that changes tokens, selection color, etc).

My recommandation for now is to remove the colors from the settings, and maybe increase the priority of #1231.

@aeschli Your thoughts?

@aeschli
Copy link
Contributor

aeschli commented May 25, 2016

"integratedTerminal.ansiColors.brightBlack"
"integratedTerminal.ansiColors.cyan"
"integratedTerminal.ansiColors.blue"

I also think it make no sense to have these colors as user settings. A user would not choose to have 'blue' being a red color.
VS Code should define these colors, in three flavors, for the light and dark and hc_black (high contrast) base theme.

I agree that once we add theming the workbench we can look into this again.

Also in general, let's not add a configuration setting before we get multiple requests for it. We want to keep the number of settings small to keep the product slick and avoid feature creep.

@Tyriar
Copy link
Member

Tyriar commented May 25, 2016

I'm all for increasing the priority of #1231, was going with settings values as that's what most other editors. I'll remove the colors for now, it was trivial to add.

Tyriar added a commit that referenced this issue May 25, 2016
@Tyriar
Copy link
Member

Tyriar commented May 25, 2016

@alexandrudima @aeschli is there some event I can listen to for when the theme changes?

@Tyriar
Copy link
Member

Tyriar commented May 25, 2016

Never mind, I found ThemeService 😄

@Tyriar
Copy link
Member

Tyriar commented May 25, 2016

Improved light theme in a38efd8

@Tyriar Tyriar closed this as completed May 25, 2016
@Tyriar
Copy link
Member

Tyriar commented May 26, 2016

FYI I closed #1231 in favor of #76

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
terminal Integrated terminal issues ux User experience issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants