feat: User rendering package and eliminate FilePreviewBox, and other unused functions#1245
Merged
Merged
feat: User rendering package and eliminate FilePreviewBox, and other unused functions#1245
Conversation
Contributor
|
Warning Rate limit exceeded@lazysegtree has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaced direct Lip Gloss box rendering with a centralized Renderer for previews, removed multiple border-style helpers, added file-preview message constants and a style-modifier mechanism on Renderer, and eliminated several small preview helper functions and utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Model
participant Renderer
participant Terminal
participant FS as FileSystem
Note over Model,Renderer: Preview request flow (image/text)
Model->>Renderer: Create/Configure renderer (width/height, AddStyleModifier)
Model->>FS: Read file / determine type
FS-->>Model: File content / metadata
alt image (kitty protocol)
Model->>Renderer: Append kitty protocol lines
Renderer-->>Terminal: Render kitty lines + clearCmd
else image (non-kitty) or text
Model->>Renderer: AddLines / content
Renderer-->>Terminal: Render boxed output (centered/aligned) + clearCmd
end
Terminal-->>Model: (displayed)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Deploying superfile with
|
| Latest commit: |
29b50f5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://29612512.superfile.pages.dev |
| Branch Preview URL: | https://fix-preview-border.superfile.pages.dev |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/internal/common/style_function_test.go (1)
56-72: Consider adding edge case tests.The current tests cover basic scenarios well, but small height values (height=0, height=1, height=2) could reveal issues with the border-building loop, particularly the special handling of
BorderMiddleRightat index 1.Suggested additional test cases
{ name: "Height of 1 with border", showBorder: true, height: 1, width: 10, expectBorder: true, }, { name: "Height of 2 with border", showBorder: true, height: 2, width: 10, expectBorder: true, },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/internal/common/config_type.gosrc/internal/common/style_function.gosrc/internal/common/style_function_test.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, 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 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 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: 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.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-12-25T04:09:23.241Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1227
File: src/internal/common/config_type.go:94-94
Timestamp: 2025-12-25T04:09:23.241Z
Learning: In the superfile sidebar component (src/internal/ui/sidebar/), the SidebarWidth configuration value represents the sidebar content width excluding borders, while the minWidth = 7 constant in consts.go represents the minimum total width including borders. The actual sidebar width is computed as SidebarWidth + BorderPadding in sidebar.go's New() function.
Applied to files:
src/internal/common/config_type.go
📚 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/common/style_function_test.gosrc/internal/common/style_function.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/common/style_function_test.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/internal/common/style_function_test.gosrc/internal/common/style_function.go
🧬 Code graph analysis (1)
src/internal/common/style_function.go (2)
src/internal/common/style.go (3)
FilePanelBGColor(78-78)FilePanelFGColor(84-84)FilePanelBorderColor(68-68)src/internal/common/default_config.go (1)
Config(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and Test (windows-latest)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
src/internal/common/config_type.go (1)
93-93: LGTM! Clean configuration field addition.The new
ShowPreviewBorderfield is well-integrated with clear naming, appropriate TOML tagging, and a descriptive comment.src/internal/common/style_function.go (2)
40-44: Good refactoring to avoid duplication.Creating the base style once and conditionally extending it with a border is cleaner than duplicating the width, height, background, and foreground settings in both branches.
46-62: Border implementation is correct and intentional.The preview panel correctly renders top and right borders only, which is an intentional design for a panel positioned on the right side of the layout. The test suite explicitly validates this behavior with the
expectRightBorderflag. Bottom and left borders are properly cleared to avoid visual overlap with adjacent panels.src/internal/common/style_function_test.go (1)
10-15: Good practice preserving original config.Properly saves and restores the
Config.ShowPreviewBordervalue to avoid test pollution.
lazysegtree
marked this pull request as draft
December 31, 2025 17:11
lazysegtree
force-pushed
the
fix-preview-border
branch
from
December 31, 2025 17:12
285eeda to
8605744
Compare
lazysegtree
force-pushed
the
fix-preview-border
branch
from
December 31, 2025 17:45
8605744 to
f74627f
Compare
lazysegtree
marked this pull request as ready for review
January 1, 2026 06:24
lazysegtree
commented
Jan 1, 2026
| "github.com/charmbracelet/lipgloss" | ||
| "github.com/yorukot/ansichroma" | ||
|
|
||
| "github.com/yorukot/superfile/src/config/icon" |
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai Ensure that this refactoring hasn't broken any existing behaviour, apart from minor updates in the error messages.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for any references to the icon package in the preview/render.go file
echo "=== Checking for 'icon.' references in preview/render.go ==="
rg -n 'icon\.' src/internal/ui/preview/render.go || echo "No references to icon. found"
echo -e "\n=== Checking for any 'icon' references (broader search) ==="
rg -n -i '\bicon\b' src/internal/ui/preview/render.go || echo "No references to icon found"
echo -e "\n=== Checking if code compiles ==="
cd src && go build ./... 2>&1 | head -20 || echo "Build completed or has errors"
echo -e "\n=== Checking the icon package location and exports ==="
fd -t f "icon" --full-path "src/config" -x echo "Found: {}"
fd -t f "icon.go" --full-path "src/config" -x head -50 {}
Length of output: 1614
@lazysegtree I've verified that this refactoring is safe and hasn't broken any existing behavior:
Verification Results:
-
✅ No direct usage: The file contains no references to
icon.- the icon package was never directly used inpreview/render.go -
✅ Compilation successful: The code compiles without errors
-
✅ Functionality preserved: The file uses
common.GetElementIcon()on line 55, which internally handles all icon-related functionality. The icon rendering behavior remains unchanged:style := common.GetElementIcon(file.Name(), file.IsDir(), isLink, common.Config.Nerdfont) res := lipgloss.NewStyle().Foreground(lipgloss.Color(style.Color)).Background(common.FilePanelBGColor). Render(style.Icon+" ") + common.FilePanelStyle.Render(file.Name())
-
✅ Proper encapsulation: The icon package is correctly imported and used in
src/internal/common/icon_utils.go, whereGetElementIcon()is defined
Conclusion: This is a clean removal of an unused import. The refactoring maintains all existing behavior since the icon functionality was always accessed through the common package abstraction, never directly through the icon package in this file.
🧬 Code Graph Analysis Results
<src/internal/ui/preview/model.go> 11-25
Model struct {
open bool
// Location denotes what is supposed to be in model.
// Might not be always in sync with content
location string
content string
contentWidth int
contentHeight int
loading bool
imagePreviewer *filepreview.ImagePreviewer
batCmd string
thumbnailGenerator *filepreview.ThumbnailGenerator
}
<src/internal/ui/spf_renderers.go> 58-71
func FilePreviewPanelRenderer(totalHeight int, totalWidth int) *rendering.Renderer {
cfg := rendering.DefaultRendererConfig(totalHeight, totalWidth)
cfg.ContentFGColor = common.FilePanelFGColor
cfg.ContentBGColor = common.FilePanelBGColor
cfg.BorderRequired = false
cfg.RendererName += "-preview"
r, err := rendering.NewRenderer(cfg)
if err != nil {
slog.Error("Error in creating renderer. Falling back to default renderer", "error", err)
r = &rendering.Renderer{}
}
return r
}
<src/internal/utils/file_utils.go> 254-277
func ReadFileContent(filepath string, maxLineLength int, previewLine int) (string, error) {
var resultBuilder strings.Builder
file, err := os.Open(filepath)
if err != nil {
return resultBuilder.String(), err
}
defer file.Close()
reader := transform.NewReader(file, unicode.BOMOverride(unicode.UTF8.NewDecoder()))
scanner := bufio.NewScanner(reader)
lineCount := 0
for scanner.Scan() {
line := scanner.Text()
line = ansi.Truncate(line, maxLineLength, "")
resultBuilder.WriteString(line)
resultBuilder.WriteRune('\n')
lineCount++
if previewLine > 0 && lineCount >= previewLine {
break
}
}
// returns the first non-EOF error that was encountered by the [Scanner]
return resultBuilder.String(), scanner.Err()
}
<src/internal/common/string_function.go> 187-201
func IsTextFile(filename string) (bool, error) {
file, err := os.Open(filename)
if err != nil {
return false, err
}
defer file.Close()
reader := bufio.NewReader(file)
buffer := make([]byte, DefaultBufferSize)
cnt, err := reader.Read(buffer)
if err != nil && !errors.Is(err, io.EOF) {
return false, err
}
return IsBufferPrintable(buffer[:cnt]), nil
}
<src/internal/common/icon_utils.go> 51-72
func GetElementIcon(file string, isDir bool, isLink bool, nerdFont bool) icon.Style {
if !nerdFont {
return icon.Style{
Icon: "",
Color: Theme.FilePanelFG,
}
}
if isDir {
if isLink {
return icon.Folders["link_folder"]
}
resultIcon := icon.Folders["folder"]
betterIcon, hasBetterIcon := icon.Folders[file]
if hasBetterIcon {
resultIcon = betterIcon
}
return resultIcon
}
return getFileIcon(file, isLink)
}
<src/internal/ui/preview/ render.go> (referenced for context)
<src/internal/common/predefined_variable.go> 57-65
FilePreviewDirectoryUnreadableText string
<src/internal/common/predefined_variable.go> 58-58
FilePreviewEmptyText string
<src/internal/common/predefined_variable.go> 60-60
FilePreviewPanelClosedText string
<src/internal/common/predefined_variable.go> 61-61
FilePreviewImagePreviewDisabledText string
<src/internal/common/predefined_variable.go> 62-62
FilePreviewUnsupportedImageFormatsText string
<src/internal/common/predefined_variable.go> 63-63
FilePreviewImageConversionErrorText string
<src/internal/common/predefined_variable.go> 64-64
FilePreviewBatNotInstalledText string
<src/internal/common/predefined_variable.go> 55-55
FilePreviewUnsupportedFormatText string
<src/internal/common/predefined_variable.go> 80-80
UnsupportedPreviewFormats = []string{".torrent"}
🧠 Learnings used
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: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/model_msg.go:58-62
Timestamp: 2025-09-16T07:17:07.854Z
Learning: lazysegtree correctly identified a race condition bug in DeleteOperationMsg.ApplyToModel() where asynchronous delete operations unconditionally reset the current file selection even if the user has selected different files after starting the delete operation, leading to unexpected selection clearing.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 965
File: src/internal/config_function.go:8-8
Timestamp: 2025-07-29T03:42:36.581Z
Learning: When reviewing large refactoring PRs that introduce new async patterns (like moving from global channels to dedicated processbar channels), always check for incomplete refactoring where critical functionality like navigation depends on data structures that are no longer being populated, as this often leads to completely broken features.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:32:06.922Z
Learning: When reviewing large refactoring PRs that change async patterns (like moving from goroutines to tea.Cmd), always check for incomplete refactoring where some call sites still use the old pattern while others use the new pattern, as this often leads to compilation errors and architectural inconsistencies.
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: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
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.
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai I meant to review all changes I did in src/internal/ui/preview/render.go
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the full diff for src/internal/ui/preview/render.go
echo "=== Full git diff for src/internal/ui/preview/render.go ==="
cd /tmp/coderabbit && git diff HEAD~1 HEAD -- src/internal/ui/preview/render.go
echo -e "\n=== Compilation check ==="
cd /tmp/coderabbit/src && go build ./internal/ui/preview 2>&1 || echo "Build completed"
echo -e "\n=== Check clearCmd usage patterns ==="
rg -n 'clearCmd' src/internal/ui/preview/render.go
echo -e "\n=== Check all Render() calls ==="
rg -n '\.Render\(\)' src/internal/ui/preview/render.go
echo -e "\n=== Check new FilePreview constants usage ==="
rg -n 'common\.FilePreview' src/internal/ui/preview/render.go
echo -e "\n=== Check for any direct lipgloss.Style usage or box creation ==="
rg -n 'lipgloss\.NewStyle\(\)|Box\(' src/internal/ui/preview/render.go
Length of output: 4043
🏁 Script executed:
#!/bin/bash
# Try to get changes by comparing with git show
echo "=== Show the commit changes for render.go ==="
git log --oneline -5 -- src/internal/ui/preview/render.go
git show HEAD:src/internal/ui/preview/render.go > /tmp/old_render.go 2>/dev/null || echo "Could not get old version"
diff -u /tmp/old_render.go src/internal/ui/preview/render.go | head -200 || echo "Diff command failed"
echo -e "\n=== Check for imagePreviewer initialization patterns ==="
rg -n 'imagePreviewer.*=' src/internal/ui/preview/ -A 2 -B 2
echo -e "\n=== Check renderImagePreview calls and clearCmd handling ==="
rg -n 'renderImagePreview' src/internal/ui/preview/render.go -A 1 -B 1
echo -e "\n=== Check if renderDirectoryPreview signature or behavior changed ==="
ast-grep --pattern 'func renderDirectoryPreview($$$) $_ {
$$$
}'
echo -e "\n=== Verify renderer usage consistency across all preview functions ==="
rg -n 'func.*render.*Preview' src/internal/ui/preview/render.go -A 3
Length of output: 4502
🏁 Script executed:
#!/bin/bash
# Check how RenderWithPath orchestrates the clearCmd
echo "=== RenderWithPath function showing clearCmd flow ==="
sed -n '163,210p' src/internal/ui/preview/render.go
echo -e "\n=== Check if renderTextPreview should handle images ==="
rg -n 'isImageFile|ImagePreview' src/internal/ui/preview/render.go
echo -e "\n=== Verify the comment about clearCmd issues ==="
sed -n '194,201p' src/internal/ui/preview/render.go
echo -e "\n=== Check old helper functions that were removed ==="
rg -n 'FilePreviewBox|renderFileInfoError|renderUnsupportedFormat' src/internal/ui/preview/
echo -e "\n=== Check if tests pass ==="
cd src && go test ./internal/ui/preview/... -v 2>&1 | head -50
echo -e "\n=== Check for any panics or nil pointer dereference risks ==="
rg -n 'imagePreviewer\.|thumbnailGenerator\.' src/internal/ui/preview/render.go | head -20
Length of output: 6219
🏁 Script executed:
#!/bin/bash
# Final verification checks
echo "=== Check for duplicate ImagePreviewer initialization (from learnings) ==="
rg -n 'ImagePreviewer' src/internal/ui/preview/ src/internal/default_config.go -B 2 -A 2
echo -e "\n=== Verify all error constants are defined ==="
rg -n 'FilePreviewPanelClosedText|FilePreviewImagePreviewDisabledText|FilePreviewUnsupportedImageFormatsText|FilePreviewImageConversionErrorText|FilePreviewBatNotInstalledText|FilePreviewThumbnailGenerationErrorText' src/internal/common/predefined_variable.go
echo -e "\n=== Check StyleModifier usage for alignment ==="
rg -n 'AddStyleModifier|AlignHorizontal|AlignVertical' src/internal/ui/preview/render.go
echo -e "\n=== Verify no box-based rendering remains ==="
rg -n 'FilePreviewBox' src/internal/
echo -e "\n=== Check rendering test results ==="
cd src && go test ./internal/ui/preview/... -run TestFilePreviewRenderWithDimensions 2>&1 | tail -20
Length of output: 1908
@lazysegtree I've completed a comprehensive review of all changes in src/internal/ui/preview/render.go. Here's my analysis:
✅ Major Refactoring Successfully Completed
1. Architectural Migration to Renderer Pattern
All three preview functions now use the centralized *rendering.Renderer abstraction:
renderDirectoryPreview(r *rendering.Renderer, ...)renderImagePreview(r *rendering.Renderer, ..., clearCmd string)renderTextPreview(r *rendering.Renderer, ...)
This eliminates direct lipgloss.Style box construction and provides consistent rendering behavior.
2. Proper clearCmd Management ✅
The Kitty image clearing is correctly handled:
- RenderWithPath: Creates
clearCmdonce (line 164) and appends it to all non-image paths (lines 169, 177, 182, 186, 193, 209) - renderImagePreview: Takes
clearCmdas parameter and includes it in returns where appropriate - Kitty protocol path (lines 86-89): Intentionally omits
clearCmdsince Kitty escape sequences handle rendering differently - ANSI rendering path (line 94): Correctly appends
clearCmd
The comment at lines 195-197 about potential clearCmd issues is noted, but the current implementation is correct - renderImagePreview already returns with clearCmd included.
3. Error Message Centralization ✅
All error messages migrated to common.FilePreview*Text constants:
FilePreviewPanelClosedTextFilePreviewImagePreviewDisabledTextFilePreviewUnsupportedImageFormatsTextFilePreviewImageConversionErrorTextFilePreviewBatNotInstalledTextFilePreviewThumbnailGenerationErrorText
All properly defined in predefined_variable.go.
4. StyleModifier for Alignment ✅
New capability for proper Kitty protocol handling (lines 92-94):
return r.AddStyleModifier(func(s lipgloss.Style) lipgloss.Style {
return s.AlignHorizontal(lipgloss.Center).AlignVertical(lipgloss.Center)
}).AddLines(imageRender).Render() + clearCmd5. Import Cleanup ✅
- Removed unused
iconpackage import - Icon functionality properly accessed via
common.GetElementIcon()(line 53) FilePreviewBoxcompletely removed from codebase
6. No Regression from PR #1021 ✅
Verified single ImagePreviewer initialization in preview.New() - no duplicate instances that caused crashes in the previous refactoring.
7. Tests Passing ✅
ok github.com/yorukot/superfile/src/internal/ui/preview 0.053s
Conclusion
This refactoring maintains all existing behavior while successfully:
- Migrating to the centralized rendering package
- Eliminating
FilePreviewBoxand related helper functions - Centralizing error messages
- Properly managing Kitty image clearing
- Compiling cleanly and passing all tests
The only changes are architectural improvements and minor updates to error message text as you intended. No functional regressions detected. 🎉
🧬 Code Graph Analysis Results
[src/internal/ui/preview/model.go] (lines 11-25)
Model struct {
open bool
// Location denotes what is supposed to be in model.
// Might not be always in sync with content
location string
content string
contentWidth int
contentHeight int
loading bool
imagePreviewer *filepreview.ImagePreviewer
batCmd string
thumbnailGenerator *filepreview.ThumbnailGenerator
}
[src/internal/ui/rendering/renderer.go] (lines 18-66)
Renderer struct {
// Current sectionization will not allow to predefine section
// but only allow adding them via AddSection(). Hence trucateWill be applicable to
// last section only.
contentSections []ContentRenderer
// Empty for last section . len(sectionDividers) should be equal to len(contentSections) - 1
sectionDividers []string
curSectionIdx int
// Including Dividers - Count of actual lines that were added. It maybe <= totalHeight - 2
actualContentHeight int
defTruncateStyle TruncateStyle
// Whether to reduce rendered height to fit number of lines
truncateHeight bool
border BorderConfig
// Should this go in contentRenderer - No . ContentRenderer is not for storing style configs
contentFGColor lipgloss.TerminalColor
contentBGColor lipgloss.TerminalColor
// Should this go in borderConfig ?
borderFGColor lipgloss.TerminalColor
borderBGColor lipgloss.TerminalColor
// Use this to add additional style modifications
// This is applied before any style update that are defined by other configurations,
// like border, height, width. Hence if conflicting styles are used, they can get
// overridden
styleModifiers []StyleModifier
// Maybe better rename these to maxHeight
// Final rendered string should have exactly this many lines, including borders
// But if truncateHeight is true, it maybe be <= totalHeight
totalHeight int
// Every line should have at most this many characters, including borders
totalWidth int
contentHeight int
contentWidth int
borderRequired bool
borderStrings lipgloss.Border
// for logging
name string
}
[src/internal/ui/spf_renderers.go] (lines 58-71)
func FilePreviewPanelRenderer(totalHeight int, totalWidth int) *rendering.Renderer {
cfg := rendering.DefaultRendererConfig(totalHeight, totalWidth)
cfg.ContentFGColor = common.FilePanelFGColor
cfg.ContentBGColor = common.FilePanelBGColor
cfg.BorderRequired = false
cfg.RendererName += "-preview"
r, err := rendering.NewRenderer(cfg)
if err != nil {
slog.Error("Error in creating renderer. Falling back to default renderer", "error", err)
r = &rendering.Renderer{}
}
return r
}
[src/internal/common/predefined_variable.go] (lines 57-65)
FilePreviewDirectoryUnreadableText string
[src/internal/common/predefined_variable.go] (lines 58-58)
FilePreviewEmptyText string
[src/internal/common/predefined_variable.go] (lines 60-64)
FilePreviewPanelClosedText string
[src/internal/common/predefined_variable.go] (lines 61-63)
FilePreviewImagePreviewDisabledText string
[src/internal/common/predefined_variable.go] (lines 62-66)
FilePreviewUnsupportedImageFormatsText string
[src/internal/common/predefined_variable.go] (lines 63-66)
FilePreviewImageConversionErrorText string
[src/internal/common/predefined_variable.go] (lines 55-65)
FilePreviewUnsupportedFormatText string
[src/internal/common/predefined_variable.go] (lines 64-65)
FilePreviewBatNotInstalledText string
[src/internal/utils/file_utils.go] (lines 255-278)
func ReadFileContent(filepath string, maxLineLength int, previewLine int) (string, error) {
var resultBuilder strings.Builder
file, err := os.Open(filepath)
if err != nil {
return resultBuilder.String(), err
}
defer file.Close()
reader := transform.NewReader(file, unicode.BOMOverride(unicode.UTF8.NewDecoder()))
scanner := bufio.NewScanner(reader)
lineCount := 0
for scanner.Scan() {
line := scanner.Text()
line = ansi.Truncate(line, maxLineLength, "")
resultBuilder.WriteString(line)
resultBuilder.WriteRune('\n')
lineCount++
if previewLine > 0 && lineCount >= previewLine {
break
}
}
// returns the first non-EOF error that was encountered by the [Scanner]
return resultBuilder.String(), scanner.Err()
}
[src/internal/common/string_function.go] (lines 162-176)
func IsTextFile(filename string) (bool, error) {
file, err := os.Open(filename)
if err != nil {
return false, err
}
defer file.Close()
reader := bufio.NewReader(file)
buffer := make([]byte, DefaultBufferSize)
cnt, err := reader.Read(buffer)
if err != nil && !errors.Is(err, io.EOF) {
return false, err
}
return IsBufferPrintable(buffer[:cnt]), nil
}
🧠 Learnings used
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 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 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: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:32:06.922Z
Learning: When reviewing large refactoring PRs that change async patterns (like moving from goroutines to tea.Cmd), always check for incomplete refactoring where some call sites still use the old pattern while others use the new pattern, as this often leads to compilation errors and architectural inconsistencies.
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.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 965
File: src/internal/config_function.go:8-8
Timestamp: 2025-07-29T03:42:36.581Z
Learning: When reviewing large refactoring PRs that introduce new async patterns (like moving from global channels to dedicated processbar channels), always check for incomplete refactoring where critical functionality like navigation depends on data structures that are no longer being populated, as this often leads to completely broken features.
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: 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: 1011
File: src/internal/handle_modal.go:145-203
Timestamp: 2025-08-29T13:56:33.955Z
Learning: lazysegtree identified that help menu navigation functions (helpMenuListUp and helpMenuListDown) in src/internal/handle_modal.go are too complicated, can be simplified, are similar to sidebar navigation but inconsistent, and lack unit tests. He prefers to track such technical debt in separate GitHub issues rather than addressing it in the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
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: 1227
File: src/internal/ui/filemodel/update.go:66-102
Timestamp: 2025-12-25T09:22:10.090Z
Learning: In yorukot/superfile, the preview panel's UpdatePreviewPanel should NOT validate by comparing msg.GetReqID() against m.ioReqCnt to filter stale responses. During fast scrolling, ioReqCnt increments rapidly, causing legitimate async preview responses to have reqID < ioReqCnt by the time they complete, which would prevent previews from ever displaying. The correct approach is location-based filtering: only apply the preview if selectedItem.Location still matches msg.GetLocation() when the response arrives.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/rendering/renderer_core.go:85-93
Timestamp: 2025-04-28T04:09:13.997Z
Learning: For the rendering package in superfile, truncation of content should be handled during the content addition phase through ContentRenderer rather than attempting to truncate after full rendering, as post-rendering truncation would break ANSI codes and border calculations.
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 (6)
src/internal/common/predefined_variable.gosrc/internal/common/style_function.gosrc/internal/ui/preview/render.gosrc/internal/ui/preview/render_utils.gosrc/internal/ui/rendering/renderer.gosrc/internal/ui/rendering/renderer_core.go
💤 Files with no reviewable changes (2)
- src/internal/ui/preview/render_utils.go
- src/internal/common/style_function.go
🧰 Additional context used
🧠 Learnings (8)
📓 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 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: 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: 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.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/rendering/renderer_core.go:85-93
Timestamp: 2025-04-28T04:09:13.997Z
Learning: For the rendering package in superfile, truncation of content should be handled during the content addition phase through ContentRenderer rather than attempting to truncate after full rendering, as post-rendering truncation would break ANSI codes and border calculations.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1227
File: src/internal/ui/filemodel/update.go:66-102
Timestamp: 2025-12-25T09:22:10.090Z
Learning: In yorukot/superfile, the preview panel's UpdatePreviewPanel should NOT validate by comparing msg.GetReqID() against m.ioReqCnt to filter stale responses. During fast scrolling, ioReqCnt increments rapidly, causing legitimate async preview responses to have reqID < ioReqCnt by the time they complete, which would prevent previews from ever displaying. The correct approach is location-based filtering: only apply the preview if selectedItem.Location still matches msg.GetLocation() when the response arrives.
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: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-05-22T04:45:44.934Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 825
File: src/internal/model_render.go:634-644
Timestamp: 2025-05-22T04:45:44.934Z
Learning: In the superfile codebase, the `r.AddLines(res)` method handles truncation internally. When a string is passed to the `AddLines` method of a renderer, it internally calls `AddLineWithCustomTruncate` which truncates the line using `TruncateBasedOnStyle` with the configured maximum line width. Therefore, there's no need to truncate strings before passing them to `AddLines`.
Applied to files:
src/internal/ui/rendering/renderer_core.go
📚 Learning: 2025-05-22T04:42:07.369Z
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.
Applied to files:
src/internal/ui/rendering/renderer.gosrc/internal/ui/preview/render.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/internal/common/predefined_variable.gosrc/internal/ui/preview/render.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/internal/common/predefined_variable.gosrc/internal/ui/preview/render.go
📚 Learning: 2025-06-04T09:58:25.572Z
Learnt from: yorukot
Repo: yorukot/superfile PR: 841
File: src/internal/model_render.go:334-334
Timestamp: 2025-06-04T09:58:25.572Z
Learning: Before suggesting the creation of new constants for string literals, always check if a constant already exists in the codebase, especially in common packages like `src/internal/common/predefined_variable.go`.
Applied to files:
src/internal/common/predefined_variable.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/internal/ui/preview/render.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/ui/preview/render.go
🧬 Code graph analysis (3)
src/internal/ui/rendering/renderer_core.go (1)
src/internal/ui/rendering/renderer.go (2)
Renderer(18-66)StyleModifier(12-12)
src/internal/common/predefined_variable.go (1)
src/config/icon/icon.go (2)
Error(37-37)Space(10-10)
src/internal/ui/preview/render.go (5)
src/internal/ui/preview/model.go (1)
Model(11-25)src/internal/common/predefined_variable.go (10)
FilePreviewPanelClosedText(60-60)FilePreviewImagePreviewDisabledText(61-61)FilePreviewUnsupportedImageFormatsText(62-62)FilePreviewImageConversionErrorText(63-63)FilePreviewError(59-59)FilePreviewUnsupportedFormatText(55-55)FilePreviewEmptyText(58-58)FilePreviewBatNotInstalledText(64-64)FilePreviewUnsupportedFileMode(56-56)FilePreviewThumbnailGenerationErrorText(65-65)src/config/icon/icon.go (1)
Error(37-37)src/internal/common/string_function.go (1)
IsTextFile(187-201)src/internal/utils/file_utils.go (1)
ReadFileContent(254-277)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (windows-latest)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
src/internal/ui/rendering/renderer.go (1)
12-13: LGTM! Well-designed extensibility pattern.The
StyleModifiertype provides a clean and flexible way to compose styles. The documentation clearly explains that modifiers are applied before other configuration-based styles (border, height, width), and that conflicting styles may be overridden. This design choice gives users fine-grained control while maintaining a predictable application order.Also applies to: 45-49
src/internal/ui/rendering/renderer_core.go (2)
52-55: LGTM! Clean fluent interface implementation.The
AddStyleModifiermethod correctly appends modifiers to the slice and returns*Rendererfor method chaining, maintaining consistency with other builder methods in the renderer.
137-146: LGTM! Correct application order.The style application order is implemented correctly:
- Apply all accumulated style modifiers (lines 139-141)
- Set Width (line 143)
- Set Height, Background, and Foreground (lines 144-146)
- Apply border if required (lines 148-152)
This matches the documented behavior that modifiers are applied before other style configurations.
src/internal/common/predefined_variable.go (2)
113-116: LGTM! Good refactoring to reduce duplication.The
wrapFilePreviewErrorMsghelper function centralizes the error message formatting pattern, improving maintainability. The comment clearly documents the dependency onInitIcon()being called first.
140-165: LGTM! Consistent error message formatting.All file preview error messages now use the centralized
wrapFilePreviewErrorMsghelper, ensuring consistent formatting across all error states. The new constants (FilePreviewPanelClosedText,FilePreviewImagePreviewDisabledText, etc.) provide clear, self-documenting names.src/internal/ui/preview/render.go (2)
86-94: Asymmetric clearCmd handling between Kitty and ANSI protocols.The Kitty protocol path (line 88) does not append
clearCmd, while the ANSI output path (line 94) does. This appears intentional, as Kitty protocol manages its own image state. However, the comment at lines 196-197 acknowledges potential issues with clearCmd handling in error cases.Consider whether error returns from
renderImagePreview(lines 65, 69, 76, 81) should behave differently when using Kitty protocol, or document why the current approach is correct.
61-63: LGTM! Renderer-based architecture improves consistency.The refactored method signatures now accept a
*rendering.Rendererparameter, enabling consistent rendering behavior across all preview types. The addition of theclearCmdparameter torenderImagePreviewproperly handles cleanup of terminal image state.Also applies to: 97-98
lazysegtree
force-pushed
the
fix-preview-border
branch
from
January 1, 2026 06:36
d55a3a8 to
4b781cc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/internal/utils/file_utils.go (1)
23-24: Remove the space before the colon in the comment for consistent punctuation style.The deprecation comment is accurate—
WriteTomlDatahas no callers andos.WriteAtis used directly infixTomlFileinstead. However, the comment formatting deviates from standard English punctuation.Formatting fix
-// Note : This is not used anymore as we use os.WriteAt to +// Note: This is not used anymore as we use os.WriteAt to
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/internal/common/string_function.gosrc/internal/common/style_function.gosrc/internal/utils/error.gosrc/internal/utils/file_utils.go
💤 Files with no reviewable changes (3)
- src/internal/utils/error.go
- src/internal/common/string_function.go
- src/internal/common/style_function.go
🧰 Additional context used
🧠 Learnings (3)
📓 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 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: 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: 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: 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: 1227
File: src/internal/ui/filemodel/update.go:66-102
Timestamp: 2025-12-25T09:22:10.090Z
Learning: In yorukot/superfile, the preview panel's UpdatePreviewPanel should NOT validate by comparing msg.GetReqID() against m.ioReqCnt to filter stale responses. During fast scrolling, ioReqCnt increments rapidly, causing legitimate async preview responses to have reqID < ioReqCnt by the time they complete, which would prevent previews from ever displaying. The correct approach is location-based filtering: only apply the preview if selectedItem.Location still matches msg.GetLocation() when the response arrives.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/rendering/renderer_core.go:85-93
Timestamp: 2025-04-28T04:09:13.997Z
Learning: For the rendering package in superfile, truncation of content should be handled during the content addition phase through ContentRenderer rather than attempting to truncate after full rendering, as post-rendering truncation would break ANSI codes and border calculations.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-06-27T02:36:23.492Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 881
File: src/internal/utils/file_utils.go:0-0
Timestamp: 2025-06-27T02:36:23.492Z
Learning: In superfile's LoadTomlFile function refactoring, the preferred approach is to return structured errors (TomlLoadError) rather than using PrintfAndExit for all error conditions. This allows callers to decide appropriate error handling: fatal errors cause immediate exit, while non-fatal errors (like missing fields when not fixing) only print warnings and continue.
Applied to files:
src/internal/utils/file_utils.go
📚 Learning: 2025-06-24T03:17:11.949Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 881
File: src/internal/utils/file_utils.go:104-104
Timestamp: 2025-06-24T03:17:11.949Z
Learning: In superfile's LoadTomlFile function, error handling should follow this pattern: use PrintfAndExit for critical errors that prevent application functionality (config corruption, file system failures during fixing), and use fmt.Println with errorPrefix for non-critical errors that users should see but can continue from (missing fields when not fixing, cleanup warnings).
Applied to files:
src/internal/utils/file_utils.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
This was referenced Jan 2, 2026
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Jan 17, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [yorukot/superfile](https://github.com/yorukot/superfile) | minor | `v1.4.0` → `v1.5.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>yorukot/superfile (yorukot/superfile)</summary> ### [`v1.5.0`](https://github.com/yorukot/superfile/releases/tag/v1.5.0) [Compare Source](yorukot/superfile@v1.4.0...v1.5.0) This release brings major new features, including video and PDF preview support, multi-column file panels, and configurable navigation, alongside significant code refactoring and comprehensive bug fixes. #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - Added video and PDF preview support via loading the first frame/page as an image. Thanks [@​Yassen-Higazi](https://github.com/Yassen-Higazi) for the implementation <details><summary>Screenshots</summary> <p> <img width="951" height="596" alt="image" src="https://github.com/user-attachments/assets/6edfa9c2-ebcd-4622-a115-f71fa533b3e1" /> <img width="949" height="594" alt="image" src="https://github.com/user-attachments/assets/8d15fa46-5178-422d-8eea-455cac31fdd0" /> </p> </details> - Multi-column file panel view with date/size/permission columns. Thanks [@​xelavopelk](https://github.com/xelavopelk) for the implementation <details><summary>Screenshots</summary> <p> <img width="420" height="264" alt="image" src="https://github.com/user-attachments/assets/e172f1e8-c2a5-42d2-8eeb-62e721f61a4f" /> </p> </details> - Configurable fast navigation in the Filepanel. See this MR for more details: [#​1220](yorukot/superfile#1220) - You can now configure `spf` to open files with specific extensions with your choice of editor application. Thanks [@​litvinov-git](https://github.com/litvinov-git) for the implementation See this MR for more details: [#​1197](yorukot/superfile#1197) - Terminal stdout support for shell commands - Allow launching with the filename. `spf /a/b/c.txt` will launch in `/a/b` with `c.txt` as the selected file. - Various bug fixes, including modal confirmations, layout issues, and race conditions. See 'Detailed Change Summary' ##### Internal Updates - Separated FilePanel and FileModel into a dedicated package for better code organization - Comprehensive end-to-end testing with layout fixes - Enhanced dimension validation and sidebar fixes - Updated multiple dependencies including Astro, Go toolchain, and linters - Added gosec linter and MND linter with magic number cleanup #### Detailed Change Summary <details><summary>Details</summary> <p> ##### Update - allow hover to file [`#1177`](yorukot/superfile#1177) by @​lazysegtree - show count selected items in select mode [`#1187`](yorukot/superfile#1187) by @​xelavopelk - Add icon alias for kts to kt [`#1153`](yorukot/superfile#1153) by @​nicolaic - link icon and metadata [`#1171`](yorukot/superfile#1171) by @​xelavopelk - user configuration of editors by file extension [`#1197`](yorukot/superfile#1197) by @​litvinov-git - add video preview support [`#1178`](yorukot/superfile#1178) by @​Yassen-Higazi - Add pdf preview support [`#1198`](yorukot/superfile#1198) by @​Yassen-Higazi - Add icons in pinned directories [`#1215`](yorukot/superfile#1215) by @​lazysegtree - Enable fast configurable navigation [`#1220`](yorukot/superfile#1220) by @​lazysegtree - add Trash bin to default directories for Linux [`#1236`](yorukot/superfile#1236) by @​lazysegtree - add terminal stdout support for shell commands [`#1250`](yorukot/superfile#1250) by @​majiayu000 - More columns in file panel (MVP) [`#1268`](yorukot/superfile#1268) by @​xelavopelk ##### Bug Fix - only calculate checksum on files [`#1119`](yorukot/superfile#1119) by @​nikero41 - Linter issue with PrintfAndExit [`#1133`](yorukot/superfile#1133) by @​xelavopelk - Remove repeated os.ReadDir calls [`#1155`](yorukot/superfile#1155) by @​lazysegtree - Disable COPYFILE in macOS [`#1194`](yorukot/superfile#1194) by @​lazysegtree - add missing hotkeys to help menu [`#1192`](yorukot/superfile#1192) by @​lazysegtree - Fetch latest version automatically [`#1127`](yorukot/superfile#1127) by @​lazysegtree - Use async methods to prevent test race conditions [`#1201`](yorukot/superfile#1201) by @​lazysegtree - update metadata and process bar sizes when toggling footer [`#1218`](yorukot/superfile#1218) by @​lazysegtree - File panel dimension management [`#1222`](yorukot/superfile#1222) by @​lazysegtree - Layout fixes with full end-to-end tests [`#1227`](yorukot/superfile#1227) by @​lazysegtree - Fix flaky tests [`#1233`](yorukot/superfile#1233) by @​lazysegtree - modal confirmation bug with arrow keys [`#1243`](yorukot/superfile#1243) by @​lazysegtree - small file panel optimization [`#1241`](yorukot/superfile#1241) by @​xelavopelk - use ExtractOperationMsg for extraction [`#1248`](yorukot/superfile#1248) by @​lazysegtree - skip open_with from missing field validation [`#1251`](yorukot/superfile#1251) by @​lazysegtree - border height validation fixes [`#1267`](yorukot/superfile#1267) by @​lazysegtree - fix case with two active panes [`#1271`](yorukot/superfile#1271) by @​xelavopelk - help model formatting [`#1277`](yorukot/superfile#1277) by @​booth-w ##### Optimization - simplify renameIfDuplicate logic [`#1100`](yorukot/superfile#1100) by @​sarff - separate FilePanel into dedicated package [`#1195`](yorukot/superfile#1195) by @​lazysegtree - File model separation [`#1223`](yorukot/superfile#1223) by @​lazysegtree - Dimension validations [`#1224`](yorukot/superfile#1224) by @​lazysegtree - layout validation and sidebar dimension fixes [`#1228`](yorukot/superfile#1228) by @​lazysegtree - user rendering package and removal of unused preview code [`#1245`](yorukot/superfile#1245) by @​lazysegtree - user rendering package for file preview [`#1249`](yorukot/superfile#1249) by @​lazysegtree ##### Documentation - update Fish shell setup docs [`#1142`](yorukot/superfile#1142) by @​wleoncio - fix macOS typo [`#1212`](yorukot/superfile#1212) by @​wcbing - stylistic and linguistic cleanup of config documentation [`#1184`](yorukot/superfile#1184) by @​ninetailedtori ##### Dependencies - update astro monorepo [`#1010`](yorukot/superfile#1010) by @​renovate[bot] - update starlight-giscus [`#1020`](yorukot/superfile#1020) by @​renovate[bot] - bump astro versions [`#1138`](yorukot/superfile#1138), [`#1157`](yorukot/superfile#1157), [`#1158`](yorukot/superfile#1158) by @​dependabot[bot], @​renovate[bot] - bump vite [`#1134`](yorukot/superfile#1134) by @​dependabot[bot] - update setup-go action [`#1038`](yorukot/superfile#1038) by @​renovate[bot] - update expressive-code plugins [`#1189`](yorukot/superfile#1189), [`#1246`](yorukot/superfile#1246) by @​renovate[bot] - update sharp [`#1256`](yorukot/superfile#1256) by @​renovate[bot] - update fontsource monorepo [`#1257`](yorukot/superfile#1257) by @​renovate[bot] - update urfave/cli [`#1136`](yorukot/superfile#1136), [`#1190`](yorukot/superfile#1190) by @​renovate[bot] - update astro / starlight / ansi / toolchain deps [`#1275`](yorukot/superfile#1275), [`#1278`](yorukot/superfile#1278), [`#1280`](yorukot/superfile#1280) by @​renovate[bot] - update python and go versions [`#1276`](yorukot/superfile#1276), [`#1191`](yorukot/superfile#1191) by @​renovate[bot] - update golangci-lint action [`#1286`](yorukot/superfile#1286) by @​renovate[bot] ##### Misc - update CI input names [`#1120`](yorukot/superfile#1120) by @​nikero41 - Everforest Dark Hard theme [`#1114`](yorukot/superfile#1114) by @​fzahner - migrate tutorial demo assets to local [`#1140`](yorukot/superfile#1140) by @​yorukot - new logo asset [`#1145`](yorukot/superfile#1145) by @​nonepork - mirror repository to codeberg [`#1141`](yorukot/superfile#1141) by @​yorukot - sync package lock [`#1143`](yorukot/superfile#1143) by @​yorukot - bump golangci-lint version [`#1135`](yorukot/superfile#1135) by @​lazysegtree - add gosec linter [`#1185`](yorukot/superfile#1185) by @​lazysegtree - enable MND linter and clean magic numbers [`#1180`](yorukot/superfile#1180) by @​lazysegtree - skip permission tests when running as root [`#1186`](yorukot/superfile#1186) by @​lazysegtree - release v1.4.1-rc [`#1203`](yorukot/superfile#1203) by @​lazysegtree - 1.5.0-rc1 housekeeping changes [`#1264`](yorukot/superfile#1264) by @​lazysegtree </p> </details> #### New Contributors * @​fzahner made their first contribution in yorukot/superfile#1114 * @​sarff made their first contribution in yorukot/superfile#1100 * @​nicolaic made their first contribution in yorukot/superfile#1153 * @​Yassen-Higazi made their first contribution in yorukot/superfile#1178 * @​ninetailedtori made their first contribution in yorukot/superfile#1184 * @​litvinov-git made their first contribution in yorukot/superfile#1197 * @​wcbing made their first contribution in yorukot/superfile#1212 * @​majiayu000 made their first contribution in yorukot/superfile#1250 **Full Changelog**: <yorukot/superfile@v1.4.0...v1.5.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi44MC4xIiwidXBkYXRlZEluVmVyIjoiNDIuODAuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6Om1pbm9yIl19-->
This was referenced Jul 1, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.