[GH-ISSUE #254] Update tcell to the latest 2.x version #139

Closed
opened 2026-03-03 16:22:45 +03:00 by kerem · 19 comments
Owner

Originally created by @mum4k on GitHub (Nov 13, 2020).
Original GitHub issue: https://github.com/mum4k/termdash/issues/254

Originally assigned to: @dyc3 on GitHub.

Changelog: https://github.com/gdamore/tcell/blob/master/CHANGESv2.adoc

And while we are at it, we can also consider making tcell the default in all code examples.

Originally created by @mum4k on GitHub (Nov 13, 2020). Original GitHub issue: https://github.com/mum4k/termdash/issues/254 Originally assigned to: @dyc3 on GitHub. Changelog: https://github.com/gdamore/tcell/blob/master/CHANGESv2.adoc And while we are at it, we can also consider making `tcell` the default in all code examples.
kerem 2026-03-03 16:22:45 +03:00
Author
Owner

@mum4k commented on GitHub (Nov 14, 2020):

@dyc3 I got some prep work done (making tcell default, updating documentation), but didn't get to create the PR that updates to 2.x yet.

The next steps should be something like:

  1. update the dependency
  2. update all import paths
  3. verify that none of the breaking changes actually break our build and functionality. The only one that looks worrying is the change of mouse button handling. We may need to use or modify one of the demos to verify the new behavior.

If you have the time to prepare the PR with at least the first two steps (and anything I might have missed), that would help a lot. Alternatively I can get to it a few days from now. I will be of course happy to help with the testing step, maybe we can both poke at it - more eyes gives us a better chance of catching anything that broke.

<!-- gh-comment-id:727161460 --> @mum4k commented on GitHub (Nov 14, 2020): @dyc3 I got some prep work done (making tcell default, updating documentation), but didn't get to create the PR that updates to 2.x yet. The next steps should be something like: 1. update the dependency 1. update all import paths 1. verify that none of the [breaking changes](https://github.com/gdamore/tcell/blob/master/CHANGESv2.adoc#breaking-changes-in-tcell-v2) actually break our build and functionality. The only one that looks worrying is the change of mouse button handling. We may need to use or modify one of the demos to verify the new behavior. If you have the time to prepare the PR with at least the first two steps (and anything I might have missed), that would help a lot. Alternatively I can get to it a few days from now. I will be of course happy to help with the testing step, maybe we can both poke at it - more eyes gives us a better chance of catching anything that broke.
Author
Owner

@mum4k commented on GitHub (Nov 14, 2020):

@dyc3 if you choose to work on the PR, please fork from the devel branch to avoid conflicts and include the latest changes we made.

<!-- gh-comment-id:727161531 --> @mum4k commented on GitHub (Nov 14, 2020): @dyc3 if you choose to work on the PR, please fork from the **devel** branch to avoid conflicts and include the latest changes we made.
Author
Owner

@dyc3 commented on GitHub (Nov 14, 2020):

I'll take a look at it tomorrow. It's currently 3 am where I am and I need sleep haha

<!-- gh-comment-id:727162646 --> @dyc3 commented on GitHub (Nov 14, 2020): I'll take a look at it tomorrow. It's currently 3 am where I am and I need sleep haha
Author
Owner

@mum4k commented on GitHub (Nov 14, 2020):

We seem to be in the same timezone, so I can relate...

And thank you of course.

<!-- gh-comment-id:727162753 --> @mum4k commented on GitHub (Nov 14, 2020): We seem to be in the same timezone, so I can relate... And thank you of course.
Author
Owner

@dyc3 commented on GitHub (Nov 14, 2020):

So far I've only updated the import path for tcell to reference v2, and all the tests pass. Seems like a good sign.

<!-- gh-comment-id:727232282 --> @dyc3 commented on GitHub (Nov 14, 2020): So far I've only updated the import path for tcell to reference v2, and all the tests pass. Seems like a good sign.
Author
Owner

@mum4k commented on GitHub (Nov 14, 2020):

While that is good news, it is likely that it only gives a false sense of security. The unit tests in termdash only verify its own logic and only reach up to the terminalapi. The terminal drivers themselves are abstracted behind a fake terminal implementation in the tests.

If I remember correctly, we don't have a single integration test with either termbox or tcell, so the only way to tell if this works correctly may be by running the demos and exercising the functionality.

<!-- gh-comment-id:727243040 --> @mum4k commented on GitHub (Nov 14, 2020): While that is good news, it is likely that it only gives a false sense of security. The unit tests in `termdash` only verify its own logic and only reach up to the [terminalapi](https://github.com/mum4k/termdash/tree/master/terminal/terminalapi). The terminal drivers themselves are abstracted behind a [fake terminal](https://github.com/mum4k/termdash/tree/master/private/faketerm) implementation in the tests. If I remember correctly, we don't have a single integration test with either `termbox` or `tcell`, so the only way to tell if this works correctly may be by running the demos and exercising the functionality.
Author
Owner

@dyc3 commented on GitHub (Nov 14, 2020):

Color is broken because the number system was changed. Font modifiers still work though.

colors.go diff of 1.4 and 2.0:

1c1
< // Copyright 2015 The TCell Authors
---
> // Copyright 2020 The TCell Authors
23a24,26
> // We use a 64-bit integer to allow future expansion if we want to add an
> // 8-bit alpha, while still leaving us some room for extra options.
> //
28c31
< type Color int32
---
> type Color uint64
32,33c35,41
< 	// system or teminal default may exist.
< 	ColorDefault Color = -1
---
> 	// system or terminal default may exist.  It's also the zero value.
> 	ColorDefault Color = 0
>
> 	// ColorIsValid is used to indicate the color value is actually
> 	// valid (initialized).  This is useful to permit the zero value
> 	// to be treated as the default.
> 	ColorValid Color = 1 << 32
38c46,50
< 	ColorIsRGB Color = 1 << 24
---
> 	ColorIsRGB Color = 1 << 33
>
> 	// ColorSpecial is a flag used to indicate that the values have
> 	// special meaning, and live outside of the color space(s).
> 	ColorSpecial Color = 1 << 34
45c57
< 	ColorBlack Color = iota
---
> 	ColorBlack = ColorValid + iota
820a833,839
> // Special colors.
> const (
> 	// ColorReset is used to indicate that the color should use the
> 	// vanilla terminal colors.  (Basically go back to the defaults.)
> 	ColorReset = ColorSpecial | iota
> )
>
971a991,1000
> // Valid indicates the color is a valid value (has been set).
> func (c Color) Valid() bool {
> 	return c&ColorValid != 0
> }
>
> // IsRGB is true if the color is an RGB specific value.
> func (c Color) IsRGB() bool {
> 	return c&(ColorValid|ColorIsRGB) == (ColorValid | ColorIsRGB)
> }
>
975a1005,1007
> 	if !c.Valid() {
> 		return -1
> 	}
977c1009
< 		return (int32(c) & 0xffffff)
---
> 		return int32(c) & 0xffffff
995a1028,1040
> // TrueColor returns the true color (RGB) version of the provided color.
> // This is useful for ensuring color accuracy when using named colors.
> // This will override terminal theme colors.
> func (c Color) TrueColor() Color {
> 	if !c.Valid() {
> 		return ColorDefault
> 	}
> 	if c&ColorIsRGB != 0 {
> 		return c
> 	}
> 	return Color(c.Hex()) | ColorIsRGB | ColorValid
> }
>
1004c1049
< 	return ColorIsRGB | Color(v)
---
> 	return ColorIsRGB | Color(v) | ColorValid
1019a1065,1069
>
> // PaletteColor creates a color based on the palette index.
> func PaletteColor(index int) Color {
> 	return Color(index) | ColorValid
> }
\ No newline at end of file
<!-- gh-comment-id:727266868 --> @dyc3 commented on GitHub (Nov 14, 2020): Color is broken because the number system was changed. Font modifiers still work though. `colors.go` diff of 1.4 and 2.0: ```diff 1c1 < // Copyright 2015 The TCell Authors --- > // Copyright 2020 The TCell Authors 23a24,26 > // We use a 64-bit integer to allow future expansion if we want to add an > // 8-bit alpha, while still leaving us some room for extra options. > // 28c31 < type Color int32 --- > type Color uint64 32,33c35,41 < // system or teminal default may exist. < ColorDefault Color = -1 --- > // system or terminal default may exist. It's also the zero value. > ColorDefault Color = 0 > > // ColorIsValid is used to indicate the color value is actually > // valid (initialized). This is useful to permit the zero value > // to be treated as the default. > ColorValid Color = 1 << 32 38c46,50 < ColorIsRGB Color = 1 << 24 --- > ColorIsRGB Color = 1 << 33 > > // ColorSpecial is a flag used to indicate that the values have > // special meaning, and live outside of the color space(s). > ColorSpecial Color = 1 << 34 45c57 < ColorBlack Color = iota --- > ColorBlack = ColorValid + iota 820a833,839 > // Special colors. > const ( > // ColorReset is used to indicate that the color should use the > // vanilla terminal colors. (Basically go back to the defaults.) > ColorReset = ColorSpecial | iota > ) > 971a991,1000 > // Valid indicates the color is a valid value (has been set). > func (c Color) Valid() bool { > return c&ColorValid != 0 > } > > // IsRGB is true if the color is an RGB specific value. > func (c Color) IsRGB() bool { > return c&(ColorValid|ColorIsRGB) == (ColorValid | ColorIsRGB) > } > 975a1005,1007 > if !c.Valid() { > return -1 > } 977c1009 < return (int32(c) & 0xffffff) --- > return int32(c) & 0xffffff 995a1028,1040 > // TrueColor returns the true color (RGB) version of the provided color. > // This is useful for ensuring color accuracy when using named colors. > // This will override terminal theme colors. > func (c Color) TrueColor() Color { > if !c.Valid() { > return ColorDefault > } > if c&ColorIsRGB != 0 { > return c > } > return Color(c.Hex()) | ColorIsRGB | ColorValid > } > 1004c1049 < return ColorIsRGB | Color(v) --- > return ColorIsRGB | Color(v) | ColorValid 1019a1065,1069 > > // PaletteColor creates a color based on the palette index. > func PaletteColor(index int) Color { > return Color(index) | ColorValid > } \ No newline at end of file ```
Author
Owner

@mum4k commented on GitHub (Nov 14, 2020):

Thanks for catching that, I do remember reading something about color palette changes in the v2 changelog. We may need to update our tcell library to do any necessary translation.

Let me know if you get tired of looking at it, I can check out your branch and try to propose some solutions.

<!-- gh-comment-id:727271547 --> @mum4k commented on GitHub (Nov 14, 2020): Thanks for catching that, I do remember reading something about color palette changes in the v2 changelog. We may need to update our tcell library to do any necessary translation. Let me know if you get tired of looking at it, I can check out your branch and try to propose some solutions.
Author
Owner

@dyc3 commented on GitHub (Nov 14, 2020):

I'm kinda stumped.

I think fixColor needs to add tcell.ColorValid to the color value? When I try that, I get colors back, but they are all wrong.

And the tests for cellColor and fixColor are still passing, which doesn't really make sense because I'm changing the behavior of both of those functions...

<!-- gh-comment-id:727272520 --> @dyc3 commented on GitHub (Nov 14, 2020): I'm kinda stumped. I think `fixColor` needs to add `tcell.ColorValid` to the color value? When I try that, I get colors back, but they are all wrong. And the tests for `cellColor` and `fixColor` are still passing, which doesn't really make sense because I'm changing the behavior of both of those functions...
Author
Owner

@mum4k commented on GitHub (Nov 14, 2020):

I will take a look a bit later tonight and update here. I need to spend some time understanding the change they did to v2.

<!-- gh-comment-id:727273176 --> @mum4k commented on GitHub (Nov 14, 2020): I will take a look a bit later tonight and update here. I need to spend some time understanding the change they did to v2.
Author
Owner

@mum4k commented on GitHub (Nov 15, 2020):

I think I understand the problem now and I am fairly convinced this is a bug in our tcell library. I will send you a PR branched off your branch that will fix that and also align our color definitions with the standard tcell is using. Since that is the official standard. I just need to figure out how to make this backward compatible for the termbox users out there.

<!-- gh-comment-id:727287327 --> @mum4k commented on GitHub (Nov 15, 2020): I think I understand the problem now and I am fairly convinced this is a bug in our `tcell` library. I will send you a PR branched off your branch that will fix that and also align our color definitions with the standard `tcell` is using. Since that is the official standard. I just need to figure out how to make this backward compatible for the `termbox` users out there.
Author
Owner

@mum4k commented on GitHub (Nov 15, 2020):

The following PR is addressing the color problem: https://github.com/dyc3/termdash/pull/1

The issue was that our tcell library was depending on the values of tcell's color constants. We shouldn't have done that, those are internal to their implementation and they did change them when upgrading to v2.

Feel free to merge these commits into your PR, we can push it into the devel branch altogether.

Did you find any other incompatible or breaking changes? Any concerns about the change related to the middle mouse button?

<!-- gh-comment-id:727509212 --> @mum4k commented on GitHub (Nov 15, 2020): The following PR is addressing the color problem: https://github.com/dyc3/termdash/pull/1 The issue was that our tcell library was depending on the **values** of `tcell`'s color constants. We shouldn't have done that, those are internal to their implementation and they did change them when upgrading to v2. Feel free to merge these commits into your PR, we can push it into the devel branch altogether. Did you find any other incompatible or breaking changes? Any concerns about the change related to the middle mouse button?
Author
Owner

@dyc3 commented on GitHub (Nov 15, 2020):

I'll go ahead and merge that in.

I haven't tested the mouse button stuff yet. I'll look at it in a bit, I'm currently fighting fires on another project.

<!-- gh-comment-id:727516081 --> @dyc3 commented on GitHub (Nov 15, 2020): I'll go ahead and merge that in. I haven't tested the mouse button stuff yet. I'll look at it in a bit, I'm currently fighting fires on another project.
Author
Owner

@mum4k commented on GitHub (Nov 15, 2020):

Take your time and feel free to let me know if you would like me to take point on this.

<!-- gh-comment-id:727517441 --> @mum4k commented on GitHub (Nov 15, 2020): Take your time and feel free to let me know if you would like me to take point on this.
Author
Owner

@dyc3 commented on GitHub (Nov 15, 2020):

I've tested mouse inputs on Linux, and they work as expected. I don't have an easy way to test on windows currently.

The function keys also work as expected, however modifiers are not passed along at all. Is this intentional?

<!-- gh-comment-id:727602210 --> @dyc3 commented on GitHub (Nov 15, 2020): I've tested mouse inputs on Linux, and they work as expected. I don't have an easy way to test on windows currently. The function keys also work as expected, however modifiers are not passed along at all. Is this intentional?
Author
Owner

@mum4k commented on GitHub (Nov 15, 2020):

Thank you for testing it, I will give it a go too to confirm later today.

Can you give me a few more details about the modifiers? Do you mean that some of our keys don't work, e.g. KeyCtrlA:
github.com/mum4k/termdash@2a7dafa3a8/keyboard/keyboard.go (L59)

Or do you mean that the new support for modifiers added by tcell in v2 isn't reflected in our API?
github.com/gdamore/tcell@7d87d8188c/key.go (L73-L79)

In other words - did any existing functionality change after upgrading tcell to v2 or are we just missing new functionality?

Additionally is #260 ready for review or do we still need to add something?

<!-- gh-comment-id:727608658 --> @mum4k commented on GitHub (Nov 15, 2020): Thank you for testing it, I will give it a go too to confirm later today. Can you give me a few more details about the modifiers? Do you mean that some of our keys don't work, e.g. `KeyCtrlA`: https://github.com/mum4k/termdash/blob/2a7dafa3a835dbe45d9a6eaa2f22e184666c8065/keyboard/keyboard.go#L59 Or do you mean that the new support for modifiers added by `tcell` in v2 isn't reflected in our API? https://github.com/gdamore/tcell/blob/7d87d8188c8d3a236c778107d4925a9b2465a5f6/key.go#L73-L79 In other words - did any existing functionality change after upgrading `tcell` to v2 or are we just missing new functionality? Additionally is #260 ready for review or do we still need to add something?
Author
Owner

@dyc3 commented on GitHub (Nov 15, 2020):

Oh wait, I see.
The keys like KeyCtrlA work, but Shift+F1 gives KeyF1.

We're just missing new functionality. #260 is ready to review

<!-- gh-comment-id:727615133 --> @dyc3 commented on GitHub (Nov 15, 2020): Oh wait, I see. The keys like `KeyCtrlA` work, but <kbd>Shift</kbd>+<kbd>F1</kbd> gives `KeyF1`. We're just missing new functionality. #260 is ready to review
Author
Owner

@mum4k commented on GitHub (Nov 15, 2020):

Yes that would be a missing functionality and is tracked in #262.

The PR looks good, please let me know if you are planning to send one more PR to add italic and strikethrough before we push the release.

<!-- gh-comment-id:727624259 --> @mum4k commented on GitHub (Nov 15, 2020): Yes that would be a missing functionality and is tracked in #262. The PR looks good, please let me know if you are planning to send one more PR to add italic and strikethrough before we push the release.
Author
Owner

@mum4k commented on GitHub (Nov 18, 2020):

Resolved by #266.

<!-- gh-comment-id:729400026 --> @mum4k commented on GitHub (Nov 18, 2020): Resolved by #266.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/termdash#139
No description provided.