Skip to content

Fix unresolved review issues in WidthStrategy implementations #570

@apstndb

Description

@apstndb

Summary

Address two unresolved review comments from #569 (merged) that were identified after merge.

Issues

1. GreedyFrequencyStrategy: potential panic on short rows

File: internal/mycli/format/width_strategy_greedy.go:54

lo.Must(lo.Nth(in, columnIdx)) will panic if a row has fewer columns than len(headers). The other strategies (ProportionalStrategy, MarginalCostStrategy) already have bounds checks. The fix is to add a similar guard:

func(in Row) string {
    if columnIdx < len(in) {
        return in[columnIdx].RawText()
    }
    return ""
},

2. ProportionalStrategy: remaining width not distributed when totalDeficit == 0

File: internal/mycli/format/width_strategy_proportional.go:95

When the initial header-based allocation already covers the natural width of all columns (totalDeficit == 0), the remaining width is not distributed at all, resulting in a table narrower than available screen width. The fix is to restructure the distribution block so that leftover is always assigned even when totalDeficit == 0.

References

Scope

  • Add bounds check in GreedyFrequencyStrategy.CalculateWidths()
  • Fix ProportionalStrategy to distribute remaining width when totalDeficit == 0
  • Add test cases for both edge cases

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions