Skip to content

fix: maintain input width when showing suggestion#143

Merged
yimsk merged 1 commit intomainfrom
fix/suggestion-width
Jan 11, 2026
Merged

fix: maintain input width when showing suggestion#143
yimsk merged 1 commit intomainfrom
fix/suggestion-width

Conversation

@yimsk
Copy link
Copy Markdown
Contributor

@yimsk yimsk commented Jan 11, 2026

Restore padding after suffix to keep original width.

Restore padding after suffix to keep original width.
@claude
Copy link
Copy Markdown

claude Bot commented Jan 11, 2026

Code Review - PR #143: Fix maintain input width when showing suggestion

Summary

This PR fixes a visual bug where the input field width was being reduced when fish-style inline suggestions were shown. The fix restores padding after the suggestion text to maintain the original input box width.

✅ Positive Aspects

  1. Clear Problem Statement: The fix addresses a specific UI issue with a focused solution
  2. Logic is Sound: The calculation correctly determines how much padding was removed and restores it after the suggestion
  3. Minimal Change: The fix is surgical - only modifying the specific function that renders suggestions
  4. Well-Commented: The code includes inline comments explaining the padding restoration logic

🔍 Code Quality & Best Practices

Overall: Good - The code follows Go conventions and is readable.

Minor Observations:

  1. Variable Naming (internal/view/command_input.go:258-261): The variable names are descriptive and clear
  2. Edge Case Handling: The code properly handles the case where paddingNeeded < 0 by clamping to 0

🐛 Potential Issues

None identified - The logic appears correct:

  • Calculates removed padding: len(baseView) - len(trimmedView)
  • Subtracts suggestion length: removedPadding - len(suffix)
  • Clamps to 0 if negative (when suffix is longer than removed padding)
  • Restores padding with strings.Repeat(" ", paddingNeeded)

⚡ Performance Considerations

No concerns - The changes are minimal and performant:

  • strings.Repeat(" ", n) is efficient for small values of n (typically < 100)
  • Two simple integer operations (subtraction, max with 0)
  • This function is called during rendering, but the operations are O(1) with small constants

🔒 Security Concerns

None - No security implications for this UI rendering change.

🧪 Test Coverage

Needs Improvement - While the codebase has good test coverage overall, the specific renderInputWithSuggestion function doesn't have direct unit tests.

Recommendations:

  1. Add Test Case for Width Preservation: Consider adding a test that verifies the rendered output maintains the expected width when suggestions are shown. Example:

```go
func TestCommandInput_SuggestionMaintainsWidth(t *testing.T) {
ctx := context.Background()
reg := registry.New()
reg.RegisterCustom("ec2", "instances", registry.Entry{})

ci := NewCommandInput(ctx, reg)
ci.Activate()

// Set short input that triggers suggestion
ci.textInput.SetValue("ec2")
ci.updateSuggestions()

// Get rendered view without suggestion (empty input)
ci.textInput.SetValue("")
viewWithoutSuggestion := ci.View()
widthWithout := visualWidth(viewWithoutSuggestion)

// Get rendered view with suggestion
ci.textInput.SetValue("ec2")
viewWithSuggestion := ci.View()
widthWith := visualWidth(viewWithSuggestion)

// Widths should be similar (within threshold padding)
// This ensures the input box doesn't shrink when showing suggestions
if abs(widthWith - widthWithout) > 5 {
    t.Errorf("Width changed significantly with suggestion: without=%d, with=%d", 
             widthWithout, widthWith)
}

}
```

  1. Visual Regression Test: Since this is a visual bug fix, consider adding a test that captures the rendered output for manual verification

📝 Suggested Improvements

Optional Enhancements (not blockers):

  1. Extract Magic Constant: The clamping logic could be slightly more explicit:
    ```go
    paddingNeeded := max(0, removedPadding - len(suffix))
    ```
    However, the current if-statement is perfectly clear, so this is purely stylistic.

  2. Add Debug Comment: If this bug resurfaces, having a comment about WHY we restore padding would be helpful:
    ```go
    // Restore padding after suffix to maintain original width
    // Without this, the input box would shrink when showing suggestions
    removedPadding := len(baseView) - len(trimmedView)
    ```

✅ Recommendation

APPROVE - This is a solid fix for a specific UI issue. The code is correct, performant, and safe.

The only suggestion is to add test coverage for this functionality to prevent regression, but this can be done in a follow-up PR if desired.


Testing Notes:

  • The fix should be manually tested by typing partial commands (e.g., "ec2", "cloud") and verifying the input box doesn't visibly shrink when suggestions appear
  • Test with different input lengths to verify width preservation across all threshold levels (15, 30, 60, 90)

🤖 Generated with Claude Code

@yimsk yimsk merged commit 0f2646a into main Jan 11, 2026
6 checks passed
@yimsk yimsk deleted the fix/suggestion-width branch January 11, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant