feat(ui): enhance color styling with adaptive colors and improve term… #22
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,11 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| package ui | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||||||
| "os" | ||||||||||||||||||||||||||||||||||||||||||||
| "strings" | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| "github.com/charmbracelet/lipgloss" | ||||||||||||||||||||||||||||||||||||||||||||
| "golang.org/x/term" | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Table renders data in a compact markdown-style table format. | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -41,6 +43,61 @@ func (t *Table) ColumnWidths() []int { | |||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Auto-constrain to terminal width when MaxWidth is not set | ||||||||||||||||||||||||||||||||||||||||||||
| if t.MaxWidth == 0 { | ||||||||||||||||||||||||||||||||||||||||||||
| termWidth := GetTerminalWidth() | ||||||||||||||||||||||||||||||||||||||||||||
| // Account for leading space + column separators (2 chars between each column) | ||||||||||||||||||||||||||||||||||||||||||||
| overhead := 1 | ||||||||||||||||||||||||||||||||||||||||||||
| if len(widths) > 1 { | ||||||||||||||||||||||||||||||||||||||||||||
| overhead += (len(widths) - 1) * 2 | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| available := termWidth - overhead | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+46
to
+54
There was a problem hiding this comment. Overhead formula underestimates when When
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: internal/ui/table.go
Line: 46-51
Comment:
**Overhead formula underestimates when `len(widths) == 0`**
When `t.Headers` is empty the guard in `Render()` returns early, so `ColumnWidths()` will never reach this block in the current code. However, `ColumnWidths()` is a public method and can be called directly on a zero-header `Table`. In that case:
```
overhead = 1 + (0 - 1) * 2 → 1 + (-2) → -1
available = termWidth - (-1) → termWidth + 1
```
`available > 0` is satisfied but the `total` loop over an empty slice produces `0 > available` → false, so no crash occurs. Still, `-1` overhead is semantically wrong and makes the guard appear broader than it is. A minimal defensive fix:
```suggestion
// Account for leading space + column separators (2 chars between each column)
overhead := 1
if len(widths) > 1 {
overhead += (len(widths) - 1) * 2
}
available := termWidth - overhead
```
How can I resolve this? If you propose a fix, please make it concise.
Owner
Author
There was a problem hiding this comment. Fixed — overhead formula now guards against len(widths) <= 1 to avoid negative overhead. |
||||||||||||||||||||||||||||||||||||||||||||
| if available > 0 { | ||||||||||||||||||||||||||||||||||||||||||||
| total := 0 | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+53
to
+56
There was a problem hiding this comment. Mixed byte-count vs. terminal-column comparison in auto-constrain logic
The fix is to measure visual width using // Instead of len(h) / len(cell), use:
import "github.com/charmbracelet/lipgloss"
// In ColumnWidths():
for i, h := range t.Headers {
widths[i] = lipgloss.Width(h)
}
for _, row := range t.Rows {
for i, cell := range row {
if i < len(widths) && lipgloss.Width(cell) > widths[i] {
widths[i] = lipgloss.Width(cell)
}
}
}
Prompt To Fix With AIThis is a comment left during a code review.
Path: internal/ui/table.go
Line: 53-56
Comment:
**Mixed byte-count vs. terminal-column comparison in auto-constrain logic**
`widths[i]` values are populated using `len(h)` (header) and `len(cell)` (row cells), which return **byte lengths**. `GetTerminalWidth()` returns **terminal columns** (visual width). For pure ASCII these are equivalent, but multi-byte UTF-8 content (e.g., emoji in headers like `"🔍 Symbol"` used in `explain.go`, or CJK characters) will cause `total` (bytes) to exceed `available` (columns) even when the table would visually fit, triggering unnecessary proportional shrinking. In extreme cases with wide characters, columns get over-shrunk and the table renders narrower than the terminal.
The fix is to measure visual width using `lipgloss.Width(s)` (which accounts for Unicode and ANSI sequences) when calculating `widths`:
```go
// Instead of len(h) / len(cell), use:
import "github.com/charmbracelet/lipgloss"
// In ColumnWidths():
for i, h := range t.Headers {
widths[i] = lipgloss.Width(h)
}
for _, row := range t.Rows {
for i, cell := range row {
if i < len(widths) && lipgloss.Width(cell) > widths[i] {
widths[i] = lipgloss.Width(cell)
}
}
}
```
`padRight` has the same `len(s)` issue — it should also use `lipgloss.Width(s)` to avoid producing visually misaligned padding for Unicode content.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||
| for _, w := range widths { | ||||||||||||||||||||||||||||||||||||||||||||
| total += w | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| if total > available { | ||||||||||||||||||||||||||||||||||||||||||||
| // Proportionally shrink columns, but keep a minimum of 4 chars | ||||||||||||||||||||||||||||||||||||||||||||
| ratio := float64(available) / float64(total) | ||||||||||||||||||||||||||||||||||||||||||||
| for i := range widths { | ||||||||||||||||||||||||||||||||||||||||||||
| newW := int(float64(widths[i]) * ratio) | ||||||||||||||||||||||||||||||||||||||||||||
| if newW < 4 { | ||||||||||||||||||||||||||||||||||||||||||||
| newW = 4 | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| widths[i] = newW | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| // Post-clamp: if min-floor caused overflow, trim widest columns | ||||||||||||||||||||||||||||||||||||||||||||
| for { | ||||||||||||||||||||||||||||||||||||||||||||
| postTotal := 0 | ||||||||||||||||||||||||||||||||||||||||||||
| for _, w := range widths { | ||||||||||||||||||||||||||||||||||||||||||||
| postTotal += w | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| excess := postTotal - available | ||||||||||||||||||||||||||||||||||||||||||||
| if excess <= 0 { | ||||||||||||||||||||||||||||||||||||||||||||
| break | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| // Find widest column and shrink it | ||||||||||||||||||||||||||||||||||||||||||||
| maxIdx, maxW := 0, 0 | ||||||||||||||||||||||||||||||||||||||||||||
| for i, w := range widths { | ||||||||||||||||||||||||||||||||||||||||||||
| if w > maxW { | ||||||||||||||||||||||||||||||||||||||||||||
| maxIdx, maxW = i, w | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| // Don't shrink below minimum | ||||||||||||||||||||||||||||||||||||||||||||
| if maxW <= 4 { | ||||||||||||||||||||||||||||||||||||||||||||
| break | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| shrink := excess | ||||||||||||||||||||||||||||||||||||||||||||
| if shrink > maxW-4 { | ||||||||||||||||||||||||||||||||||||||||||||
| shrink = maxW - 4 | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| widths[maxIdx] -= shrink | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+60
to
+98
There was a problem hiding this comment. Minimum-clamp can still produce overflowing column totals. After applying the proportional Example: A follow-up validation is needed after the clamp loop to either re-trim the widest columns or document that overflow is accepted:
Suggested change
At minimum, add a comment documenting that the 4-char floor can cause overflow. Prompt To Fix With AIThis is a comment left during a code review.
Path: internal/ui/table.go
Line: 57-68
Comment:
Minimum-clamp can still produce overflowing column totals.
After applying the proportional `ratio` (line 59), each column is floored to a minimum of 4 characters (lines 62–64). With many columns, the total `4*n + overhead` can exceed `termWidth`, meaning the table will overflow even after the "constrain" pass. There is no post-clamp overflow check.
Example: `termWidth=80`, 15 columns, overhead=29, available=51, ratio≈0.34. Each `newW = int(10*0.34)=3`, clamped to `4`. Post-clamp total = `4*15+29 = 89 > 80`.
A follow-up validation is needed after the clamp loop to either re-trim the widest columns or document that overflow is accepted:
```suggestion
// After the minimum-clamp loop, check if post-clamp total still exceeds available
postClampTotal := 0
for _, w := range widths {
postClampTotal += w
}
if postClampTotal > available {
// Post-clamp overflow detected. Accept gracefully or trim.
// Consider logging a debug message or trimming extra from widest columns.
}
```
At minimum, add a comment documenting that the 4-char floor can cause overflow.
How can I resolve this? If you propose a fix, please make it concise.
Owner
Author
There was a problem hiding this comment. Fixed — added post-clamp overflow loop that trims widest columns until total fits within available width. |
||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| return widths | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -101,6 +158,20 @@ func padRight(s string, width int) string { | |||||||||||||||||||||||||||||||||||||||||||
| return s + strings.Repeat(" ", width-len(s)) | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // GetTerminalWidthFor returns the terminal width for the given file descriptor, defaulting to 80. | ||||||||||||||||||||||||||||||||||||||||||||
| func GetTerminalWidthFor(f *os.File) int { | ||||||||||||||||||||||||||||||||||||||||||||
| w, _, err := term.GetSize(int(f.Fd())) | ||||||||||||||||||||||||||||||||||||||||||||
| if err != nil || w <= 0 { | ||||||||||||||||||||||||||||||||||||||||||||
| return 80 | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| return w | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // GetTerminalWidth returns the current stdout terminal width, defaulting to 80. | ||||||||||||||||||||||||||||||||||||||||||||
| func GetTerminalWidth() int { | ||||||||||||||||||||||||||||||||||||||||||||
| return GetTerminalWidthFor(os.Stdout) | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // TruncateID shortens an ID for display (first 6 chars). | ||||||||||||||||||||||||||||||||||||||||||||
| func TruncateID(id string) string { | ||||||||||||||||||||||||||||||||||||||||||||
| if len(id) > 6 { | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highly specific benchmark claims lack reproducibility caveats
The figures
25,000 tokens,1,500 tokens,42 seconds, and3 minutesare labeled "Real session, real numbers" but will vary significantly by repository size, AI model, query complexity, network latency, and TaskWing knowledge-base completeness. Presenting single-datapoint figures as representative benchmarks without any caveat could mislead users whose results differ substantially.Consider adding a qualifier such as:
Or add a footnote explaining the test environment (repo LOC, model used, knowledge-base node count).
Prompt To Fix With AI