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

Revert termbox-go dependency bump #247

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Revert termbox-go dependency bump #247

merged 1 commit into from
Mar 30, 2023

Conversation

gardockt
Copy link

@gardockt gardockt commented Feb 16, 2023

termui is incompatible with the latest version of termbox-go, which caused highlighted text to disappear (#226). The reason behind it is that termui converts each terminal cell's desired colors and modifiers to termbox-go format, using the formula present in render.go:

tb.SetCell(
	/* ... */
	tb.Attribute(cell.Style.Fg+1)|tb.Attribute(cell.Style.Modifier), tb.Attribute(cell.Style.Bg+1),
)

termbox-go then reads the modifiers by comparing them with defined constants. Version v0.0.0-20190121233118-02980233997d, marked as termui's dependency inside its go.mod, contains these definitions:

const (
        AttrBold Attribute = 1 << (iota + 9)
        AttrUnderline
        AttrReverse
)

However, v1.1.1, marked as minimum required version by gotop, had the code above changed to:

const (
        AttrBold Attribute = 1 << (iota + 9)
        AttrBlink
        AttrHidden
        AttrDim
        AttrUnderline
        AttrCursive
        AttrReverse
        max_attr
)

As can be seen, what was AttrReverse in v0.0.0-20190121233118-02980233997d, became AttrHidden in v1.1.1. It's also not hard to notice why changing termui's ReverseModifier constant to 1 << 15 inside gotop's code worked as a workaround.

termui correctly marked minimum required version of termbox-go as v0.0.0-20190121233118-02980233997d, but dependency bump in c3aeb75 caused a compatibility issue. Instead of removing termbox-go from go.mod and letting termui manage it, I lowered the minimum required version to v0.0.0-20200418040025-38ba6e5628f1 in order to avoid possible regression on FreeBSD (#95).

Fixes #226
Fixes #235

@xxxserxxx
Copy link
Owner

Running the rather byzantine cross-platform build process fails compiling smart.go on Darwin. This may not be a result of the patch, but I have to dig into and resolve it before I can merge this.

Thanks for the well-written PR; I agree with the approach as a stop-gap for resolving the conflicts.

@xxxserxxx
Copy link
Owner

Everything seems to build now, so merged and pushed. Thanks again.

@xxxserxxx xxxserxxx merged commit 3e50a67 into xxxserxxx:master Mar 30, 2023
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.

$TERM rendering bug Selection fails to highlight
2 participants