Fixing color setting for tcell.

Our tcell library incorrectly referenced tcell color values rather than just names. The values aren't part of the public API and did change with the update to v2. This commit switches our tcell library to using the public constants exported by the `tcell` project. Also aligning our color definition of the first 16 colors with `tcell` and Xterm. Adding two additional colors to make this change backward compatible with `termbox-go`.
2025-04-25 13:48:50 +08:00 · 2020-11-14 23:21:07 -05:00 · 2020-11-14 23:21:07 -05:00 · 503c0ada52
commit 503c0ada52
parent 4d320e3f2e
5 changed files with 332 additions and 134 deletions
--- a/cell/color.go
+++ b/cell/color.go
@ -48,17 +48,32 @@ var colorNames = map[Color]string{
 const (
 	ColorDefault Color = iota

-	// 8 "system" colors.
+	// The 16 Xterm colors.
+	// See https://jonasjacek.github.io/colors/
 	ColorBlack
-	ColorRed
+	ColorMaroon
 	ColorGreen
+	ColorOlive
+	ColorNavy
+	ColorPurple
+	ColorTeal
+	ColorSilver
+	ColorGray
+	ColorRed
+	ColorLime
 	ColorYellow
 	ColorBlue
-	ColorMagenta
-	ColorCyan
+	ColorFuchsia
+	ColorAqua
 	ColorWhite
 )

+// Colors defined for backward compatibility with termbox-go.
+const (
+	ColorMagenta Color = ColorPurple
+	ColorCyan    Color = ColorTeal
+)
+
 // ColorNumber sets a color using its number.
 // Make sure your terminal is set to a terminalapi.ColorMode that supports the
 // target color. The provided value must be in the range 0-255.
@ -86,6 +101,8 @@ func ColorRGB6(r, g, b int) Color {
 			return ColorDefault
 		}
 	}
+	// Explanation:
+	// https://stackoverflow.com/questions/27159322/rgb-values-of-the-colors-in-the-ansi-extended-colors-index-17-255
 	return Color(0x10 + 36*r + 6*g + b + 1) // Colors are off-by-one due to ColorDefault being zero.
 }

--- a/terminal/tcell/cell_options.go
+++ b/terminal/tcell/cell_options.go
@ -15,34 +15,65 @@
 package tcell

 import (
-	"github.com/gdamore/tcell/v2"
+	tcell "github.com/gdamore/tcell/v2"
 	"github.com/mum4k/termdash/cell"
 	"github.com/mum4k/termdash/terminal/terminalapi"
 )

 // cellColor converts termdash cell color to the tcell format.
 func cellColor(c cell.Color) tcell.Color {
-	return tcell.Color(c&0x1ff) - 1
+	/*
+		switch c {
+		case cell.ColorDefault:
+			return tcell.ColorDefault
+		case cell.ColorBlack:
+			return tcell.ColorBlack
+		case cell.ColorRed:
+			return tcell.ColorRed
+		case cell.ColorGreen:
+			return tcell.ColorGreen
+		case cell.ColorYellow:
+			return tcell.ColorYellow
+		case cell.ColorBlue:
+			return tcell.ColorBlue
+		case cell.ColorMagenta:
+			return tcell.ColorFuchsia
+		case cell.ColorCyan:
+			return tcell.ColorAqua
+		case cell.ColorWhite:
+			return tcell.ColorWhite
+		}
+	*/
+	if c == cell.ColorDefault {
+		return tcell.ColorDefault
+	}
+	// Subtract one, because cell.ColorBlack has value one instead of zero.
+	// Zero is used for cell.ColorDefault instead.
+	return tcell.Color(c-1) + tcell.ColorValid
 }

-// fixColor converts the target color for the current color mode
-func fixColor(c tcell.Color, colorMode terminalapi.ColorMode) tcell.Color {
-	if c == tcell.ColorDefault {
+// colorToMode adjusts the color to the color mode.
+func colorToMode(c cell.Color, colorMode terminalapi.ColorMode) cell.Color {
+	if c == cell.ColorDefault {
 		return c
 	}
 	switch colorMode {
 	case terminalapi.ColorModeNormal:
-		c %= tcell.Color(16)
+		c %= 16 + 1 // Add one for cell.ColorDefault.
 	case terminalapi.ColorMode256:
-		c %= tcell.Color(256)
+		c %= 256 + 1 // Add one for cell.ColorDefault.
 	case terminalapi.ColorMode216:
-		c %= tcell.Color(216)
-		c += tcell.Color(16)
+		if c <= 216 { // Add one for cell.ColorDefault.
+			return c + 16
+		}
+		c = c%216 + 16
 	case terminalapi.ColorModeGrayscale:
-		c %= tcell.Color(24)
-		c += tcell.Color(232)
+		if c <= 24 { // Add one for cell.ColorDefault.
+			return c + 232
+		}
+		c = c%24 + 232
 	default:
-		c = tcell.ColorDefault
+		c = cell.ColorDefault
 	}
 	return c
 }
@ -51,11 +82,8 @@ func fixColor(c tcell.Color, colorMode terminalapi.ColorMode) tcell.Color {
 func cellOptsToStyle(opts *cell.Options, colorMode terminalapi.ColorMode) tcell.Style {
 	st := tcell.StyleDefault

-	fg := cellColor(opts.FgColor)
-	bg := cellColor(opts.BgColor)
-
-	fg = fixColor(fg, colorMode)
-	bg = fixColor(bg, colorMode)
+	fg := cellColor(colorToMode(opts.FgColor, colorMode))
+	bg := cellColor(colorToMode(opts.BgColor, colorMode))

 	// FIXME: tcell doesn't have a strikethrough style option until #254 is resolved.
 	st = st.Foreground(fg).Background(bg).Bold(opts.Bold).Italic(opts.Italic).Underline(opts.Underline)
--- a/terminal/tcell/cell_options_test.go
+++ b/terminal/tcell/cell_options_test.go
@ -15,131 +15,285 @@
 package tcell

 import (
+	"reflect"
 	"testing"

-	"github.com/gdamore/tcell/v2"
+	tcell "github.com/gdamore/tcell/v2"
+	"github.com/kylelemons/godebug/pretty"
 	"github.com/mum4k/termdash/cell"
 	"github.com/mum4k/termdash/terminal/terminalapi"
 )

-func TestCellColor(t *testing.T) {
-	tests := []struct {
-		color cell.Color
-		want  tcell.Color
-	}{
-		{cell.ColorDefault, tcell.ColorDefault},
-		{cell.ColorBlack, tcell.ColorBlack},
-		{cell.ColorRed, tcell.ColorMaroon},
-		{cell.ColorGreen, tcell.ColorGreen},
-		{cell.ColorYellow, tcell.ColorOlive},
-		{cell.ColorBlue, tcell.ColorNavy},
-		{cell.ColorMagenta, tcell.ColorPurple},
-		{cell.ColorCyan, tcell.ColorTeal},
-		{cell.ColorWhite, tcell.ColorSilver},
-		{cell.ColorNumber(42), tcell.Color(42)},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.color.String(), func(t *testing.T) {
-			got := cellColor(tc.color)
-			if got != tc.want {
-				t.Errorf("cellColor(%v) => got %v, want %v", tc.color, got, tc.want)
-			}
-		})
-	}
-}
-
-func TestFixColor(t *testing.T) {
-	tests := []struct {
-		colorMode terminalapi.ColorMode
-		color     cell.Color
-		want      tcell.Color
-	}{
-		// See https://jonasjacek.github.io/colors/ for a good reference of all 256 xterm colors
-		// All 256 colors
-		{terminalapi.ColorMode256, cell.ColorDefault, tcell.ColorDefault},
-		{terminalapi.ColorMode256, cell.ColorBlack, tcell.ColorBlack},
-		{terminalapi.ColorMode256, cell.ColorRed, tcell.ColorMaroon},
-		{terminalapi.ColorMode256, cell.ColorGreen, tcell.ColorGreen},
-		{terminalapi.ColorMode256, cell.ColorYellow, tcell.ColorOlive},
-		{terminalapi.ColorMode256, cell.ColorBlue, tcell.ColorNavy},
-		{terminalapi.ColorMode256, cell.ColorMagenta, tcell.ColorPurple},
-		{terminalapi.ColorMode256, cell.ColorCyan, tcell.ColorTeal},
-		{terminalapi.ColorMode256, cell.ColorWhite, tcell.ColorSilver},
-		{terminalapi.ColorMode256, cell.ColorNumber(42), tcell.Color(42)},
-		// 8 system colors
-		{terminalapi.ColorModeNormal, cell.ColorDefault, tcell.ColorDefault},
-		{terminalapi.ColorModeNormal, cell.ColorBlack, tcell.ColorBlack},
-		{terminalapi.ColorModeNormal, cell.ColorRed, tcell.ColorMaroon},
-		{terminalapi.ColorModeNormal, cell.ColorGreen, tcell.ColorGreen},
-		{terminalapi.ColorModeNormal, cell.ColorYellow, tcell.ColorOlive},
-		{terminalapi.ColorModeNormal, cell.ColorBlue, tcell.ColorNavy},
-		{terminalapi.ColorModeNormal, cell.ColorMagenta, tcell.ColorPurple},
-		{terminalapi.ColorModeNormal, cell.ColorCyan, tcell.ColorTeal},
-		{terminalapi.ColorModeNormal, cell.ColorWhite, tcell.ColorSilver},
-		{terminalapi.ColorModeNormal, cell.ColorNumber(42), tcell.Color(10)},
-		// Grayscale colors (all the grey colours from 231 to 255)
-		{terminalapi.ColorModeGrayscale, cell.ColorDefault, tcell.ColorDefault},
-		{terminalapi.ColorModeGrayscale, cell.ColorBlack, tcell.Color232},
-		{terminalapi.ColorModeGrayscale, cell.ColorRed, tcell.Color233},
-		{terminalapi.ColorModeGrayscale, cell.ColorGreen, tcell.Color234},
-		{terminalapi.ColorModeGrayscale, cell.ColorYellow, tcell.Color235},
-		{terminalapi.ColorModeGrayscale, cell.ColorBlue, tcell.Color236},
-		{terminalapi.ColorModeGrayscale, cell.ColorMagenta, tcell.Color237},
-		{terminalapi.ColorModeGrayscale, cell.ColorCyan, tcell.Color238},
-		{terminalapi.ColorModeGrayscale, cell.ColorWhite, tcell.Color239},
-		{terminalapi.ColorModeGrayscale, cell.ColorNumber(42), tcell.Color(250)},
-		// 216 colors (16 to 231)
-		{terminalapi.ColorMode216, cell.ColorDefault, tcell.ColorDefault},
-		{terminalapi.ColorMode216, cell.ColorBlack, tcell.Color16},
-		{terminalapi.ColorMode216, cell.ColorRed, tcell.Color17},
-		{terminalapi.ColorMode216, cell.ColorGreen, tcell.Color18},
-		{terminalapi.ColorMode216, cell.ColorYellow, tcell.Color19},
-		{terminalapi.ColorMode216, cell.ColorBlue, tcell.Color20},
-		{terminalapi.ColorMode216, cell.ColorMagenta, tcell.Color21},
-		{terminalapi.ColorMode216, cell.ColorCyan, tcell.Color22},
-		{terminalapi.ColorMode216, cell.ColorWhite, tcell.Color23},
-		{terminalapi.ColorMode216, cell.ColorNumber(42), tcell.Color(58)},
-		// Unknown color mode
-		{-1, cell.ColorRed, tcell.ColorDefault},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.colorMode.String()+"_"+tc.color.String(), func(t *testing.T) {
-			color := cellColor(tc.color)
-			got := fixColor(color, tc.colorMode)
-			if got != tc.want {
-				t.Errorf("fixColor(%v_%v), => got %v, want %v", tc.colorMode, tc.color, got, tc.want)
-			}
-		})
-	}
-}
-
 func TestCellOptsToStyle(t *testing.T) {
 	tests := []struct {
+		desc      string
 		colorMode terminalapi.ColorMode
 		opts      cell.Options
 		want      tcell.Style
 	}{
 		{
+			desc:      "ColorMode256: ColorDefault and ColorBlack",
 			colorMode: terminalapi.ColorMode256,
-			opts:      cell.Options{FgColor: cell.ColorWhite, BgColor: cell.ColorBlack},
-			want:      tcell.StyleDefault.Foreground(tcell.ColorSilver).Background(tcell.ColorBlack),
+			opts: cell.Options{
+				FgColor: cell.ColorDefault,
+				BgColor: cell.ColorBlack,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorDefault).
+				Background(tcell.ColorBlack),
 		},
 		{
+			desc:      "ColorMode256: ColorMaroon and ColorGreen",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorMaroon,
+				BgColor: cell.ColorGreen,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorMaroon).
+				Background(tcell.ColorGreen),
+		},
+		{
+			desc:      "ColorMode256: ColorOlive and ColorNavy",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorOlive,
+				BgColor: cell.ColorNavy,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorOlive).
+				Background(tcell.ColorNavy),
+		},
+		{
+			desc:      "ColorMode256: ColorPurple and ColorTeal",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorPurple,
+				BgColor: cell.ColorTeal,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorPurple).
+				Background(tcell.ColorTeal),
+		},
+		{
+			desc:      "ColorMode256: ColorSilver and ColorGray",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorSilver,
+				BgColor: cell.ColorGray,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorSilver).
+				Background(tcell.ColorGray),
+		},
+		{
+			desc:      "ColorMode256: ColorRed and ColorLime",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorRed,
+				BgColor: cell.ColorLime,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorRed).
+				Background(tcell.ColorLime),
+		},
+		{
+			desc:      "ColorMode256: ColorYellow and ColorBlue",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorYellow,
+				BgColor: cell.ColorBlue,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorYellow).
+				Background(tcell.ColorBlue),
+		},
+		{
+			desc:      "ColorMode256: ColorFuchsia and ColorAqua",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorFuchsia,
+				BgColor: cell.ColorAqua,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorFuchsia).
+				Background(tcell.ColorAqua),
+		},
+		{
+			desc:      "ColorMode256: ColorWhite and ColorDefault",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorWhite,
+				BgColor: cell.ColorDefault,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorWhite).
+				Background(tcell.ColorDefault),
+		},
+		{
+			desc:      "ColorMode256: termbox compatibility colors ColorMagenta and ColorCyan",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorMagenta,
+				BgColor: cell.ColorCyan,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorPurple).
+				Background(tcell.ColorTeal),
+		},
+		{
+			desc:      "ColorMode256: first(0) and last(255) numbered color",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(0),
+				BgColor: cell.ColorNumber(255),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorBlack).
+				Background(tcell.Color255),
+		},
+		{
+			desc:      "ColorMode256: two numbered colors",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(33),
+				BgColor: cell.ColorNumber(200),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color33).
+				Background(tcell.Color200),
+		},
+		{
+			desc:      "ColorMode256: first and last RGB6 color",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorRGB6(0, 0, 0),
+				BgColor: cell.ColorRGB6(5, 5, 5),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color16).
+				Background(tcell.Color231),
+		},
+		{
+			desc:      "ColorMode256: first and last RGB24 color",
+			colorMode: terminalapi.ColorMode256,
+			opts: cell.Options{
+				FgColor: cell.ColorRGB24(0, 0, 0),
+				BgColor: cell.ColorRGB24(255, 255, 255),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color16).
+				Background(tcell.Color231),
+		},
+		{
+			desc:      "ColorModeNormal: first and last color",
 			colorMode: terminalapi.ColorModeNormal,
-			opts:      cell.Options{FgColor: cell.ColorWhite, BgColor: cell.ColorBlack},
-			want:      tcell.StyleDefault.Foreground(tcell.ColorSilver).Background(tcell.ColorBlack),
+			opts: cell.Options{
+				FgColor: cell.ColorBlack,
+				BgColor: cell.ColorWhite,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorBlack).
+				Background(tcell.ColorWhite),
 		},
 		{
-			colorMode: terminalapi.ColorModeGrayscale,
-			opts:      cell.Options{FgColor: cell.ColorWhite, BgColor: cell.ColorBlack},
-			want:      tcell.StyleDefault.Foreground(tcell.Color239).Background(tcell.Color232),
+			desc:      "ColorModeNormal: colors in the middle",
+			colorMode: terminalapi.ColorModeNormal,
+			opts: cell.Options{
+				FgColor: cell.ColorGreen,
+				BgColor: cell.ColorOlive,
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorGreen).
+				Background(tcell.ColorOlive),
 		},
 		{
+			desc:      "ColorModeNormal: colors above the range rotate back",
+			colorMode: terminalapi.ColorModeNormal,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(17),
+				BgColor: cell.ColorNumber(18),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorBlack).
+				Background(tcell.ColorMaroon),
+		},
+		{
+			desc:      "ColorMode216: first and last color",
 			colorMode: terminalapi.ColorMode216,
-			opts:      cell.Options{FgColor: cell.ColorWhite, BgColor: cell.ColorBlack},
-			want:      tcell.StyleDefault.Foreground(tcell.Color23).Background(tcell.Color16),
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(0),
+				BgColor: cell.ColorNumber(215),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color16).
+				Background(tcell.Color231),
+		},
+		{
+			desc:      "ColorMode216: colors in the middle",
+			colorMode: terminalapi.ColorMode216,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(1),
+				BgColor: cell.ColorNumber(2),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color17).
+				Background(tcell.Color18),
+		},
+		{
+			desc:      "ColorMode216: colors above the range rotate back",
+			colorMode: terminalapi.ColorMode216,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(216),
+				BgColor: cell.ColorNumber(217),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color16).
+				Background(tcell.Color17),
+		},
+		{
+			desc:      "ColorModeGrayscale: first and last color",
+			colorMode: terminalapi.ColorModeGrayscale,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(0),
+				BgColor: cell.ColorNumber(23),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color232).
+				Background(tcell.Color255),
+		},
+		{
+			desc:      "ColorModeGrayscale: colors in the middle",
+			colorMode: terminalapi.ColorModeGrayscale,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(1),
+				BgColor: cell.ColorNumber(2),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color233).
+				Background(tcell.Color234),
+		},
+		{
+			desc:      "ColorModeGrayscale: colors above the range rotate back",
+			colorMode: terminalapi.ColorModeGrayscale,
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(24),
+				BgColor: cell.ColorNumber(25),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.Color232).
+				Background(tcell.Color233),
+		},
+		{
+			desc:      "Unknown color mode converts to default color",
+			colorMode: terminalapi.ColorMode(-1),
+			opts: cell.Options{
+				FgColor: cell.ColorNumber(24),
+				BgColor: cell.ColorNumber(25),
+			},
+			want: tcell.StyleDefault.
+				Foreground(tcell.ColorDefault).
+				Background(tcell.ColorDefault),
 		},
 		{
 			colorMode: terminalapi.ColorModeNormal,
@ -159,13 +313,12 @@ func TestCellOptsToStyle(t *testing.T) {
 	}

 	for _, tc := range tests {
-		t.Run(tc.opts.FgColor.String()+"+"+tc.opts.BgColor.String(), func(t *testing.T) {
+		t.Run(tc.desc, func(t *testing.T) {
 			got := cellOptsToStyle(&tc.opts, tc.colorMode)
-			if got != tc.want {
-				fg, bg, _ := got.Decompose()
-				wantFg, wantBg, _ := tc.want.Decompose()
-				t.Errorf("cellOptsToStyle(%v, fg=%v, bg=%v) => got (fg=%X, bg=%X), want (fg=%X, bg=%X)",
-					tc.colorMode, tc.opts.FgColor, tc.opts.BgColor, fg, bg, wantFg, wantBg)
+			if !reflect.DeepEqual(got, tc.want) {
+				diff := pretty.Compare(tc.want, got)
+				t.Logf("opts: %+v\nstyle:%+v", tc.opts, got)
+				t.Errorf("cellOptsToStyle => unexpected diff (-want, +got):\n%s", diff)
 			}
 		})
 	}
--- a/terminal/tcell/tcell.go
+++ b/terminal/tcell/tcell.go
@ -19,7 +19,7 @@ import (
 	"fmt"
 	"image"

-	"github.com/gdamore/tcell/v2"
+	tcell "github.com/gdamore/tcell/v2"
 	"github.com/gdamore/tcell/v2/encoding"
 	"github.com/mum4k/termdash/cell"
 	"github.com/mum4k/termdash/private/event/eventqueue"
--- a/terminal/terminalapi/color_mode.go
+++ b/terminal/terminalapi/color_mode.go
@ -37,13 +37,13 @@ var colorModeNames = map[ColorMode]string{

 // Supported color modes.
 const (
-	// ColorModeNormal supports 8 "system" colors.
+	// ColorModeNormal supports 16 Xterm colors.
 	// These are defined as constants in the cell package.
 	ColorModeNormal ColorMode = iota

 	// ColorMode256 enables using any of the 256 terminal colors.
-	//     0-7: the 8 "system" colors accessible in ColorModeNormal.
-	//    8-15: the 8 "bright system" colors.
+	//     0-7: the 8 Xterm colors accessible in ColorModeNormal.
+	//    8-15: the 8 "bright" Xterm colors.
 	//  16-231: the 216 different terminal colors.
 	// 232-255: the 24 different shades of grey.
 	ColorMode256