Bugfix/assignee introducing new line #59

Merged
noe.trevino merged 7 commits from bugfix/assignee-introducing-new-line into main 2026-02-08 02:55:11 -06:00
Owner
No description provided.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:07 -06:00
@ -0,0 +79,4 @@
Foreground(lipgloss.Color(subtleColor)).
Render(separator),
ellipsis: ellipsisStyle.Background(lipgloss.Color(bg)).Render(ellipsisText),
}
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:07 -06:00

Hardcoded dependency on theme.Subtle violates dependency injection. The newSegmentTruncator function already accepts a bg parameter but directly imports and uses theme.Subtle instead of receiving it as a parameter. This couples the truncator to the theme package unnecessarily and makes testing harder.

Hardcoded dependency on `theme.Subtle` violates dependency injection. The `newSegmentTruncator` function already accepts a `bg` parameter but directly imports and uses `theme.Subtle` instead of receiving it as a parameter. This couples the truncator to the theme package unnecessarily and makes testing harder.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:10 -06:00
@ -0,0 +15,4 @@
// NewStyledSegment creates a Segment with the given text, style, and background color.
// The background is applied to the style upfront rather than deferred to each render call.
func NewStyledSegment(text string, style lipgloss.Style, bg string) Segment {
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:10 -06:00

Closure captures bg parameter unnecessarily. The Render function in NewStyledSegment creates a closure that captures the bg parameter and applies it on every render. This could lead to bugs if the background changes after segment creation. Consider applying the background to the style upfront instead of deferring it.

Closure captures `bg` parameter unnecessarily. The `Render` function in `NewStyledSegment` creates a closure that captures the `bg` parameter and applies it on every render. This could lead to bugs if the background changes after segment creation. Consider applying the background to the style upfront instead of deferring it.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:13 -06:00
@ -0,0 +102,4 @@
// tryFitWithTruncation walks backwards through segments to find how many complete
// segments fit, and whether a partial last segment can be shown.
func (t *segmentTruncator) tryFitWithTruncation() string {
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:13 -06:00

Complex nested logic in tryFitWithTruncation. This method mixes two different truncation strategies (partial segment truncation and complete segment truncation) in a single loop with multiple conditions. This violates the Single Responsibility Principle and makes the code hard to follow. Consider extracting into separate methods: tryFitPartialSegment(i) and tryFitCompleteSegments(i).

Complex nested logic in `tryFitWithTruncation`. This method mixes two different truncation strategies (partial segment truncation and complete segment truncation) in a single loop with multiple conditions. This violates the Single Responsibility Principle and makes the code hard to follow. Consider extracting into separate methods: `tryFitPartialSegment(i)` and `tryFitCompleteSegments(i)`.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:15 -06:00
@ -0,0 +1,157 @@
package text
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:15 -06:00

Missing validation. newSegmentTruncator doesn't validate inputs. If maxWidth is negative or zero, or if segments is empty, the behavior is undefined and could lead to out-of-bounds access in prefixWidth. Add guard clauses or document preconditions.

Missing validation. `newSegmentTruncator` doesn't validate inputs. If `maxWidth` is negative or zero, or if `segments` is empty, the behavior is undefined and could lead to out-of-bounds access in `prefixWidth`. Add guard clauses or document preconditions.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:18 -06:00
@ -171,3 +99,4 @@
}
// Blocked badge
if task.IsBlocked {
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:18 -06:00

DRY violation: repeated pattern in all render functions. renderTaskSummaryTitle, renderTaskCardLabels, and renderTaskSummaryMetadata all follow the exact same pattern of creating segments and calling TruncateSegments with a leading space prepended. This repetition could be extracted into a helper function like renderWithLeadingSpace(segments, separator, maxWidth, bg).

DRY violation: repeated pattern in all render functions. `renderTaskSummaryTitle`, `renderTaskCardLabels`, and `renderTaskSummaryMetadata` all follow the exact same pattern of creating segments and calling `TruncateSegments` with a leading space prepended. This repetition could be extracted into a helper function like `renderWithLeadingSpace(segments, separator, maxWidth, bg)`.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:21 -06:00
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:21 -06:00

Long method with sequential append operations. This function has 35 lines with repetitive segment building logic. Each conditional block follows the pattern: check condition, create style, append segment. This is a textbook case for the Builder pattern or Extract Method refactoring. Consider extracting methods: appendTypeSegment(segments, task), appendPrioritySegment(segments, task), etc.

Long method with sequential append operations. This function has 35 lines with repetitive segment building logic. Each conditional block follows the pattern: check condition, create style, append segment. This is a textbook case for the Builder pattern or Extract Method refactoring. Consider extracting methods: `appendTypeSegment(segments, task)`, `appendPrioritySegment(segments, task)`, etc.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:23 -06:00
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:24 -06:00

Duplicate style creation logic. Lines 89-92 and 201-204 (in the diff) both create similar "subtle italic" styles for missing data. This is code duplication that violates DRY. Extract to a function or constant style: MissingDataStyle or createSubtleItalicStyle(bg).

Duplicate style creation logic. Lines 89-92 and 201-204 (in the diff) both create similar "subtle italic" styles for missing data. This is code duplication that violates DRY. Extract to a function or constant style: `MissingDataStyle` or `createSubtleItalicStyle(bg)`.
github-actions[bot] commented 2026-02-08 02:36:24 -06:00 (Migrated from github.com)

PR Summary (auto-generated)

• Fixed text wrapping bug in task cards that was causing assignee names and labels to introduce unwanted newlines when rendered
• Refactored task card rendering (internal/tui/components/task.go) to use a new segment-based truncation system with character-level precision instead of word-level truncation
• Created new text/segment_truncator.go package implementing TruncateSegments() function that measures plain text width using runewidth before applying lipgloss styling, eliminating measurement discrepancies
• Added taskCardSafetyBuffer constant (value: 3) to account for leading space and margins, preventing lipgloss word-wrapping edge cases
• Removed unused EllipsisStyle from styles.go as ellipsis rendering is now handled by the truncator
• Added comprehensive test coverage: task_test.go with 13 tests and segment_truncator_test.go with 24 tests validating truncation behavior at various widths, character-level truncation, and Unicode handling
• Updated snapshot tests to reflect corrected rendering (titles now have proper spacing after the leading border character)

**PR Summary** _(auto-generated)_ • Fixed text wrapping bug in task cards that was causing assignee names and labels to introduce unwanted newlines when rendered • Refactored task card rendering (internal/tui/components/task.go) to use a new segment-based truncation system with character-level precision instead of word-level truncation • Created new text/segment_truncator.go package implementing TruncateSegments() function that measures plain text width using runewidth before applying lipgloss styling, eliminating measurement discrepancies • Added taskCardSafetyBuffer constant (value: 3) to account for leading space and margins, preventing lipgloss word-wrapping edge cases • Removed unused EllipsisStyle from styles.go as ellipsis rendering is now handled by the truncator • Added comprehensive test coverage: task_test.go with 13 tests and segment_truncator_test.go with 24 tests validating truncation behavior at various widths, character-level truncation, and Unicode handling • Updated snapshot tests to reflect corrected rendering (titles now have proper spacing after the leading border character)
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:26 -06:00
@ -0,0 +111,4 @@
if truncated != "" {
// i >= 1 is guaranteed by the loop bounds, so segments[:i] is
// always non-empty and renderJoined always returns content.
prefix := t.renderJoined(t.segments[:i])
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:26 -06:00

Guard clause inversion opportunity. Lines 115-118 check if prefix != "" to decide how to format the result. This is a guard pattern that should be flattened: check if prefix == "" and return early, then handle the normal case without nesting.

Guard clause inversion opportunity. Lines 115-118 check `if prefix != ""` to decide how to format the result. This is a guard pattern that should be flattened: check `if prefix == ""` and return early, then handle the normal case without nesting.
claude[bot] (Migrated from github.com) reviewed 2026-02-08 02:36:29 -06:00
@ -43,46 +43,13 @@ func RenderTask(task *models.TaskSummary, selected bool, width int) string {
}
// renderTaskSummaryTitle renders the task title, truncating with "..." if needed.
claude[bot] (Migrated from github.com) commented 2026-02-08 02:36:29 -06:00

Magic number calculation. taskCardSafetyBuffer is subtracted from width without explanation of why this is correct. The comment in constants.go says "1 + 2" but doesn't explain where those numbers come from or if they match the leading space in renderTaskSummaryTitle (line 48). This could lead to inconsistencies if the rendering logic changes.

Magic number calculation. `taskCardSafetyBuffer` is subtracted from width without explanation of why this is correct. The comment in constants.go says "1 + 2" but doesn't explain where those numbers come from or if they match the leading space in renderTaskSummaryTitle (line 48). This could lead to inconsistencies if the rendering logic changes.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
noe.trevino/paso!59
No description provided.