Skip to content

Make Terminal operate on and cache colors instead of RGB values#2481

Draft
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
vi-eclipse:color-terminal
Draft

Make Terminal operate on and cache colors instead of RGB values#2481
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
vi-eclipse:color-terminal

Conversation

@HeikoKlare
Copy link
Contributor

No description provided.

RGB rgb = null;
if (fPreferenceStore != null) {
try {
rgb = StringConverter.asRGB(
Copy link
Contributor

Choose a reason for hiding this comment

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

should this here not be an color already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an RGB value that is later wrapped into a color.

@@ -694,7 +695,7 @@
if (rgb == null) {
rgb = TerminalColorPresets.INSTANCE.getDefaultPreset().getRGB(terminalColor);
Copy link
Contributor

Choose a reason for hiding this comment

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

here as well ...

Comment on lines +88 to +89
Color defaultFg = fStyleMap.getForegroundColor(null);
doubleBufferGC.setForeground(defaultFg);
Copy link
Contributor

Choose a reason for hiding this comment

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

as we have Color getDefaultBackgroundColor() should the not better be a Color getForegroundColor() as well?

This would prevent repeating the code used here and in setupGC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a repetition as here it's about the default color (style == null) and in setupGC its the style-specific color. We can of course introduce additional methods like getDefaultForegroundColor() or the like, but it's will only be for consistency, not for reuse.

@github-actions
Copy link
Contributor

Test Results

 1 977 files  ±0   1 977 suites  ±0   1h 37m 8s ⏱️ -12s
 4 743 tests ±0   4 719 ✅ ±0   24 💤 ±0  0 ❌ ±0 
14 229 runs  ±0  14 047 ✅ ±0  182 💤 ±0  0 ❌ ±0 

Results for commit 860b9f4. ± Comparison against base commit 63797d0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants