xxxserxxx / gotop

A terminal based graphical activity monitor inspired by gtop and vtop
Other
2.76k stars 140 forks source link

Revert termbox-go dependency bump #247

Closed gardockt closed 1 year ago

gardockt commented 1 year ago

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 c3aeb753d9355fcf3d5e6cbbdb892e61470e5abf 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 commented 1 year ago

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 commented 1 year ago

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