feat: add go-termimg for image preview #1270
Open
Conversation
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
Add a new dependency: go-termimg Correct code for image previewing: kitty.go Fix format errors
Contributor
📝 WalkthroughWalkthroughReplaces rasterm-based Kitty image backend with go-termimg, updates go.mod dependencies, removes unused Kitty protocol constants, shortens the Kitty-renderer signature, and replaces several string concatenations with strings.Builder across rendering code. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Req as ImagePreviewRequest
participant Cap as CapabilityCheck
participant TermImg as go-termimg
participant Widget as ImageWidget
participant Buf as OutputBuffer
participant Term as Terminal
Req->>Cap: ask "is Kitty supported?"
Cap->>TermImg: termimg.KittySupported()
alt Kitty supported
Req->>TermImg: NewImageWidget(img)
TermImg->>Widget: create widget
Widget->>Widget: SetSizeWithCorrection / SetPosition / SetProtocol
Widget->>TermImg: Render()
TermImg-->>Buf: rendered bytes
Buf->>Term: termimg.ClearAll() then write bytes
else Not supported or render error
Req->>Buf: invoke ANSI fallback renderer
Buf->>Term: write ANSI output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-08-27T11:32:17.326ZApplied to files:
📚 Learning: 2025-08-27T11:32:17.326ZApplied to files:
📚 Learning: 2025-09-14T05:34:28.184ZApplied to files:
📚 Learning: 2025-08-27T11:32:17.326ZApplied to files:
📚 Learning: 2025-08-27T11:32:17.326ZApplied to files:
📚 Learning: 2025-03-29T10:02:04.260ZApplied to files:
📚 Learning: 2025-12-25T09:22:10.090ZApplied to files:
📚 Learning: 2025-08-24T03:24:50.857ZApplied to files:
🔇 Additional comments (7)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/internal/ui/rendering/border.go (1)
111-119: Partial refactor:infoTextvariable is now redundant.The
infoTextvariable is initialized as empty string and only used to append the builder's result. You could simplify by using the builder's output directly at line 124.🔎 Suggested simplification
- infoText := "" - var infoTextSb112 strings.Builder + var infoTextSb strings.Builder for _, item := range b.infoItems { item = ansi.Truncate(item, availWidth, "") - infoTextSb112.WriteString( + infoTextSb.WriteString( borderStrings.MiddleRight + item + borderStrings.MiddleLeft + borderStrings.Bottom, ) } - infoText += infoTextSb112.String() + infoText := infoTextSb.String() // Fill the rest with border char. remainingWidth := actualWidth - ansi.StringWidth(infoText)src/pkg/file_preview/kitty.go (1)
95-97: Error fromtermimg.ClearAll()is logged but not propagated.The error is logged but the function continues. If clearing fails, subsequent image rendering may produce artifacts. Consider whether this should affect the return value or if silent continuation is acceptable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modsrc/internal/model_render.gosrc/internal/ui/rendering/border.gosrc/pkg/file_preview/constants.gosrc/pkg/file_preview/image_preview.gosrc/pkg/file_preview/kitty.go
💤 Files with no reviewable changes (1)
- src/pkg/file_preview/constants.go
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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, 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, 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 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: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
📚 Learning: 2025-04-28T04:02:28.747Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/rendering/border.go:104-114
Timestamp: 2025-04-28T04:02:28.747Z
Learning: The BorderConfig.GetBorder method in src/internal/ui/rendering/border.go already protects against negative availWidth values through the condition `actualWidth >= cnt*4` which ensures that availWidth is at least 1.
Applied to files:
src/internal/ui/rendering/border.go
📚 Learning: 2025-09-09T14:23:14.164Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/ui/rendering/truncate.go:6-6
Timestamp: 2025-09-09T14:23:14.164Z
Learning: The truncate test failure in src/internal/ui/rendering/truncate_test.go during the ANSI package migration from experimental to stable was caused by a truncation bug in the experimental package. The experimental package incorrectly truncated strings even when input length equaled maxWidth (e.g., "1234" with maxWidth=4 became "1..."), while the stable package correctly only truncates when input exceeds maxWidth. The test expectation was based on the buggy behavior and needs to be corrected.
Applied to files:
src/internal/ui/rendering/border.gosrc/pkg/file_preview/image_preview.go
📚 Learning: 2025-08-24T03:24:50.857Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:18-21
Timestamp: 2025-08-24T03:24:50.857Z
Learning: The superfile project uses github.com/charmbracelet/x/ansi as the standardized ANSI package and deprecates github.com/charmbracelet/x/exp/term/ansi to avoid binary bloat and potential incompatibilities.
Applied to files:
src/internal/model_render.gogo.modsrc/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/image_preview.gosrc/pkg/file_preview/kitty.go
📚 Learning: 2025-09-14T05:34:28.184Z
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.
Applied to files:
src/pkg/file_preview/image_preview.gosrc/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/image_preview.gosrc/pkg/file_preview/kitty.go
🧬 Code graph analysis (2)
src/internal/model_render.go (2)
src/config/icon/icon.go (1)
Cursor(32-32)src/internal/common/style.go (2)
FilePanelCursorStyle(30-30)ModalStyle(20-20)
src/pkg/file_preview/kitty.go (1)
src/pkg/file_preview/image_preview.go (1)
ImagePreviewer(41-44)
🔇 Additional comments (5)
src/internal/model_render.go (1)
236-244: LGTM!The
strings.Builderrefactor avoids repeated string allocations in the loop, which is a good performance improvement. The logic and output remain unchanged.src/pkg/file_preview/image_preview.go (1)
139-140: LGTM!The call site correctly aligns with the updated
renderWithKittyUsingTermCapsignature that removes thepathparameter. The error handling flow (Kitty-first with ANSI fallback) remains intact.go.mod (2)
42-42: Note:nfnt/resizeis an unmaintained package.The
github.com/nfnt/resizepackage (indirect dependency via go-termimg) uses a commit from 2018 and hasn't been updated since. This is common for stable image processing libraries but worth noting for long-term maintenance awareness.
10-10: go-termimg v0.1.24 is the latest stable version.This dependency is confirmed as the current latest version according to the Go module proxy with no known security vulnerabilities. It is suitable for production use as the replacement for
rastermfor Kitty protocol rendering.src/pkg/file_preview/kitty.go (1)
126-131: LGTM on the rendering flow.The widget-based rendering with explicit protocol selection and proper error handling is clean. The rendered output is correctly written to the buffer.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pkg/file_preview/kitty.go
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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, 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, 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 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: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-09-14T05:34:28.184Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-24T03:24:50.857Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:18-21
Timestamp: 2025-08-24T03:24:50.857Z
Learning: The superfile project uses github.com/charmbracelet/x/ansi as the standardized ANSI package and deprecates github.com/charmbracelet/x/exp/term/ansi to avoid binary bloat and potential incompatibilities.
Applied to files:
src/pkg/file_preview/kitty.go
🔇 Additional comments (3)
src/pkg/file_preview/kitty.go (3)
117-122: Past review issue has been addressed.The previous review flagged that line 122 had swapped parameters in
SetPosition(dstRows, dstCols). The current code shows both lines 117 and 122 now use consistent parameter ordering:SetPosition(dstCols, dstRows). This appears to resolve the earlier concern, pending verification of the correct parameter semantics (see previous comment).
95-97:termimg.ClearAll()correctly clears Kitty images and placements.The implementation properly replaces the previous manual Kitty protocol commands (
\x1b_Ga=d\x1b\\for images and\x1b_Ga=d,p=1\x1b\\for placements). The API sends the Kitty graphics delete action (a=d) for all placements, achieving the same result with cleaner, more maintainable code.
18-18:termimg.KittySupported()and related APIs are correctly used.The code uses the documented go-termimg v0.1.24 APIs:
KittySupported()(line 18),ClearAll()(line 95),NewImageWidget(), and the widget methodsSetPosition(),SetSizeWithCorrection(),SetProtocol(), andRender(). All are exported functions available in the library's public API and are used correctly here.
lazysegtree
reviewed
Jan 5, 2026
lazysegtree
reviewed
Jan 5, 2026
Collaborator
|
One slightly hard part is ensuring it works with all major terminals, works on resizing, images can get cleared. We need to test it thoroughly. Could you post some screenshots or preferably videos. |
Collaborator
|
@olegshestakov Please make sure to address the coderabbitai comments, or explain why they aren't applicable. |
lazysegtree
added
the
pr needs work
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/pkg/file_preview/kitty.go:
- Around line 111-131: The widget is never positioned before rendering: call
widget.SetPosition(col, row) (note SetPosition is 0-based) after sizing with
widget.SetSizeWithCorrection and before calling widget.SetProtocol /
widget.Render so the image is placed correctly in the terminal grid (use the
same computed row/col values currently used for the manual cursor move); keep
SetProtocol(termimg.Kitty) and then call Render() and write the rendered output
as before.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pkg/file_preview/kitty.go
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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 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: 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, 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: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
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.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/sidebar/render.go:46-48
Timestamp: 2025-04-28T03:48:46.327Z
Learning: The user (lazysegtree) prefers to keep PRs focused and manageable in size, sometimes intentionally leaving TODO comments to track minor issues for later PRs rather than addressing everything at once.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:54:31.444Z
Learning: When lazysegtree says a review is "inaccurate and pre-existing issues. Not much useful," it means I should focus specifically on bugs or problems introduced by the current PR changes, not architectural concerns or code quality issues that were already present. The refactoring work in superfile PRs is generally well-implemented and should be evaluated on whether the specific changes work correctly, not on broader codebase architecture.
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-09-14T05:34:28.184Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-24T03:24:50.857Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:18-21
Timestamp: 2025-08-24T03:24:50.857Z
Learning: The superfile project uses github.com/charmbracelet/x/ansi as the standardized ANSI package and deprecates github.com/charmbracelet/x/exp/term/ansi to avoid binary bloat and potential incompatibilities.
Applied to files:
src/pkg/file_preview/kitty.go
🔇 Additional comments (4)
src/pkg/file_preview/kitty.go (4)
11-11: LGTM: Dependency replacement.The import of
go-termimgcorrectly replaces the rasterm-based implementation for Kitty protocol support.
84-86: LGTM: Simplified function signature.Removing the
pathparameter is correct, as placement ID generation is no longer needed with the go-termimg implementation.
95-97: LGTM: Proper error handling for image clearing.The use of
termimg.ClearAll()with error logging is appropriate. Continuing to render even if clearing fails is reasonable, as the new image should still be displayed.
18-18: The API verification confirms thattermimg.KittySupported()exists in the go-termimg library and returns aboolvalue indicating whether the current terminal supports the Kitty graphics protocol. The code change at line 18 is correct.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/file_preview/kitty.go (1)
124-149: Critical: SetPosition must be called BEFORE Render(), not after.Lines 125-148 have the execution order backwards.
widget.Render()is called at line 125, butwidget.SetPosition()isn't called until line 148—after the image has already been rendered. This violates the termimg API contract and makes positioning ineffective.Additionally, the manual cursor move at line 149 is redundant and contradicts the purpose of
SetPosition.Impact: Images will render at incorrect/undefined positions, particularly problematic when file preview borders are enabled (a key concern from the PR objectives).
🔧 Proposed fix: Reorder position → SetPosition → SetProtocol → Render
widget.SetSizeWithCorrection(dstCols, dstRows) } + // Calculate position considering side area and border + row := 1 + col := sideAreaWidth + 1 + if common.Config.EnableFilePreviewBorder { + row++ + col++ + } + + // Position the widget BEFORE rendering (0-based indexing for terminal cells) + widget.SetPosition(col-1, row-1) + // Write image using Kitty protocol widget.SetProtocol(termimg.Kitty) rendered, renderError := widget.Render() if renderError != nil { return "", renderError } buf.WriteString(rendered) // TODO: using internal/common package in pkg package is against the standards // We shouldn't use that here. // Other usage of common in `file_preview` should be removed too. // common.VideoExtensions should be moved to fixed_variables // and internal/common/utils shoud move to pkg/utils so that it can // be used by everyone // TODO : Ideally we should not need the kitty previewer to be // aware of full modal width and make decisions based on global config // A better solutions than this is needed for it. - row := 1 - col := sideAreaWidth + 1 - if common.Config.EnableFilePreviewBorder { - row++ - col++ - } - - widget.SetPosition(col-1, row-1) - buf.WriteString(fmt.Sprintf("\x1b[%d;%dH", row, col)) return buf.String(), nilNote: This directly addresses the unresolved past review comment that stated "call widget.SetPosition(col, row) after sizing and before calling widget.SetProtocol / widget.Render".
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pkg/file_preview/kitty.go
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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 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: 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, 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: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
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.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/sidebar/render.go:46-48
Timestamp: 2025-04-28T03:48:46.327Z
Learning: The user (lazysegtree) prefers to keep PRs focused and manageable in size, sometimes intentionally leaving TODO comments to track minor issues for later PRs rather than addressing everything at once.
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-09-14T05:34:28.184Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
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.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-24T03:24:50.857Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:18-21
Timestamp: 2025-08-24T03:24:50.857Z
Learning: The superfile project uses github.com/charmbracelet/x/ansi as the standardized ANSI package and deprecates github.com/charmbracelet/x/exp/term/ansi to avoid binary bloat and potential incompatibilities.
Applied to files:
src/pkg/file_preview/kitty.go
🧬 Code graph analysis (1)
src/pkg/file_preview/kitty.go (2)
src/pkg/file_preview/image_preview.go (1)
ImagePreviewer(41-44)src/internal/ui/preview/model.go (1)
New(27-48)
🔇 Additional comments (4)
src/pkg/file_preview/kitty.go (4)
11-11: LGTM: Clean dependency migration to go-termimg.The import change aligns with the PR objective to replace rasterm with go-termimg for Kitty protocol support.
17-45: Good fallback detection for Kitty support.The primary detection using
termimg.KittySupported()is appropriate, and the fallback logic preserves compatibility with terminals that might not be auto-detected. The known terminals list covers major implementations.
84-86: Signature simplification aligns with go-termimg API.Removing the
pathparameter is appropriate since go-termimg doesn't require path-based placement identifiers, simplifying the API surface.
109-121: Correct widget sizing with aspect ratio preservation.The widget creation and
SetSizeWithCorrectioncalls properly handle dimension calculations for both aspect ratio branches. The past issue with incorrectly passing dimensions toSetPositionhas been resolved.
Author
|
@lazysegtree demo https://drive.google.com/file/d/1i2v7gnjGY6t2A9oVClG8nBWWncndoGY0/view?usp=drive_link. |
lazysegtree
reviewed
Jan 8, 2026
lazysegtree
reviewed
Jan 8, 2026
| @@ -7,10 +7,11 @@ require ( | |||
| github.com/alecthomas/chroma/v2 v2.16.0 | |||
Collaborator
There was a problem hiding this comment.
One thing is that intermittently, first few keypress are getting ignored. I think something is messing with the terminal stdin.
Thats causing testsuite to fail, which sends a, then ctrl+c, then down, then enter, and then ctrl+v. And then esc to close the application
But only ctrl+v and esc reaches the application
Details
➜ ~/Workspace/Other_proj/spf_forks/olegshestakov/testsuite git:(main) ✗ [8:55:06] .venv/bin/python3 main.py -d
[2026-01-08 08:56:00 - DEBUG] server object : Server(socket_name=superfile)
[2026-01-08 08:56:00 - DEBUG] Initialized TestFSManager
[2026-01-08 08:56:00 - DEBUG] Found a testcase CopyDirTest, in module copy_dir_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase CommandTest, in module command_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase ChooserFileTest, in module chooser_file_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase CutTest, in module cut_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase DeleteDirTest, in module delete_dir_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase CopyWTest, in module copyw_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase EmptyPanelTest, in module empty_panel_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase CopyTest, in module copy_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase DeleteTest, in module delete_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase CompressExtractTest, in module compress_extract_test
[2026-01-08 08:56:00 - DEBUG] Found a testcase RenameTest, in module rename_test
[2026-01-08 08:56:00 - INFO] Testcases : [CopyDirTest, CommandTest, ChooserFileTest, CutTest, DeleteDirTest, CopyWTest, EmptyPanelTest, CopyTest, DeleteTest, CompressExtractTest, RenameTest]
[2026-01-08 08:56:00 - INFO] Running test CopyDirTest
[2026-01-08 08:56:00 - DEBUG] Current file structure :
D-dir2
D-dir1
D-dir1/nested1
D-dir1/nested2
F-dir1/nested1/file1.txt
[2026-01-08 08:56:00 - DEBUG] windows_command : /Users/kuknitin/Workspace/Other_proj/spf_forks/olegshestakov/bin/spf -c /Users/kuknitin/Workspace/Other_proj/spf_forks/olegshestakov/src/superfile_config/config.toml --hf /Users/kuknitin/Workspace/Other_proj/spf_forks/olegshestakov/src/superfile_config/hotkeys.toml
[2026-01-08 08:56:00 - DEBUG] spf_session initialised : Session($0 spf_session)
[2026-01-08 08:56:01 - DEBUG] sending key : 'a'
[2026-01-08 08:56:01 - DEBUG] sending key : '\x03'
[2026-01-08 08:56:01 - DEBUG] sending key : 'Down'
[2026-01-08 08:56:01 - DEBUG] sending key : '\r'
[2026-01-08 08:56:01 - DEBUG] sending key : '\x16'
[2026-01-08 08:56:01 - DEBUG] sending key : '\x1b'
[2026-01-08 08:56:02 - DEBUG] Finished Execution
[2026-01-08 08:56:02 - DEBUG] spf_manager info : [], Current file structure :
D-dir2
D-dir1
D-dir1/nested1
D-dir1/nested2
F-dir1/nested1/file1.txt
[2026-01-08 08:56:02 - DEBUG] Test assertion failed : File copy_dir/dir2/dir1 does not exists
Traceback (most recent call last):
File "/Users/kuknitin/Workspace/Other_proj/spf_forks/olegshestakov/testsuite/core/base_test.py", line 138, in validate
assert self.env.fs_mgr.check_exists(file_path), f"File {file_path} does not exists"
AssertionError: File copy_dir/dir2/dir1 does not exists
[2026-01-08 08:56:02 - ERROR] Failed test CopyDirTest
[2026-01-08 08:56:02 - INFO] Finished running 1 test. 0 passed
➜ ~/Workspace/Other_proj/spf_forks/olegshestakov/testsuite git:(main) ✗ [8:56:02]
app logs
time=2026-01-08T09:09:54.471+05:30 level=DEBUG msg="Runtime information" runtime.GOOS=darwin
time=2026-01-08T09:09:54.471+05:30 level=DEBUG msg="Memory usage" alloc_bytes=2189192 total_alloc_bytes=4735384 heap_objects=27223 sys_bytes=12013584
time=2026-01-08T09:09:54.471+05:30 level=DEBUG msg="Object sizes" model_size_bytes=32056 filePanel_size_bytes=10600 sidebarModel_size_bytes=10448 renderer_size_bytes=520 borderConfig_size_bytes=88 process_size_bytes=888
time=2026-01-08T09:09:54.472+05:30 level=DEBUG msg="Directory configuration" cwd=/var/folders/tp/8gddycn174j124g7lkl42trh0000gn/T/tmpn0wp7mx0/copy_dir start_paths=[/var/folders/tp/8gddycn174j124g7lkl42trh0000gn/T/tmpn0wp7mx0/copy_dir]
time=2026-01-08T09:09:54.472+05:30 level=DEBUG msg="Runtime information" runtime.GOOS=darwin
time=2026-01-08T09:09:54.472+05:30 level=DEBUG msg="Memory usage" alloc_bytes=2305608 total_alloc_bytes=4851800 heap_objects=28974 sys_bytes=12013584
time=2026-01-08T09:09:54.472+05:30 level=DEBUG msg="Object sizes" model_size_bytes=32056 filePanel_size_bytes=10600 sidebarModel_size_bytes=10448 renderer_size_bytes=520 borderConfig_size_bytes=88 process_size_bytes=888
time=2026-01-08T09:09:54.473+05:30 level=INFO msg="Successfully detected terminal cell size via ioctl" pixels_per_column=16 pixels_per_row=32
time=2026-01-08T09:09:54.473+05:30 level=INFO msg="Terminal cell size detection" pixels_per_column=16 pixels_per_row=32
time=2026-01-08T09:09:54.473+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=0 footerHeight=0 fullHeight=0 fullWidth=0 panelCount=1 singlePanelWidth=10 maxPanels=0 sideBarWidth=20 firstFilePanelWidth=18
time=2026-01-08T09:09:54.473+05:30 level=DEBUG msg="model.Update() called" msgType=tea.WindowSizeMsg
time=2026-01-08T09:09:55.045+05:30 level=DEBUG msg="Submitting file preview render request" id=0 path=/var/folders/tp/8gddycn174j124g7lkl42trh0000gn/T/tmpn0wp7mx0/copy_dir/dir1 w=29 h=16
time=2026-01-08T09:09:55.045+05:30 level=DEBUG msg="Submitting metadata fetch request" id=0 path=/var/folders/tp/8gddycn174j124g7lkl42trh0000gn/T/tmpn0wp7mx0/copy_dir/dir1
time=2026-01-08T09:09:55.047+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
time=2026-01-08T09:09:55.051+05:30 level=DEBUG msg="model.Update() called" msgType=cursor.initialBlinkMsg
time=2026-01-08T09:09:55.051+05:30 level=DEBUG msg="Message of type that is not explicitly handled"
time=2026-01-08T09:09:55.051+05:30 level=DEBUG msg="Attempting to render preview" itemPath=/var/folders/tp/8gddycn174j124g7lkl42trh0000gn/T/tmpn0wp7mx0/copy_dir/dir1 mode=drwxr-xr-x isRegular=false
time=2026-01-08T09:09:55.052+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
time=2026-01-08T09:09:55.055+05:30 level=DEBUG msg="model.Update() called" msgType=tea.setWindowTitleMsg
time=2026-01-08T09:09:55.055+05:30 level=DEBUG msg="Message of type that is not explicitly handled"
time=2026-01-08T09:09:55.055+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
time=2026-01-08T09:09:55.056+05:30 level=DEBUG msg="model.Update() called" msgType=preview.UpdateMsg
time=2026-01-08T09:09:55.056+05:30 level=DEBUG msg="Got ModelUpdate message" id=0
time=2026-01-08T09:09:55.057+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
time=2026-01-08T09:09:55.059+05:30 level=DEBUG msg="model.Update() called" msgType=internal.MetadataMsg
time=2026-01-08T09:09:55.059+05:30 level=DEBUG msg="Got ModelUpdate message" id=0
time=2026-01-08T09:09:55.059+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
time=2026-01-08T09:09:55.227+05:30 level=DEBUG msg="model.Update() called" msgType=tea.KeyMsg
time=2026-01-08T09:09:55.227+05:30 level=DEBUG msg=model.handleKeyInput msg=ctrl+c typestr=ctrl+c runes=[] type=3 paste=false alt=false
time=2026-01-08T09:09:55.227+05:30 level=DEBUG msg="model.handleKeyInput. model info. " fileModel.FocusedPanelIndex=0 filePanel.isFocused=true filePanel.panelMode=browserMode typingModal.open=false notifyModel.open=false promptModal.open=false fileModel.renaming=false searchBar.focused=false helpMenu.open=false firstTextInput=false focusPanel=nonePanelFocus
time=2026-01-08T09:09:55.227+05:30 level=DEBUG msg=handle_file_operations.copySingleItem cut=false "panel location"=/var/folders/tp/8gddycn174j124g7lkl42trh0000gn/T/tmpn0wp7mx0/copy_dir/dir1
time=2026-01-08T09:09:55.227+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
time=2026-01-08T09:09:55.832+05:30 level=DEBUG msg="model.Update() called" msgType=tea.KeyMsg
time=2026-01-08T09:09:55.832+05:30 level=DEBUG msg=model.handleKeyInput msg=esc typestr=esc runes=[] type=27 paste=false alt=false
time=2026-01-08T09:09:55.832+05:30 level=DEBUG msg="model.handleKeyInput. model info. " fileModel.FocusedPanelIndex=0 filePanel.isFocused=true filePanel.panelMode=browserMode typingModal.open=false notifyModel.open=false promptModal.open=false fileModel.renaming=false searchBar.focused=false helpMenu.open=false firstTextInput=false focusPanel=nonePanelFocus
time=2026-01-08T09:09:55.832+05:30 level=DEBUG msg="Quitting superfile" "current dir"=/var/folders/tp/8gddycn174j124g7lkl42trh0000gn/T/tmpn0wp7mx0/copy_dir
time=2026-01-08T09:09:55.833+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
time=2026-01-08T09:09:55.833+05:30 level=DEBUG msg="model.View() called" mainPanelHeight=14 footerHeight=6 fullHeight=24 fullWidth=80 panelCount=1 singlePanelWidth=29 maxPanels=1 sideBarWidth=20 firstFilePanelWidth=29
Author
There was a problem hiding this comment.
got it, I thought it was some bug in my env. I'll try to fix
lazysegtree
reviewed
Jan 8, 2026
| @@ -7,10 +7,11 @@ require ( | |||
| github.com/alecthomas/chroma/v2 v2.16.0 | |||
| github.com/atotto/clipboard v0.1.4 | |||
Collaborator
There was a problem hiding this comment.
Second thing is that the images are getting rendered in low-res on specific terminal sizes.
MacOS, kitty/wezterm
https://drive.google.com/file/d/1JwSXl78UMwhVQSla43hDXkGCdhcXjm9M/view?usp=drive_link
Author
There was a problem hiding this comment.
could you please try these images with lib examples? I mean try these ones https://pkg.go.dev/github.com/blacktop/go-termimg#section-readme
I cloned package on my laptop and I got blurred images for low resolution
Author
There was a problem hiding this comment.
Author
There was a problem hiding this comment.
@lazysegtree could you please take a look?
if you confirm this behavior, does it have any sense to integrate this lib instead of existing?
Collaborator
There was a problem hiding this comment.
Okay, I will check. What I am guessing is that the library isn't broken, but we might not be using it correctly. Only way to figure it out is comparing the code of go-terming and rasterm, and pointing out the difference that make go-terming images blurry in our case.
If its actually broken, then I will rather push a fix in the go-terming library. Its better for the long term. As rasterm's author has demonstrated the non-willingness to maintain it.
Collaborator
|
Looks like there might be issue with go-terming |
Collaborator
|
I checked your code, it looks that there are no issues. Will need to dig into go-terming implementation to understand whats going on. |
Collaborator
|
Image preview is my least favorite thing in superfile :) I am already half-stuck in getting inline protocol to work. |
lazysegtree
added
needs investigation
image-preview
difficulty:hard
upstream issue
lazysegtree
removed
difficulty:hard
upstream issue
lazysegtree
added
the
pr needs maintainer action
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes - #1217
Summary by CodeRabbit
Improvements
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.