search

atom-community / terminal

Terminal integrated with atom-community/atom
https://atom.io/packages/atomic-terminal
Other
19 stars 4 forks source link

fix: add terminal syntax variables to standard theme #80

Closed UziTech closed 3 years ago

UziTech commented 3 years ago

Allow themes to specify terminal colors for standard theme.

@terminal-color-black: #2e3436;
@terminal-color-red: #cc0000;
@terminal-color-green: #4e9a06;
@terminal-color-yellow: #c4a000;
@terminal-color-blue: #3465a4;
@terminal-color-magenta: #75507b;
@terminal-color-cyan: #06989a;
@terminal-color-white: #d3d7cf;
@terminal-color-bright-black: #555753;
@terminal-color-bright-red: #ef2929;
@terminal-color-bright-green: #8ae234;
@terminal-color-bright-yellow: #fce94f;
@terminal-color-bright-blue: #729fcf;
@terminal-color-bright-magenta: #ad7fa8;
@terminal-color-bright-cyan: #34e2e2;
@terminal-color-bright-white: #eeeeec;

TODO:

UziTech commented 3 years ago

Or we can let the syntax themes define the less variables. Then we don't have to guess which scopes have which color.

How would that work? which scopes would we use for each color?

aminya commented 3 years ago

That could work if we had access to all the themes. But at this point in time, we need to find a way to set a reasonable default.

For the "background-color" and "text-color", we can use StyleReader to set the default color. We should still make it possible for others to override this in the less files if they want to.

UziTech commented 3 years ago

The background and text colors already work with the theme variables

UziTech commented 3 years ago 
background color: @app-background-color
text color: @text-color
selected background color: @background-color-selected
selected text color: @text-color-highlight

These are all standard variables that every ui theme uses

UziTech commented 3 years ago

We could also make the standard theme the default.

aminya commented 3 years ago 
background color: @app-background-color
text color: @text-color
selected background color: @background-color-selected
selected text color: @text-color-highlight

These are all standard variables that every ui theme uses

The last time I tested this PR, it didn't respect the theme changes. This is how we fixed it in Minimap https://github.com/atom-minimap/minimap/blob/1894eb523070f974c1c368058a6b0f57adb87b48/lib/main.js#L390-L395

UziTech commented 3 years ago

currently the terminal is not set to refresh on theme changes but that could be easily implemented without StyleReader

If you close the terminal, change the theme, then open a new terminal the theme should be updated.

UziTech commented 3 years ago

Ya I have to fix the tests, add documentation, and add the style change observers.

I should have time tonight or tomorrow for this.

UziTech commented 3 years ago

I think this is ready to go.

aminya commented 3 years ago

The last time I tested this PR, it didn't respect the theme changes. This is how we fixed it in Minimap atom-minimap/minimap@1894eb5/lib/main.js#L390-L395

Could you add these listeners so the colours reset when the theme changes?

image

UziTech commented 3 years ago

I did https://github.com/atom-community/terminal/pull/80/files#diff-80b106dfebcf0f2ffdc8f0b21bc9faa0dea9f9b051480bc437170a7ebf9160afR127

UziTech commented 3 years ago

Animation

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 1.1.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket: