Skip to content

feat: move from rasterm to go-termimg for image preview#1385

Open
krios2146 wants to merge 1 commit into
yorukot:mainfrom
krios2146:dev
Open

feat: move from rasterm to go-termimg for image preview#1385
krios2146 wants to merge 1 commit into
yorukot:mainfrom
krios2146:dev

Conversation

@krios2146
Copy link
Copy Markdown

@krios2146 krios2146 commented Mar 20, 2026

Potentially closes #1217

As #1270 has been inactive for some time, I've made another migration attempt from rasterm to go-termimg

This blacktop/go-termimg#27 PR appears to add the unicode placeholder type of kitty protocol, but I haven't managed to integrate it properly (or it's a go-termimg issue). By properly I mean kitty image protocol working inside tmux

I've tested my changes in ghostty and kitty on Linux, all working fine, no scaling issues/blurred previews when changing terminal size. Preview borders enabled in screenshots

Kitty
kitty

Ghostty
ghostty

Summary by CodeRabbit

  • Chores
    • Updated Go toolchain from 1.25.5 to 1.26
    • Updated dependencies for improved terminal compatibility and image preview rendering support

- remove `BourgeoisBear/rasterm` dependency
- add `blacktop/go-termimg` dependency
- remove kitty constants
- replace rasterm with go-termimg in `file_preview/kitty.go`

fixup xdd
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

The pull request replaces the rasterm library with go-termimg for terminal image rendering, updates the Go toolchain from 1.25.5 to 1.26, and removes now-unused Kitty protocol constants. The isKittyCapable() function now uses go-termimg's capability detection, and renderWithKittyUsingTermCap() is rewritten to use go-termimg for rendering.

Changes

Cohort / File(s) Summary
Dependency Management
go.mod
Updated Go version to 1.26, added github.com/blacktop/go-termimg v0.1.26, updated direct dependencies (charmbracelet/x/ansi, golang.org/x/image, golang.org/x/mod), and numerous indirect image processing and terminal utility dependencies; removed rasterm dependency.
Image Preview Code
src/pkg/file_preview/constants.go, src/pkg/file_preview/image_preview.go, src/pkg/file_preview/kitty.go
Removed unused Kitty hash constants; minor formatting adjustment in function call; rewrote Kitty capability detection and rendering to use go-termimg API instead of rasterm, including clearing command generation and image rendering pipeline.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • #1094: Adds calls to clear Kitty images after rendering, building on the new go-termimg rendering approach introduced here.
  • #904: Modifies Kitty terminal capability detection and handling, directly related to the isKittyCapable() function rewrites.
  • #1267: Adds TODO comments in src/pkg/file_preview/kitty.go, the same file with major refactoring in this PR.

Suggested reviewers

  • yorukot
  • lazysegtree

Poem

🐰 From rasterm's old ways to termimg's bright glow,
Images render so smoothly and clean,
New protocols bloom in the library's code,
A rabbit hops forth to a rendering dream! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: migrating from rasterm to go-termimg for image preview functionality.
Linked Issues check ✅ Passed Changes successfully address #1217 objectives: rasterm replaced with go-termimg, Kitty protocol detection updated, rendering refactored with termimg API, and iTerm2 capability detection preserved.
Out of Scope Changes check ✅ Passed Changes are narrowly scoped to dependencies and image preview implementation; all modifications directly support the rasterm-to-go-termimg migration without extraneous alterations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.

Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/pkg/file_preview/kitty.go (1)

17-45: Consider caching the Kitty capability detection result.

isKittyCapable() is called frequently (every clear and render operation per the callers in model_render.go and preview/render.go). The terminal's graphics capability doesn't change during runtime, so querying termimg.KittySupported() and termimg.QueryTerminalFeatures() repeatedly is wasteful.

♻️ Suggested approach: Cache detection at package init or first call
+var (
+	kittyCapableOnce   sync.Once
+	kittyCapableResult bool
+)
+
 // isKittyCapable checks if the terminal supports Kitty graphics protocol
 func isKittyCapable() bool {
+	kittyCapableOnce.Do(func() {
+		kittyCapableResult = detectKittyCapability()
+	})
+	return kittyCapableResult
+}
+
+func detectKittyCapability() bool {
 	// Current go-termimg (0.1.26) unicode implementation of kitty protocol breaks UI inside tmux
 	if termimg.KittySupported() && !termimg.QueryTerminalFeatures().IsTmux {
 		return true
 	}
 	// ... rest of detection logic
+	return false
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pkg/file_preview/kitty.go` around lines 17 - 45, isKittyCapable currently
performs repeated runtime checks (termimg.KittySupported and
termimg.QueryTerminalFeatures) on every render; cache the boolean result in the
package by adding a package-level variable (e.g., kittyCapable bool) and
initialize it once (either in init() or lazily with sync.Once) so subsequent
calls to isKittyCapable simply return the cached value; ensure you still compute
the same logic (including TERM and TERM_PROGRAM checks and knownTerminals list)
when setting the cached value and reference the symbols isKittyCapable,
termimg.KittySupported, and termimg.QueryTerminalFeatures so the change is
localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pkg/file_preview/kitty.go`:
- Around line 130-136: The code constructs an image renderer with
termimg.New(img) and sets Width(dstCols) and Height(dstRows) but then calls
Scale(termimg.ScaleNone), which ignores those target dimensions; remove the
Scale(termimg.ScaleNone) call or replace it with a bounded mode such as
Scale(termimg.ScaleFit) so that the renderer respects the calculated
dstCols/dstRows while keeping Protocol(termimg.Kitty) and PNG(true) and the
existing Render() invocation.
- Around line 129-142: The cursor positioning escape is being appended after the
Kitty image escape sequence (buf.WriteString(rendered) then fmt.Fprintf(&buf,
"\x1b[%d;%dH", row, col)); move the cursor positioning before writing the
rendered image so the terminal evaluates the cursor location first. In practice,
in the function that builds the output (the block using
termimg.New(...).Render() and buf), write the fmt.Fprintf(&buf, "\x1b[%d;%dH",
row, col) call immediately before buf.WriteString(rendered) (or otherwise ensure
the cursor escape is placed earlier in the buffer) so the Kitty protocol uses
the correct cursor position when the final image chunk is received.

---

Nitpick comments:
In `@src/pkg/file_preview/kitty.go`:
- Around line 17-45: isKittyCapable currently performs repeated runtime checks
(termimg.KittySupported and termimg.QueryTerminalFeatures) on every render;
cache the boolean result in the package by adding a package-level variable
(e.g., kittyCapable bool) and initialize it once (either in init() or lazily
with sync.Once) so subsequent calls to isKittyCapable simply return the cached
value; ensure you still compute the same logic (including TERM and TERM_PROGRAM
checks and knownTerminals list) when setting the cached value and reference the
symbols isKittyCapable, termimg.KittySupported, and
termimg.QueryTerminalFeatures so the change is localized.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 72d2c606-f074-45c9-8d35-308b43caff67

📥 Commits

Reviewing files that changed from the base of the PR and between cba7254 and 26062cb.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • go.mod
  • src/pkg/file_preview/constants.go
  • src/pkg/file_preview/image_preview.go
  • src/pkg/file_preview/kitty.go
💤 Files with no reviewable changes (1)
  • src/pkg/file_preview/constants.go

Comment on lines +129 to +142

rendered, err := termimg.New(img).
Width(dstCols).
Height(dstRows).
Protocol(termimg.Kitty).
Scale(termimg.ScaleNone).
PNG(true).
Render()
if err != nil {
return "", err
}
buf.WriteString(rendered)

fmt.Fprintf(&buf, "\x1b[%d;%dH", row, col)
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find and read the kitty.go file
find . -name "kitty.go" -type f

Repository: yorukot/superfile

Length of output: 92


🏁 Script executed:

# Read the file to understand the full context of the renderWithKittyUsingTermCap function
cat -n src/pkg/file_preview/kitty.go | head -150

Repository: yorukot/superfile

Length of output: 5318


🏁 Script executed:

# Check if there are any tests or examples showing how cursor positioning should work
rg -A 5 -B 5 "fmt.Fprintf.*%d;%d" src/pkg/file_preview/

Repository: yorukot/superfile

Length of output: 603


🌐 Web query:

Kitty graphics protocol cursor positioning order before or after image render

💡 Result:

In the Kitty graphics protocol, the image is rendered at the current cursor position before the cursor is advanced. The official documentation explicitly states: "The image is rendered at the current cursor position, from the upper left corner of the current cell." Additionally, "Note that the cursor position used to display the image must be the position when the final chunk is received." After rendering the image (placement), the cursor is then moved forward by the number of columns and rows spanned by the image rectangle (default C=0 behavior), unless C=1 is specified to prevent cursor movement.

Citations:


🏁 Script executed:

# Search for tests related to kitty rendering
find . -name "*test*" -o -name "*_test.go" | xargs grep -l "kitty\|Kitty" 2>/dev/null || true

Repository: yorukot/superfile

Length of output: 43


🏁 Script executed:

# Search for any bug reports or issues mentioning image positioning
rg "kitty|Kitty" --type go -A 3 -B 3 | grep -i "position\|render\|wrong\|incorrect" || true

Repository: yorukot/superfile

Length of output: 4505


🏁 Script executed:

# Check how renderWithKittyUsingTermCap is called and what happens after
rg "renderWithKittyUsingTermCap" -A 10 -B 5

Repository: yorukot/superfile

Length of output: 2228


🌐 Web query:

Kitty graphics protocol cursor position timing absolute positioning escape sequence order

💡 Result:

In the Kitty terminal graphics protocol, images are positioned at the current text cursor position when the final chunk of the image data is received by the terminal. For multi-chunk transmissions (using the 'm' key), the client sends sequential escape sequences: _G[control data];[base64 payload]\ for the first chunk (with full keys like a=T,f=100,s=width,v=height), followed by _Gm=[0|1];[payload]\ for subsequent chunks (m=1 for more data, m=0 for final). The terminal buffers data and displays the image only after receiving and validating the final chunk (m=0), using the cursor position at that exact moment. No other graphics escape codes can be sent between chunks for the same image. Positioning is relative to the cursor's cell: from the upper-left corner, with optional pixel offsets X= and Y= within the cell. The image spans c= columns and r= rows (default: fitted to image size). For absolute-like positioning, move the cursor explicitly first using standard CSI escape sequences (e.g., [10;20H to row 10, col 20), then transmit the image. After display (for actions a=T, a=p), the cursor advances by the placement size (right by c cols, down by r rows), unless overridden by cursor movement policy C=: C=0 (default: advance), C=1 (no movement, cursor stays at original position). Escape sequence order: Position cursor -> send full image transmission sequence (potentially chunked, final chunk determines position/timing) -> optionally adjust with C=1 to prevent advance. Sources: Official Kitty graphics protocol documentation confirms cursor at final chunk determines position (1,2,4,6); chunking requires completing all chunks before other graphics codes, display post-final chunk (1,2); C= policy detailed as 0=advance, 1=no move (1,4); positioning via current cursor cell (2,6).

Citations:


Cursor positioning must come before the image escape sequence.

The Kitty graphics protocol evaluates the cursor position when the final chunk of image data is received. Currently, the cursor is positioned after the image escape sequence is added to the buffer. When the terminal processes this buffer sequentially, it renders the image at the current cursor position, then repositions the cursor—the opposite of the correct order.

According to the official Kitty protocol documentation, the correct sequence is: position cursor first, then send the image data.

🐛 Proposed fix: Position cursor before rendering
+	fmt.Fprintf(&buf, "\x1b[%d;%dH", row, col)
+
 	rendered, err := termimg.New(img).
 		Width(dstCols).
 		Height(dstRows).
 		Protocol(termimg.Kitty).
 		Scale(termimg.ScaleNone).
 		PNG(true).
 		Render()
 	if err != nil {
 		return "", err
 	}
 	buf.WriteString(rendered)
-
-	fmt.Fprintf(&buf, "\x1b[%d;%dH", row, col)

 	return buf.String(), nil
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pkg/file_preview/kitty.go` around lines 129 - 142, The cursor positioning
escape is being appended after the Kitty image escape sequence
(buf.WriteString(rendered) then fmt.Fprintf(&buf, "\x1b[%d;%dH", row, col));
move the cursor positioning before writing the rendered image so the terminal
evaluates the cursor location first. In practice, in the function that builds
the output (the block using termimg.New(...).Render() and buf), write the
fmt.Fprintf(&buf, "\x1b[%d;%dH", row, col) call immediately before
buf.WriteString(rendered) (or otherwise ensure the cursor escape is placed
earlier in the buffer) so the Kitty protocol uses the correct cursor position
when the final image chunk is received.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what happens if cursor position is set before image sequence, no idea why but it is what it is

cursor_before_image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Comment image shows a completely broken UI when cursor positioning is placed before the image sequence — the entire main content area (file list on left, image preview on right) is replaced with solid black, leaving only the status bar and bottom panels (Processes/Metadata/Clipboard) visible.

The PR description images (Kitty and Ghostty) show the correctly working display with the current ordering.

@krios2146, thanks for testing this — you're right, and I was wrong to flag this. My original comment was based on the raw Kitty protocol spec, but go-termimg's Render() is a higher-level abstraction that handles its own internal positioning. Unlike writing raw Kitty escape sequences, the library doesn't rely on the caller pre-positioning the cursor — the \x1b[%d;%dH at the end is almost certainly needed to reset the cursor for the surrounding TUI (bubbletea/lipgloss) layout to render correctly after the image. The current order is correct for this library's integration model.

I'll withdraw my concern.


✏️ Learnings added
Learnt from: krios2146
Repo: yorukot/superfile PR: 1385
File: src/pkg/file_preview/kitty.go:129-142
Timestamp: 2026-03-20T11:45:14.220Z
Learning: In yorukot/superfile, `go-termimg`'s `Render()` handles its own cursor/positioning internally. When integrating with a bubbletea TUI, the cursor positioning escape (`\x1b[%d;%dH`) must be written AFTER `buf.WriteString(rendered)` (the go-termimg output), not before. Placing it before causes the entire content area to go black. The escape at the end resets the cursor for the surrounding TUI layout.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 841
File: src/pkg/file_preview/kitty.go:78-79
Timestamp: 2025-09-14T05:34:28.184Z
Learning: lazysegtree correctly identified that `\x1b[0m` is an ANSI reset sequence that resets text formatting to default, not a delay command as incorrectly commented in the Kitty graphics protocol clearing code in src/pkg/file_preview/kitty.go.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, the preview panel refactoring created duplicate ImagePreviewer instances (one in defaultModelConfig and another in preview.New()), which caused image preview crashes. The refactoring also renamed SetContextWithRenderText to SetContentWithRenderText, and moved global batCmd to per-instance batCmd initialization, all contributing to crashes during image preview and test failures.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, lazysegtree reported image preview crashes and rename test failures after refactoring FilePreviewPanel to a separate preview package. The crashes were likely caused by: 1) imagePreviewer initialization differences between old global variable and new per-instance creation, 2) method name changes from SetContextWithRenderText to SetContentWithRenderText, and 3) batCmd initialization moving from global to per-instance without preserving the original configuration.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, the image preview crashes and test failures were caused by duplicate ImagePreviewer initialization - one in defaultModelConfig() and another in preview.New(). This created two separate instances that conflicted in terminal state management, causing crashes during image preview operations and test execution.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, the tests were actually passing despite lazysegtree reporting crashes. The real issue was that production image preview crashes occurred during actual application usage due to duplicate ImagePreviewer instances (one in defaultModelConfig and one in preview.New()), while the test environment didn't stress the image preview system enough to expose the conflicts.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR `#767` for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.

Comment on lines +130 to +136
rendered, err := termimg.New(img).
Width(dstCols).
Height(dstRows).
Protocol(termimg.Kitty).
Scale(termimg.ScaleNone).
PNG(true).
Render()
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

go-termimg ScaleNone Width Height behavior

💡 Result:

In the Go library github.com/blacktop/go-termimg (a modern terminal image renderer supporting Kitty, Sixel, iTerm2, and halfblocks), ScaleNone is one of the scaling modes available alongside fit, fill, and stretch. It renders the image at its original size without any scaling. When using Width(w) and Height(h) methods (which set the target terminal cell dimensions in columns and rows), ScaleNone ignores the specified Width and Height values and displays the image using the minimum number of cells required to show the original pixel dimensions (accounting for terminal font metrics, typically assuming ~2 pixels per cell width and ~3-4 per height for character aspect ratio). This contrasts with: - ScaleFit: Scales down to fit within Width x Height while preserving aspect ratio (may use fewer cells). - ScaleFill: Scales and crops to exactly fill Width x Height. - ScaleStretch: Distorts to exactly match Width x Height. CLI example with imgcat tool: imgcat --scale none image.png displays at original size, ignoring any -W/-H flags. Width and Height are primarily for bounded scaling modes but overridden by ScaleNone. Sources confirm scale modes explicitly list "none" and API usage like img.Width(50).Height(25).Scale(termimg.ScaleNone).

Citations:


Remove or replace Scale(termimg.ScaleNone) to respect calculated dimensions.

The code calculates dstCols and dstRows for aspect-ratio-correct sizing, then passes them via Width() and Height(). However, Scale(termimg.ScaleNone) ignores these parameters and renders the image at its original size instead. Either:

  • Use a bounded scaling mode like Scale(termimg.ScaleFit) to respect the calculated dimensions, or
  • Remove the Width() and Height() calls if original-size rendering is intended
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pkg/file_preview/kitty.go` around lines 130 - 136, The code constructs an
image renderer with termimg.New(img) and sets Width(dstCols) and Height(dstRows)
but then calls Scale(termimg.ScaleNone), which ignores those target dimensions;
remove the Scale(termimg.ScaleNone) call or replace it with a bounded mode such
as Scale(termimg.ScaleFit) so that the renderer respects the calculated
dstCols/dstRows while keeping Protocol(termimg.Kitty) and PNG(true) and the
existing Render() invocation.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting Scale(termimg.ScaleFit) is distorting the image

Not a big difference on screenshots, but you will definitely notice it in the terminal, images become much more rough looking with ScaleFit

I picked screenshots of a text as this roughness is more visible there

With Scale(termimg.ScaleFit)
scale_fit

With Scale(termimg.ScaleNone)
scale_none

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment image 1 (scale_fit): Shows the superfile UI previewing a weather forecast screenshot. The text in the right-panel preview is noticeably rougher/blurrier — particularly evident in the weather data rows and temperature values, where the characters appear slightly degraded compared to native terminal text rendering.

Comment image 2 (scale_none): Shows the identical preview of the same weather forecast screenshot with ScaleNone. The text in the preview panel is visibly crisper and more legible, with characters rendering much more cleanly.


Thanks for the detailed comparison, @krios2146 — the visual difference is clear. My original concern was incorrect in this context.

With the Kitty graphics protocol, Scale(termimg.ScaleNone) does not simply "ignore" Width/Height in a harmful way — it avoids software resampling by go-termimg and instead lets the Kitty terminal itself handle scaling natively (leveraging the terminal's own high-quality GPU-accelerated compositing). ScaleFit, on the other hand, applies a software resampling step before sending pixel data, which introduces the blurriness/roughness visible in your scale_fit screenshot.

The current approach of computing dstCols/dstRows for layout/cell-reservation purposes while using ScaleNone for rendering quality is the right trade-off here. No change needed.


✏️ Learnings added
Learnt from: krios2146
Repo: yorukot/superfile PR: 1385
File: src/pkg/file_preview/kitty.go:130-136
Timestamp: 2026-03-20T11:44:56.678Z
Learning: In src/pkg/file_preview/kitty.go (PR `#1385`, yorukot/superfile), using Scale(termimg.ScaleNone) with the Kitty graphics protocol is intentional and correct. It avoids software resampling by go-termimg and lets the Kitty terminal handle scaling natively, producing significantly better visual quality. Scale(termimg.ScaleFit) causes visible blurriness/roughness because it applies software interpolation before transmitting pixel data. The dstCols/dstRows values computed for layout/cell-reservation can still be passed via Width()/Height() alongside ScaleNone.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 841
File: src/pkg/file_preview/kitty.go:78-79
Timestamp: 2025-09-14T05:34:28.184Z
Learning: lazysegtree correctly identified that `\x1b[0m` is an ANSI reset sequence that resets text formatting to default, not a delay command as incorrectly commented in the Kitty graphics protocol clearing code in src/pkg/file_preview/kitty.go.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 825
File: src/internal/ui/rendering/renderer_core.go:95-108
Timestamp: 2025-05-22T04:42:07.369Z
Learning: In src/internal/ui/rendering/renderer_core.go, when truncating rendered output that exceeds the maximum height, we should drop all remaining lines after reaching totalHeight without adding an ellipsis or other truncation indicator, as the goal is to have exactly totalHeight lines.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, the preview panel refactoring created duplicate ImagePreviewer instances (one in defaultModelConfig and another in preview.New()), which caused image preview crashes. The refactoring also renamed SetContextWithRenderText to SetContentWithRenderText, and moved global batCmd to per-instance batCmd initialization, all contributing to crashes during image preview and test failures.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, lazysegtree reported image preview crashes and rename test failures after refactoring FilePreviewPanel to a separate preview package. The crashes were likely caused by: 1) imagePreviewer initialization differences between old global variable and new per-instance creation, 2) method name changes from SetContextWithRenderText to SetContentWithRenderText, and 3) batCmd initialization moving from global to per-instance without preserving the original configuration.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, the image preview crashes and test failures were caused by duplicate ImagePreviewer initialization - one in defaultModelConfig() and another in preview.New(). This created two separate instances that conflicted in terminal state management, causing crashes during image preview operations and test execution.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR `#1021` for yorukot/superfile, the tests were actually passing despite lazysegtree reporting crashes. The real issue was that production image preview crashes occurred during actual application usage due to duplicate ImagePreviewer instances (one in defaultModelConfig and one in preview.New()), while the test environment didn't stress the image preview system enough to expose the conflicts.

Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR `#767` for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.

@krios2146
Copy link
Copy Markdown
Author

Even though the AI comments are logically correct, go-termimg doesn't work this way

I tested almost all sensible configurations of parameters for the Render call, and tried the Widget API too; the only reliably working call is the one in the PR

@yorukot
Copy link
Copy Markdown
Owner

yorukot commented Mar 30, 2026

Hey, thank you for your PR. I will review it this week.

@yorukot
Copy link
Copy Markdown
Owner

yorukot commented Apr 21, 2026

Since bubbletea 2.0 includes an official image-rendering function, we suggest using bubbletea's library to render the image.

relate PR: #1406

@yorukot yorukot added pr needs maintainer action PR needs maintainer to do something, apart from reviewing(PR isn't authored by a maintainer) pr needs work PR needs work before it can be further reviewed/merged and removed awaiting pr review labels Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr needs maintainer action PR needs maintainer to do something, apart from reviewing(PR isn't authored by a maintainer) pr needs work PR needs work before it can be further reviewed/merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move from rasterm to go-terming for image preview

2 participants