feat: Make the analyzer's colour carry meaning too The browser reads its values by data-type family; the analyzer had only inherited the two bands. Now the three places it puts information on screen say what that information is. A verdict is coloured by what it means: a defect red, a clean round trip green, and a skip or a filtered packet muted, because those are not findings and must not read as though they were. That distinction is the one the demo exists to show, and until now the column stated it in words alone. The two directions get colours as well as glyphs, so an exchange can be followed down the column rather than a row at a time. Getting there meant building each row from styled cells rather than styling the row as a whole, which is the same change the browser's message table needed and for the same reason: a terminal ends a style at the next reset, so a row styled as a whole loses its styling at the first cell that colours itself. The verdict keeps its colour even on the selected row -- it is the answer the tool exists to give, and five other cells plus the marker already say which row is selected. A hex dump is three things on one line: where you are, what is there, and what it reads as in text. Only the middle is the data, and drawing all three alike made the row a wall to parse before anything could be looked up. The offset and the text pane are now context. And plc4x renders its parse trees as nested boxes, which is what makes them readable at all -- but undifferentiated the frames outweigh what is inside them and a screenful reads as texture rather than structure. The frames drop to the chrome colour. Only the Unicode box-drawing block is dimmed, deliberately: the ASCII characters a box could be drawn from occur in payload text too, and dimming those would dim data. Every assertion was checked against a mutation of the code it covers, and the existing NO_COLOR guard already covers the lot: none of this reaches a terminal that asked for no colour.
diff --git a/plc4go/tools/QUICKSTART.md b/plc4go/tools/QUICKSTART.md index ea95c1d..7732c7d 100644 --- a/plc4go/tools/QUICKSTART.md +++ b/plc4go/tools/QUICKSTART.md
@@ -157,8 +157,8 @@ | Key | View | | --- | --- | -| `b` | raw bytes | -| `t` | parsed tree | +| `b` | raw bytes -- the offset and the text pane are dimmed, so the bytes are what you read | +| `t` | parsed tree -- plc4x draws it as nested boxes, dimmed so the field names stand out of them | | `d` | diff (the default) | On a mismatch the diff shows the original against the reserialized bytes with the first
diff --git a/plc4go/tools/README.md b/plc4go/tools/README.md index d62ab84..b83c426 100644 --- a/plc4go/tools/README.md +++ b/plc4go/tools/README.md
@@ -185,6 +185,15 @@ | A count worth glancing at | the number in the accent, its label muted; a failure count red | | The top and bottom bars | a raised band, framing the panes between them | +In the analyzer specifically: + +| What | How it reads | +| --- | --- | +| A verdict | `ok` green, a defect red, and a skip or a filtered packet muted -- those are not findings and must not read as though they were | +| A direction | request and reply in different colours, so a exchange can be followed down the column rather than a row at a time | +| A hex dump | the offset and the text pane muted, the bytes at full contrast: only the middle column is the data | +| A parse tree | plc4x draws it as nested boxes; the frames drop to the chrome colour so the field names and values are the only thing at full contrast | + Two implementation notes, because both have already caused bugs: A terminal cannot nest a background. An inner style's reset ends the outer background @@ -193,7 +202,8 @@ *inside* a styled run rather than beside one. A bare space between two styled pieces shows the terminal's own background through the band and stripes the bar. -For the same reason `bubbles/table` gets a bare cell style. It styles each cell and then styles +For the same reason the analyzer builds its packet rows from styled cells rather than styling +the row, and `bubbles/table` gets a bare cell style. It styles each cell and then styles the selected row around them, so any foreground on the cell style ends with a reset that wipes the selection from every cell after the first, leaving the selected row indistinguishable.
diff --git a/plc4go/tools/plc4xpcapanalyzer/ui/appearance_test.go b/plc4go/tools/plc4xpcapanalyzer/ui/appearance_test.go index 8927286..76c6d0a 100644 --- a/plc4go/tools/plc4xpcapanalyzer/ui/appearance_test.go +++ b/plc4go/tools/plc4xpcapanalyzer/ui/appearance_test.go
@@ -93,3 +93,161 @@ } } } + +// --- colour that carries meaning --- + +// TestTheVerdictColumnIsColouredByWhatItMeans is the point of the column: a defect is red, a +// clean round trip green, and a skip neither, because a skip is not a finding and must not read +// as though it were. +func TestTheVerdictColumnIsColouredByWhatItMeans(t *testing.T) { + theme := tui.NewTheme(tui.Options{Dark: true}) + state, _ := demoState(t) + m := newTestModelWithTheme(t, state, wide, tui.Options{Dark: true}) + + for verdict, want := range map[Verdict]string{ + VerdictOK: theme.Ok.Render("x"), + VerdictParseFail: theme.Err.Render("x"), + VerdictSerializeFail: theme.Err.Render("x"), + VerdictBytesDiffer: theme.Err.Render("x"), + VerdictSkipped: theme.Muted.Render("x"), + VerdictFiltered: theme.Muted.Render("x"), + VerdictNoPayload: theme.Muted.Render("x"), + } { + assert.Equal(t, want, m.verdictStyle(verdict).Render("x"), verdict.String()) + } + + // And the three classes are actually distinguishable from one another. + ok := m.verdictStyle(VerdictOK).Render("x") + issue := m.verdictStyle(VerdictParseFail).Render("x") + quiet := m.verdictStyle(VerdictSkipped).Render("x") + assert.NotEqual(t, ok, issue) + assert.NotEqual(t, ok, quiet) + assert.NotEqual(t, issue, quiet) +} + +// TestAPacketRowIsStyledPerCell guards the defect that made the browser's selected row +// invisible: a terminal ends a style at the next reset, so a row styled as a whole loses its +// styling at the first cell that colours itself. +func TestAPacketRowIsStyledPerCell(t *testing.T) { + theme := tui.NewTheme(tui.Options{Dark: true}) + m := newTestModelWithTheme(t, mustState(t), wide, tui.Options{Dark: true}) + columns := packetLayout(wide.Width) + + row := m.renderPacketRow(okRecord(1), true, columns, wide.Width) + plain := tuitest.Strip(row) + + // The selection reaches the message column, which sits after the cells that colour + // themselves. Styling the row as a whole would have ended it at the first of them. + selection := tuitest.Escapes.FindString(theme.SelectedRow.Render("x")) + require.NotEmpty(t, selection) + message := strings.Index(plain, "CBusMessageToServer") + require.Positive(t, message, "the fixture has to have a message to look for") + assert.Contains(t, styleAt(row, message), selection, + "the selection has to still be in force at the message column") + + // The verdict keeps its own colour even on the selected row: it is the answer the tool + // exists to give. + verdict := strings.Index(plain, "ok") + require.Positive(t, verdict) + assert.Contains(t, styleAt(row, verdict), tuitest.Escapes.FindString(theme.Ok.Render("x")), + "a clean round trip stays green on the selected row") +} + +// TestARowIsExactlyTheWidthWhateverItsColours is the arithmetic that styling must not disturb. +func TestARowIsExactlyTheWidthWhateverItsColours(t *testing.T) { + m := newTestModelWithTheme(t, mustState(t), wide, tui.Options{Dark: true}) + for _, width := range []int{150, 120, 90, 70, 50} { + columns := packetLayout(width) + for _, record := range []Record{okRecord(1), brokenRecord(2)} { + for _, selected := range []bool{true, false} { + row := m.renderPacketRow(record, selected, columns, width) + assert.Equal(t, width, lipgloss.Width(row), + "%s selected=%v at %d columns", record.Verdict, selected, width) + } + } + } +} + +// TestAHexLineSeparatesTheBytesFromTheScaffolding checks the dump reads as data rather than as +// a wall: where you are and what it says as text are context, and only the middle is the data. +func TestAHexLineSeparatesTheBytesFromTheScaffolding(t *testing.T) { + m := newTestModelWithTheme(t, mustState(t), wide, tui.Options{Dark: true}) + line := HexDumpLines([]byte("322100AD\r\n"), hexBytesPerRow, 4)[0] + rendered := m.renderHexLine(line) + + assert.Equal(t, line, tuitest.Strip(rendered), "styling must not change a single character") + + plain := tuitest.Strip(rendered) + offset := styleAt(rendered, strings.Index(plain, "00000000")) + data := styleAt(rendered, strings.Index(plain, "33 32")) + text := styleAt(rendered, strings.Index(plain, "|")) + + assert.NotEqual(t, offset, data, "the offset should not read as loudly as the bytes") + assert.NotEqual(t, text, data, "nor should the text pane") + assert.Equal(t, offset, text, "the two kinds of context should read alike") +} + +// TestATreeLineDimsItsFrames is what makes plc4x's parse trees readable rather than textural: +// undifferentiated, the boxes outweigh what is inside them. +func TestATreeLineDimsItsFrames(t *testing.T) { + m := newTestModelWithTheme(t, mustState(t), wide, tui.Options{Dark: true}) + line := "╔═CBusMessage/request═╗" + rendered := m.renderTreeLine(line) + + assert.Equal(t, line, tuitest.Strip(rendered), "styling must not change a single character") + + plain := tuitest.Strip(rendered) + frame := styleAt(rendered, strings.Index(plain, "╔")) + content := styleAt(rendered, strings.Index(plain, "CBusMessage")) + assert.NotEqual(t, frame, content, "the frame and its contents must not read alike") + + // A line with no frame at all is left entirely at full contrast. + assert.Equal(t, content, styleAt(m.renderTreeLine("0x7e 126 RESET"), 0)) +} + +// TestOnlyRealBoxDrawingIsDimmed is the deliberate limit: the ASCII characters a box could be +// drawn from occur in payload text too, and dimming those would dim data. +func TestOnlyRealBoxDrawingIsDimmed(t *testing.T) { + for _, r := range []rune{'═', '║', '╔', '╝', '┄', '┆', '─', '│'} { + assert.True(t, isBoxDrawing(r), "%c is box drawing", r) + } + for _, r := range []rune{'-', '|', '+', '~', '0', 'x', '/', ' ', 'A'} { + assert.False(t, isBoxDrawing(r), "%c is not box drawing and may carry data", r) + } +} + +// styleAt returns the styling in force at a cell offset of a rendered line, as the terminal +// would have it: a style holds until something replaces or resets it. +func styleAt(rendered string, cell int) string { + if cell < 0 { + return "" + } + current, seen, rest := "", 0, rendered + for rest != "" { + if location := tuitest.Escapes.FindStringIndex(rest); location != nil && location[0] == 0 { + current = rest[:location[1]] + rest = rest[location[1]:] + continue + } + next := len(rest) + if location := tuitest.Escapes.FindStringIndex(rest); location != nil { + next = location[0] + } + for _, r := range rest[:next] { + if seen == cell { + return current + } + seen++ + _ = r + } + rest = rest[next:] + } + return current +} + +// mustState is a demo session, for the renderers that only need a model to hang off. +func mustState(t *testing.T) *State { + t.Helper() + state, _ := demoState(t) + return state +}
diff --git a/plc4go/tools/plc4xpcapanalyzer/ui/view.go b/plc4go/tools/plc4xpcapanalyzer/ui/view.go index e9a2aaa..87d3e69 100644 --- a/plc4go/tools/plc4xpcapanalyzer/ui/view.go +++ b/plc4go/tools/plc4xpcapanalyzer/ui/view.go
@@ -746,38 +746,79 @@ if selected { marker = m.theme.Glyphs.Selected } + // One style per cell rather than one for the whole row. A terminal ends a style at the next + // reset, so a semantic colour anywhere in the row would drop the row's own styling for every + // cell after it -- the defect that made the browser's selected row invisible. Building the + // row from styled cells says what each column means and keeps the selection intact. + rowStyle := m.theme.Value + if selected { + rowStyle = m.theme.SelectedRow + } + cells := make([]string, 0, len(columns)) for _, column := range columns { size := m.columnWidth(column, columns, width) - value := "" + value, style := "", rowStyle switch column.title { case "no.": - value = itoa(record.Number) + value, style = itoa(record.Number), m.contextStyle(selected) case "time": - value = formatOffset(record.Offset) + value, style = formatOffset(record.Offset), m.contextStyle(selected) case "dir": value = m.directionGlyph(record.Direction) + if !selected { + style = m.directionStyle(record.Direction) + } case "proto": - value = record.Protocol + value, style = record.Protocol, m.contextStyle(selected) case "message": value = record.Summary if value == "" { value = record.Reason } case "verdict": - value = record.Verdict.String() + // The verdict keeps its own colour even on the selected row: it is the answer the + // tool exists to give, and five other cells plus the marker already say which row + // is selected. + value, style = record.Verdict.String(), m.verdictStyle(record.Verdict) } - cells = append(cells, pad(shorten(value, size, m.theme.Glyphs.Ellipsis), size)) + cells = append(cells, style.Render(pad(shorten(value, size, m.theme.Glyphs.Ellipsis), size))) } - row := marker + " " + strings.Join(cells, " ") - style := m.theme.Value + + row := rowStyle.Render(marker+" ") + strings.Join(cells, rowStyle.Render(" ")) + return fitLine(padOnto(rowStyle, row, width), width) +} + +// contextStyle is for a cell carrying context rather than substance -- the number, the offset, +// the protocol. On the selected row it stays with the selection, so the row reads as one thing. +func (m Model) contextStyle(selected bool) lipgloss.Style { + if selected { + return m.theme.SelectedRow + } + return m.theme.Muted +} + +// verdictStyle colours an outcome by what it means: a defect is red, a clean round trip green, +// and everything else -- skipped, filtered, no payload -- is muted, because those are not +// findings and should not read as though they were. +func (m Model) verdictStyle(verdict Verdict) lipgloss.Style { switch { - case selected: - style = m.theme.SelectedRow - case record.Verdict.IsIssue(): - style = m.theme.Warn + case verdict.IsIssue(): + return m.theme.Err + case verdict == VerdictOK: + return m.theme.Ok + default: + return m.theme.Muted } - return fitLine(style.Render(pad(row, width)), width) +} + +// directionStyle distinguishes the two directions by colour as well as by glyph, so a request +// and its reply can be told apart down the column rather than one row at a time. +func (m Model) directionStyle(direction Direction) lipgloss.Style { + if direction == DirectionResponse { + return m.theme.Temporal + } + return m.theme.Numeric } // columnWidth resolves a column's width, giving the message column whatever is left. @@ -972,11 +1013,49 @@ top := clampIndex(m.detailTop, len(source)) out := make([]string, 0, rows) for i := top; i < min(top+rows, len(source)); i++ { - out = append(out, fitLine(m.theme.Value.Render(shorten(source[i], width, m.theme.Glyphs.Ellipsis)), width)) + out = append(out, fitLine(m.renderTreeLine(shorten(source[i], width, m.theme.Glyphs.Ellipsis)), width)) } return padRows(out, width, rows) } +// renderTreeLine dims the box-drawing plc4x renders its parse trees with. +// +// The tree arrives already drawn as nested boxes, which is what makes it readable at all. But +// undifferentiated, the frames outweigh what is inside them and a screenful reads as texture +// rather than as structure. Dropping the frames to the chrome colour leaves the field names and +// their values as the only thing at full contrast. +func (m Model) renderTreeLine(line string) string { + var out strings.Builder + var run []rune + runIsFrame := false + flush := func() { + if len(run) == 0 { + return + } + style := m.theme.Value + if runIsFrame { + style = m.theme.Chrome + } + out.WriteString(style.Render(string(run))) + run = run[:0] + } + for _, r := range line { + if frame := isBoxDrawing(r); frame != runIsFrame { + flush() + runIsFrame = frame + } + run = append(run, r) + } + flush() + return out.String() +} + +// isBoxDrawing reports whether a rune is in the Unicode block plc4x draws its trees with. +// +// That block only, deliberately: the ASCII characters a box could be drawn from -- the hyphen, +// the pipe, the plus -- occur in payload text too, and dimming those would dim data. +func isBoxDrawing(r rune) bool { return r >= 0x2500 && r <= 0x257F } + // hexBlock renders up to rows of a hex dump, starting at a row offset. func (m Model) hexBlock(data []byte, top, rows, width int) []string { if rows < 1 { @@ -986,11 +1065,29 @@ top = clampIndex(top, len(all)) out := make([]string, 0, rows) for i := top; i < min(top+rows, len(all)); i++ { - out = append(out, fitLine(m.theme.Value.Render(all[i]), width)) + out = append(out, fitLine(m.renderHexLine(all[i]), width)) } return out } +// renderHexLine dims the scaffolding of a hex dump so the bytes stand out of it. +// +// A dump is three things on one line: where you are, what is there, and what it reads as in +// text. Only the middle one is the data, and drawing all three in the same colour makes the row +// a wall the eye has to parse before it can look anything up. The rest is context. +func (m Model) renderHexLine(line string) string { + offset, rest, split := strings.Cut(line, " ") + if !split { + return m.theme.Value.Render(line) + } + rendered := m.theme.Muted.Render(offset) + m.theme.Value.Render(" ") + data, text, hasText := strings.Cut(rest, "|") + if !hasText { + return rendered + m.theme.Value.Render(rest) + } + return rendered + m.theme.Value.Render(data) + m.theme.Muted.Render("|"+text) +} + // hexRowFrom renders one row of a hex dump, tolerating an offset past the end of the data. func hexRowFrom(data []byte, from, bytesPerRow int) string { if from >= len(data) {