tigrawap / slit

slit - a modern PAGER for viewing logs, get more than most in less time
Other
469 stars 19 forks source link

Ansi attribute updates #50

Closed bcicen closed 6 years ago

bcicen commented 6 years ago
tigrawap commented 6 years ago

Hey, thanks for the update

Although, it seems that it broke behaviour

TigraNode.lan                      |[1;37mgetting thread tree[0m
TigraNode.lan                      |[1;30mDONE[0m in [0;35mno-time[0m (getting thread tree)

This gets "blinking" mode instead of bold

bcicen commented 6 years ago

which sequence is resulting in blinking? I tested with a few different terminal emulators and can't replicate: test

tigrawap commented 6 years ago

macos, iterm, breaks on all 1; sequences

Removing this line actually fixes it.

+   bg = bg | style

And I don't think it's necessary for other terms as well, I think they just don't support style for background Can you verify that it works the same on your terminals?

That's actually a reason why in original code applied style only to foreground, both when setting initial colors and when inverting for highlighting

            if attr.Fg != 0 {
                fg = termbox.Attribute(attr.Fg-30+1) | stylesMap[attr.Style]
            }
            if attr.Bg != 0 {
                bg = termbox.Attribute(attr.Bg - 40 + 1)
            }
...
...
...
                        if highlightStyle != termbox.Attribute(0) {
                fg = fg | highlightStyle
            }

Also, could you explain shift-3 part? not sure when it actually does anything (both fg and bg)

        if style == termbox.AttrBold {
            bg = bg | 1<<3
        }
bcicen commented 6 years ago

You are correct -- style never needs to be applied to background (and behaves badly in iterm); removed in latest push.

As the comment above it mentions, we want to use the "light" version of a basic color when the bold attribute is applied, which is a shift-3 away in the color table. e.g. black (termui.Attribute(1)) -> light black (termui.Attribute(9))

tigrawap commented 6 years ago

Interesting, ok then :)

btw, like to your XinY, I'm lazy, just googling, gonna fetch it, less reasons to leave term