Protect URL columns from being dropped in selectColumns() - #320
Conversation
github-actions
Bot
added
tests
![cubic-dev-ai[bot]](https://cdn.github.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Updates the generic table renderer’s column-selection logic to ensure URL-bearing columns are not removed when the table exceeds terminal width, prioritizing clickability over strict width fitting.
Changes:
- Track whether each column contains URL values during width measurement (
containsURL). - Update the column-dropping loop to skip URL columns and only drop non-URL columns.
- Add/adjust unit tests to verify URL width handling and that URL columns survive overflow scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/output/render.go | Adds per-column URL detection and prevents URL columns from being dropped during width-based column removal. |
| internal/output/output_test.go | Adds tests covering URL width exemption and URL-column preservation under overflow conditions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The column-dropping loop removed URL columns first (lowest priority) when their accurate width pushed the total past the terminal width. A dropped URL is worse than a garbled one — an overflowing table at least keeps the link clickable. Now the loop skips URL columns and drops only non-URL columns from the right.
Add a found-or-fail assertion so the test actually fails if the URL column gets dropped instead of silently passing with zero assertions.
1f719b0 to
8ea3dd0
Compare
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ea3dd06af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The fixture totalled 79 chars which fit within width 80, so the drop loop never fired and the test passed vacuously with both old and new logic. Use width 60 to force the overflow path.
There was a problem hiding this comment.
Pull request overview
This PR updates the generic table column-selection logic so URL-bearing columns are preserved even when the table exceeds terminal width, ensuring links remain usable/clickable in styled output.
Changes:
- Update
selectColumns()to drop the rightmost non-URL column when over the width budget (instead of always dropping the rightmost column). - Restore a longer real-world URL in
TestStyledRenderTablePreservesURLsnow that URL columns aren’t removed. - Add a focused unit test ensuring URL columns survive the column-dropping loop when overflow persists.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/output/render.go | Adjusts the column-dropping loop to preserve URL columns under width overflow. |
| internal/output/output_test.go | Updates URL preservation fixture and adds a new overflow regression test for URL columns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
selectColumns()now skips URL columns instead of removing them when the table exceeds terminal width. A dropped URL is useless; an overflowing table at least keeps the link clickable.TestStyledRenderTablePreservesURLs(the short URL was a workaround for the old dropping behavior).TestSelectColumnsPreservesURLColumnsWhenOverflowingto directly verify URL columns survive the drop loop.Follows up on #309 which fixed the width budget mismatch but revealed this worse problem — accurate URL widths could cause the column to be dropped entirely.
Test plan
Summary by cubic
Keep URL columns visible by skipping the 40-char cap and preventing them from being dropped in
selectColumns(). This keeps links clickable even when the table exceeds terminal width.formatTableCell()and flag columns withcontainsURL.Written for commit 9939dda. Summary will update on new commits.