long urls break tui by SuperCoolPencil · Pull Request #415 · SurgeDM/Surge

SuperCoolPencil · 2026-04-25T04:52:32Z

refactor: export TruncateMiddle and implement TruncateTwoLines for TUI layout consistency
refactor: truncate detail text to two lines instead of wrapping in confirmation modal
refactor: replace text wrapping with two-line truncation and improve string truncation logic across TUI views
refactor: update TruncateTwoLines to use character-based wrapping and add unit tests
chore: go mod tidy
feat: make text truncation utilities ANSI-aware and add support for visual width calculations
feat: implement dynamic modal sizing and improve TUI layout robustness by adding title truncation and overflow safety checksd
refactor: transition chunk map visibility logic from hardcoded height thresholds to dynamic content measurement

Greptile Summary

This PR fixes long URLs breaking the TUI and extension by introducing ANSI-aware two-line truncation (TruncateTwoLines, TruncateMiddle exported) across detail panes, modals, the download list, settings, and the category manager, replacing unbounded WrapText calls. It also adds dynamic chunk-map suppression, modal size clamping, and a comprehensive 698-line layout regression test suite covering all terminal sizes and modal states.

Confidence Score: 5/5

Safe to merge — only P2 findings, no runtime breakage identified.

All findings are P2 (the done=true dead-code path in TruncateTwoLines and the WrapText inconsistency on the ID field). Core truncation logic and ANSI handling are correct as verified by analysis and backed by a thorough regression suite.

internal/utils/text.go (latent done=true bug at line 272); internal/tui/view.go (ID field WrapText inconsistency at line 532)

Important Files Changed

Filename	Overview
internal/utils/text.go	Adds ANSI-aware truncation utilities (TruncateMiddle exported, TruncateTwoLines, getCharInfos, getAnsiState, stringWidth). Core logic is correct; one latent bug where done=true silently drops partially-built line-2 content.
internal/utils/text_test.go	Adds TestTruncateTwoLines (3 cases) and TestAnsiAwareness (4 sub-tests). ANSI assertions are correct for the new implementation. Missing edge-case coverage (width≤0, content>2*width) was flagged in a prior review.
internal/tui/view.go	Switches URL/File/Path detail fields to TruncateTwoLines; adds dynamic chunk-map suppression logic; applies GetDynamicModalDimensions to three bug-report modals. ID field inconsistently left using WrapText.
internal/tui/layout_regression_test.go	New regression suite (698 lines, 13 test groups) enforcing width/height invariants across all terminal sizes, modal states, and extreme edge cases. Comprehensive coverage of the core layout contract.
internal/tui/components/box.go	Adds title overflow guards (truncate left then right via MaxWidth, suppress both when width is too small) and clamps innerHeight to ≥0. Logic is sound and tested by the regression suite.
internal/tui/layout_helpers.go	Removes hardcoded height threshold for chunk-map visibility; decision deferred to View() via dynamic content measurement. Clean refactor with no correctness issues.
internal/tui/view_settings.go	Switches setting-value and description rendering to TruncateTwoLines, computes available value width dynamically, and changes label truncation to TruncateMiddle. No issues found.
internal/tui/view_category.go	Switches all four category-detail fields (Name, Description, Pattern, Path) from WrapText to TruncateTwoLines. Straightforward and consistent.
internal/tui/helpers.go	Log entries now truncated to two lines instead of fully wrapped; intentional trade-off confirmed by author reply in prior thread.
internal/tui/list.go	Prefix width now subtracted from available content width, title uses TruncateMiddle instead of Truncate. Fixes the root cause of URL overflow in the download list.
go.mod	Promotes go-runewidth and BurntSushi/toml from indirect to direct dependencies, matching their actual direct usage.

Comments Outside Diff (2)

internal/utils/text_test.go, line 1313-1321 (link)

ANSI test assertions don't match implementation behavior

Both assertions in TestAnsiAwareness are incorrect and will fail.

truncateToWidth explicitly does not append a reset (comment: "allow ANSI states (colors) to bleed into the next line"), so the actual output for truncateToWidth(text, 3) is "\x1b[31mhel", not "\x1b[31mhel\x1b[0m".
TruncateMiddle builds the right portion from the tail of the original string. For text = "\x1b[31mhello\x1b[0m" with limit=4, the right portion is the last 2 visible chars: "lo\x1b[0m" — it does not re-apply the leading \x1b[31m. The actual result is "\x1b[31mh\x1b[0m…lo\x1b[0m", not "\x1b[31mh\x1b[0m…\x1b[31mlo\x1b[0m".

These tests will fail CI.

Prompt To Fix With AI

This is a comment left during a code review.
Path: internal/utils/text_test.go
Line: 1313-1321

Comment:
**ANSI test assertions don't match implementation behavior**

Both assertions in `TestAnsiAwareness` are incorrect and will fail.

1. `truncateToWidth` explicitly does **not** append a reset (comment: "allow ANSI states (colors) to bleed into the next line"), so the actual output for `truncateToWidth(text, 3)` is `"\x1b[31mhel"`, not `"\x1b[31mhel\x1b[0m"`.

2. `TruncateMiddle` builds the right portion from the *tail* of the original string. For `text = "\x1b[31mhello\x1b[0m"` with `limit=4`, the right portion is the last 2 visible chars: `"lo\x1b[0m"` — it does not re-apply the leading `\x1b[31m`. The actual result is `"\x1b[31mh\x1b[0m…lo\x1b[0m"`, not `"\x1b[31mh\x1b[0m…\x1b[31mlo\x1b[0m"`.

These tests will fail CI.

How can I resolve this? If you propose a fix, please make it concise.

internal/utils/text_test.go, line 1290-1305 (link)

TruncateTwoLines tests missing key edge cases

The three cases cover a happy path and basic wrapping, but are missing:

width <= 0 (function currently returns the unchanged string, which could pass a very long URL through)
A string whose total length exceeds 2*width (verifying the TruncateMiddle truncation actually fires)
ANSI-containing input (the function calls getCharInfos internally)

Per the team rule, edge cases and integration points should always be tested.

Rule Used: What: All code changes must include tests for edge... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: internal/utils/text_test.go
Line: 1290-1305

Comment:
**`TruncateTwoLines` tests missing key edge cases**

The three cases cover a happy path and basic wrapping, but are missing:
- `width <= 0` (function currently returns the unchanged string, which could pass a very long URL through)
- A string whose total length exceeds `2*width` (verifying the `TruncateMiddle` truncation actually fires)
- ANSI-containing input (the function calls `getCharInfos` internally)

Per the team rule, edge cases and integration points should always be tested.

**Rule Used:** What: All code changes must include tests for edge... ([source](https://app.greptile.com/review/custom-context?memory=2b22782d-3452-4d55-b059-e631b2540ce8))

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: internal/utils/text.go
Line: 272-274

Comment:
**Second line content dropped when `done=true`**

When `done` is `true`, `currentLine` holds the valid, partially-built content of line 2 (everything that fit within `width` before the overflow was detected), but the `!done` guard prevents it from being appended to `lines`. In the current code this path is unreachable — `TruncateMiddle(s, 2*width)` guarantees the content never needs a third line — but if the invariant is ever broken (direct call with un-truncated input, or a future refactor), the second line is silently discarded. Appending unconditionally is safer:

```suggestion
	if currentLine.Len() > 0 {
		lines = append(lines, currentLine.String())
	}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/tui/view.go
Line: 532

Comment:
**`ID` field still uses `WrapText` while sibling fields use `TruncateTwoLines`**

`URL`, `File`, and `Path` were all switched to `TruncateTwoLines`, but `ID` (a UUID) still calls `WrapText`. UUIDs are 36 chars so this won't overflow in practice, but the inconsistency means a theoretical future change to the ID format (e.g. a long custom ID) would break layout while the others would not.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (4): Last reviewed commit: "fix: improve ANSI sequence parsing in te..." | Re-trigger Greptile}

…I layout consistency

…nfirmation modal

…string truncation logic across TUI views

… add unit tests

…isual width calculations

…s by adding title truncation and overflow safety checksd

… thresholds to dynamic content measurement

github-actions · 2026-04-25T04:53:16Z

Binary Size Analysis

⚠️ Size Increased

Version	Human Readable	Raw Bytes
Main	19.36 MB	`20300068`
PR	19.38 MB	`20316452`
Difference	16.00 KB	`16384`

greptile-apps · 2026-04-25T04:57:14Z

-		wrapped := utils.WrapText(entry, width)
-		rendered := wrapStyle.Render(wrapped)
-		wrappedEntries = append(wrappedEntries, strings.Split(rendered, "\n")...)
+		wrapped := utils.TruncateTwoLines(entry, width)


Log entries silently truncated to two lines

Previously, each log entry was wrapped with WrapText, allowing long entries to occupy as many lines as needed in the scrollable viewport. Switching to TruncateTwoLines silently discards any content beyond 2×viewport-width characters. Any entry that legitimately wraps past two lines — such as a long error URL or stack trace — will be cut with a … in the middle. Consider whether WrapText should be retained here.

Prompt To Fix With AI

This is a comment left during a code review. Path: internal/tui/helpers.go Line: 47 Comment: **Log entries silently truncated to two lines** Previously, each log entry was wrapped with `WrapText`, allowing long entries to occupy as many lines as needed in the scrollable viewport. Switching to `TruncateTwoLines` silently discards any content beyond 2×viewport-width characters. Any entry that legitimately wraps past two lines — such as a long error URL or stack trace — will be cut with a `…` in the middle. Consider whether `WrapText` should be retained here. How can I resolve this? If you propose a fix, please make it concise.

this is intended

…necessary reset sequences

…rsistence in TruncateTwoLines

… overflowing

…dant TUI list width capping

SuperCoolPencil added 8 commits April 25, 2026 09:45

refactor: export TruncateMiddle and implement TruncateTwoLines for TU…

d64b596

…I layout consistency

refactor: truncate detail text to two lines instead of wrapping in co…

52a8823

…nfirmation modal

refactor: replace text wrapping with two-line truncation and improve …

a420b68

…string truncation logic across TUI views

refactor: update TruncateTwoLines to use character-based wrapping and…

d71d040

… add unit tests

chore: go mod tidy

5c99258

feat: make text truncation utilities ANSI-aware and add support for v…

b0a9825

…isual width calculations

feat: implement dynamic modal sizing and improve TUI layout robustnes…

643a0c1

…s by adding title truncation and overflow safety checksd

refactor: transition chunk map visibility logic from hardcoded height…

8a9d139

… thresholds to dynamic content measurement

greptile-apps Bot reviewed Apr 25, 2026

View reviewed changes

SuperCoolPencil added 4 commits April 25, 2026 10:27

fix: preserve ANSI color states when truncating strings by injecting …

45d2cea

…necessary reset sequences

refactor: update string width calculation and implement ANSI state pe…

ea7882a

…rsistence in TruncateTwoLines

fix: ensure text wrapping truncates correctly at two lines instead of…

3f58d54

… overflowing

fix: improve ANSI sequence parsing in text utilities and remove redun…

9a34dd4

…dant TUI list width capping

SuperCoolPencil changed the title ~~long urls break tui and extension~~ long urls break tui Apr 25, 2026

SuperCoolPencil merged commit eec85c0 into main Apr 25, 2026
16 checks passed

SuperCoolPencil deleted the long-urls-break-tui-and-extension branch April 25, 2026 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

long urls break tui#415

long urls break tui#415
SuperCoolPencil merged 12 commits intomainfrom
long-urls-break-tui-and-extension

SuperCoolPencil commented Apr 25, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

github-actions Bot commented Apr 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

greptile-apps Bot Apr 25, 2026

Uh oh!

SuperCoolPencil Apr 25, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

SuperCoolPencil commented Apr 25, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Comments Outside Diff (2)

Uh oh!

github-actions Bot commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Binary Size Analysis

Uh oh!

Uh oh!

greptile-apps Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

SuperCoolPencil Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

SuperCoolPencil commented Apr 25, 2026 •

edited by greptile-apps Bot

Loading

github-actions Bot commented Apr 25, 2026 •

edited

Loading