feat: make image preview support inline image protocol #1069
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Gates Failed
(1 file with Bumpy Road Ahead)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| image_preview.go | 1 critical rule | 9.10 → 8.85 | Suppress |
Quality Gate Profile: The Bare Minimum
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
| // ImagePreview generates a preview of an image file | ||
| func (p *ImagePreviewer) ImagePreview(path string, maxWidth int, maxHeight int, | ||
| defaultBGColor string, sideAreaWidth int) (string, error) { | ||
| defaultBGColor string, sideAreaWidth int) (string, error, ImageRenderer) { |
There was a problem hiding this comment.
❌ New issue: Bumpy Road Ahead
ImagePreview has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function
| func (p *ImagePreviewer) ClearAllImages() string { | ||
| var result strings.Builder | ||
|
|
||
| // Clear Kitty protocol images if supported | ||
| if p.IsKittyCapable() { | ||
| if clearCmd := p.ClearKittyImages(); clearCmd != "" { | ||
| result.WriteString(clearCmd) | ||
| } | ||
| } | ||
|
|
||
| // Clear inline protocol images if supported | ||
| if p.IsInlineCapable() { | ||
| if clearCmd := p.ClearInlineImage(); clearCmd != "" { | ||
| result.WriteString(clearCmd) | ||
| } | ||
| } | ||
|
|
||
| return result.String() | ||
| } |
There was a problem hiding this comment.
❌ New issue: Bumpy Road Ahead
ClearAllImages has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function
| return "" // No need to clear if terminal doesn't support inline protocol | ||
| } | ||
|
|
||
| return strings.Repeat(" ", 9999) |
There was a problem hiding this comment.
Let's precompute this. Store it in a predefined variable InlineImageCleanupString or something. Will save a bit of processing, and will make code more readable.
|
The current issue is that the inline image protocol doesn’t seem to support removing images, so we need to handle this through bubbletea instead. However, during this process, I found that bubbletea recognizes an image as a single line of text, which prevents the subsequent content from being re-rendered. So I used this workaround to handle the issue, but it’s admittedly a bit of a hack. I’m still considering whether there’s a cleaner solution, or if it might be better to simply force bubbletea to re-render. |
|
I see. I will try to take a stab at the issue if possible. |
No description provided.