long urls break tui - #415
#415Merged
SuperCoolPencil merged 12 commits intoApr 25, 2026
mainSurgeDM/Surge:mainfrom
long-urls-break-tui-and-extensionSurgeDM/Surge:long-urls-break-tui-and-extensionCopy head branch name to clipboard
Merged
long urls break tui#415SuperCoolPencil merged 12 commits intomainSurgeDM/Surge:mainfrom long-urls-break-tui-and-extensionSurgeDM/Surge:long-urls-break-tui-and-extensionCopy head branch name to clipboard
SuperCoolPencil merged 12 commits into
mainSurgeDM/Surge:mainfrom
long-urls-break-tui-and-extensionSurgeDM/Surge:long-urls-break-tui-and-extensionCopy head branch name to clipboard
Conversation
…I layout consistency
…string truncation logic across TUI views
…isual width calculations
…s by adding title truncation and overflow safety checksd
… thresholds to dynamic content measurement
Binary Size Analysis
|
| wrapped := utils.WrapText(entry, width) | ||
| rendered := wrapStyle.Render(wrapped) | ||
| wrappedEntries = append(wrappedEntries, strings.Split(rendered, "\n")...) | ||
| wrapped := utils.TruncateTwoLines(entry, width) |
Contributor
There was a problem hiding this 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.
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.
Member
Author
There was a problem hiding this comment.
this is intended
…necessary reset sequences
…rsistence in TruncateTwoLines
…dant TUI list width capping
StressTestor
pushed a commit
to StressTestor/Surge
that referenced
this pull request
Jun 1, 2026
* 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 * fix: preserve ANSI color states when truncating strings by injecting necessary reset sequences * refactor: update string width calculation and implement ANSI state persistence in TruncateTwoLines * fix: ensure text wrapping truncates correctly at two lines instead of overflowing * fix: improve ANSI sequence parsing in text utilities and remove redundant TUI list width capping
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Greptile Summary
This PR fixes long URLs breaking the TUI and extension by introducing ANSI-aware two-line truncation (
TruncateTwoLines,TruncateMiddleexported) across detail panes, modals, the download list, settings, and the category manager, replacing unboundedWrapTextcalls. 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
Comments Outside Diff (2)
internal/utils/text_test.go, line 1313-1321 (link)Both assertions in
TestAnsiAwarenessare incorrect and will fail.truncateToWidthexplicitly does not append a reset (comment: "allow ANSI states (colors) to bleed into the next line"), so the actual output fortruncateToWidth(text, 3)is"\x1b[31mhel", not"\x1b[31mhel\x1b[0m".TruncateMiddlebuilds the right portion from the tail of the original string. Fortext = "\x1b[31mhello\x1b[0m"withlimit=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
internal/utils/text_test.go, line 1290-1305 (link)TruncateTwoLinestests missing key edge casesThe 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)2*width(verifying theTruncateMiddletruncation actually fires)getCharInfosinternally)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
Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "fix: improve ANSI sequence parsing in te..." | Re-trigger Greptile