feat: Bug fix, navigation changes and refactoring#1219
Conversation
WalkthroughExtracts and exposes a FileModel package, refactors per-panel internals (dimensions, navigation, rendering), centralizes focused-panel access and dimension propagation, adds a PageScrollSize config and a clipboard model, and adds comprehensive render/geometry validation. Multiple handlers and tests updated to use the new exported APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App (main model)
participant FM as filemodel.Model
participant FP as filepanel.Model
participant PV as preview.Model
participant CB as clipboard.Model
Note right of App: User action (navigate / create / render)
App->>FM: CreateNewFilePanel(location) / NextFilePanel()
FM->>FM: validate & mutate FilePanels, FocusedPanelIndex
FM->>FP: filepanel.New(...) (create or update panel)
FM->>FM: UpdateChildComponentWidth/Height
FM-->>App: updated dimensions / resizeCmd
Note right of App: Handlers use focused panel
App->>FM: GetFocusedFilePanel()
FM-->>FP: *filepanel pointer
App->>FP: ListDown / PgDown / SingleItemSelect
FP->>FP: moveCursorBy -> scrollToCursor -> adjust RenderIndex
FP-->>App: fragment render
alt FilePreview open
FM->>PV: SetWidth/SetHeight / RenderWithPath()
PV-->>FM: preview content
end
App->>CB: Clipboard queries (IsCut/GetItems) in paste/copy handlers
CB-->>App: clipboard state
FM->>App: Render() composes FP.Render + PV.Render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: |
bbf7940
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fa304719.superfile.pages.dev |
| Branch Preview URL: | https://develop.superfile.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/internal/ui/filepanel/get_elements.go (1)
79-94: Consider making the 3-second throttle configurable.The throttling logic is sound, but the hardcoded 3-second delay for non-focused panels (line 82) could be made configurable for users who want more responsive updates or less frequent refreshes.
Potential enhancement
Consider adding a configuration option like
NonFocusedPanelUpdateIntervalto make this user-configurable, though this is fine as-is for now.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
filepanel_dimension_fix.mdphase_1_detail.mdsrc/internal/key_function.gosrc/internal/model.gosrc/internal/model_render.gosrc/internal/ui/filepanel/consts.gosrc/internal/ui/filepanel/dimension.gosrc/internal/ui/filepanel/get_elements.gosrc/internal/ui/filepanel/get_elements_test.gosrc/internal/ui/filepanel/misc.gosrc/internal/ui/filepanel/model.gosrc/internal/ui/filepanel/model_utils.gosrc/internal/ui/filepanel/navigation.gosrc/internal/ui/filepanel/navigation_test.gosrc/internal/ui/filepanel/render.gosrc/internal/ui/filepanel/sort.gosrc/internal/ui/filepanel/types.gosrc/internal/ui/filepanel/update.gosrc/internal/ui/filepanel/utils.gosrc/internal/wheel_function.go
💤 Files with no reviewable changes (1)
- src/internal/ui/filepanel/model_utils.go
🚧 Files skipped from review as they are similar to previous changes (1)
- src/internal/ui/filepanel/navigation.go
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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, 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: 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 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 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:25:06.352Z
Learning: PR #770 "Sidebar code separation" is a refactoring and code reorganization effort with no new features. It involves moving code, updating import paths, renaming functions to follow PascalCase convention, and consolidating sidebar-related functionality into a dedicated package.
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-08-29T13:56:33.955Z
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.
Applied to files:
src/internal/wheel_function.gosrc/internal/ui/filepanel/get_elements_test.gosrc/internal/key_function.gosrc/internal/ui/filepanel/navigation_test.go
📚 Learning: 2025-04-12T13:51:24.691Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/render.go:0-0
Timestamp: 2025-04-12T13:51:24.691Z
Learning: In the sidebar component of yorukot/superfile, the partial list rendering in render.go is intentional. The sidebar implements scrolling functionality where ListUp/ListDown methods in navigation.go update cursor position and renderIndex to show the appropriate portion of the directory list based on available space.
Applied to files:
src/internal/wheel_function.gosrc/internal/key_function.gosrc/internal/model_render.gosrc/internal/ui/filepanel/get_elements.gosrc/internal/ui/filepanel/consts.gosrc/internal/ui/filepanel/render.go
📚 Learning: 2025-07-27T15:35:25.617Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:35:25.617Z
Learning: In yorukot/superfile, deleteItemWarn() only displays a warning modal via the channelMessage system and does not perform actual file deletion. The actual deletion happens when the user confirms the modal, which triggers getDeleteCmd() that returns a tea.Cmd. The architecture intentionally separates UI modal operations (using channelMessage/goroutines) from file I/O operations (using tea.Cmd pattern), creating clean separation of concerns between UI state management and file operations.
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-09-06T13:42:44.590Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1039
File: src/internal/ui/zoxide/model.go:53-54
Timestamp: 2025-09-06T13:42:44.590Z
Learning: The zoxide modal in src/internal/ui/zoxide/model.go is missing handling for common.Hotkeys.Quit when zClient is nil (lines 52-54), only handling ConfirmTyping and CancelTyping. This creates inconsistency with other modals like sort options menu, help menu, and notify model which all properly handle the Quit hotkey. The prompt modal has the same inconsistency.
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-08-03T15:07:56.341Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model.go:118-120
Timestamp: 2025-08-03T15:07:56.341Z
Learning: In the processbar package (src/internal/ui/processbar/model.go), the Progress.Width field is modified temporarily during rendering on a copy of the process data, and this modification is intentionally not persisted to the stored process data since it's only needed for display purposes during the render call.
Applied to files:
src/internal/model_render.gosrc/internal/ui/filepanel/dimension.gosrc/internal/ui/filepanel/types.gosrc/internal/ui/filepanel/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/model_render.gosrc/internal/model.gosrc/internal/ui/filepanel/render.go
📚 Learning: 2025-08-05T11:51:37.645Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:44-44
Timestamp: 2025-08-05T11:51:37.645Z
Learning: In Bubble Tea applications, when functions like warnModalForRenaming() capture state (like m.ioReqCnt) and increment it before returning a tea.Cmd, the increment happens synchronously in the calling context (typically Update()), not inside the returned tea.Cmd function. Since Update() is called synchronously by the Bubble Tea runtime, there are no race conditions in this pattern.
Applied to files:
src/internal/model.go
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/internal/ui/filepanel/update.gosrc/internal/ui/filepanel/get_elements.go
📚 Learning: 2025-03-29T13:20:46.467Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 730
File: src/internal/function.go:211-211
Timestamp: 2025-03-29T13:20:46.467Z
Learning: Empty panel.element checks should be added at the beginning of functions that access panel.element to prevent crashes when a panel has no elements, similar to using `if len(panel.element) == 0 { return }`.
Applied to files:
src/internal/ui/filepanel/sort.gofilepanel_dimension_fix.md
📚 Learning: 2025-04-12T14:02:46.575Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:16-16
Timestamp: 2025-04-12T14:02:46.575Z
Learning: In Go packages, constants and variables should only be exported (capitalized) if they need to be accessed from outside their package. In the sidebar package, constants like `sideBarInitialHeight` should remain unexported as they're only used within the package.
Applied to files:
src/internal/ui/filepanel/consts.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/filepanel/render.go
📚 Learning: 2025-03-29T10:02:04.260Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 730
File: src/internal/model_render.go:653-654
Timestamp: 2025-03-29T10:02:04.260Z
Learning: A panel validation function should be implemented to ensure panel.cursor doesn't become negative or go out of bounds, but this should be addressed in a separate PR - not as part of the PR fixing crashes when opening files with the editor on an empty panel.
Applied to files:
filepanel_dimension_fix.md
🧬 Code graph analysis (12)
src/internal/ui/filepanel/misc.go (2)
src/internal/ui/filepanel/types.go (3)
PanelMode(67-67)SelectMode(71-71)BrowserMode(72-72)src/internal/common/predefined_variable.go (1)
InvalidTypeString(15-15)
src/internal/ui/filepanel/get_elements_test.go (1)
src/internal/ui/filepanel/types.go (2)
Model(14-34)Element(58-64)
src/internal/ui/filepanel/utils.go (1)
src/internal/ui/filepanel/types.go (2)
Model(14-34)Element(58-64)
src/internal/model.go (4)
src/internal/ui/processbar/process_update_msg.go (1)
Cmd(3-3)src/internal/common/ui_consts.go (7)
HeightBreakA(20-20)HeightBreakB(21-21)HeightBreakC(22-22)HeightBreakD(23-23)BorderPadding(11-11)InnerPadding(12-12)FilePanelWidthUnit(27-27)src/internal/utils/ui_utils.go (1)
FullFooterHeight(17-22)src/internal/common/default_config.go (1)
Config(11-11)
src/internal/ui/filepanel/update.go (2)
src/internal/ui/filepanel/types.go (4)
Model(14-34)PanelMode(67-67)SelectMode(71-71)BrowserMode(72-72)src/internal/utils/file_utils.go (1)
ResolveAbsPath(190-203)
src/internal/ui/filepanel/dimension.go (3)
src/internal/ui/filepanel/types.go (1)
Model(14-34)src/internal/ui/filepanel/consts.go (3)
FilePanelMinWidth(7-7)FilePanelMinHeight(8-8)FilePanelContentPadding(6-6)src/internal/common/ui_consts.go (2)
InnerPadding(12-12)BorderPadding(11-11)
src/internal/ui/filepanel/get_elements.go (3)
src/internal/ui/filepanel/types.go (2)
Model(14-34)Element(58-64)src/internal/utils/fzf_utils.go (1)
FzfSearch(6-14)src/internal/common/ui_consts.go (1)
ReRenderChunkDivisor(25-25)
src/internal/ui/filepanel/sort.go (2)
src/internal/ui/filepanel/types.go (1)
Model(14-34)src/internal/common/default_config.go (1)
Config(11-11)
src/internal/ui/filepanel/navigation_test.go (1)
src/internal/ui/filepanel/types.go (3)
Model(14-34)Element(58-64)PanelMode(67-67)
src/internal/ui/filepanel/consts.go (1)
src/internal/common/ui_consts.go (1)
BorderPadding(11-11)
src/internal/ui/filepanel/model.go (3)
src/internal/ui/filepanel/types.go (2)
Model(14-34)PanelMode(67-67)src/internal/ui/filepanel/consts.go (4)
SortOptionsDefaultWidth(10-10)SortOptionsDefaultHeight(11-11)FilePanelMinWidth(7-7)FilePanelMinHeight(8-8)src/internal/common/style_function.go (1)
GenerateSearchBar(237-249)
src/internal/ui/filepanel/render.go (5)
src/internal/ui/filepanel/types.go (2)
Model(14-34)Element(58-64)src/internal/ui/spf_renderers.go (1)
FilePanelRenderer(34-54)src/internal/ui/rendering/renderer.go (1)
Renderer(14-54)src/internal/common/string_function.go (1)
TruncateTextBeginning(39-58)src/internal/common/ui_consts.go (1)
InnerPadding(12-12)
🪛 GitHub Actions: Go CI
src/internal/ui/filepanel/consts.go
[error] 3-12: Go fmt formatting changes detected. Run 'go fmt ./...' to format the code. Command 'go fmt ./...' failed (exit code 1) due to formatting differences.
🪛 LanguageTool
filepanel_dimension_fix.md
[grammar] ~8-~8: Ensure spelling is correct
Context: ...min - Replace all functions to not take hight and width arguements. - Ex: `scrollTo...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~12-~12: Ensure spelling is correct
Context: ...)inscrollToCursorshould change torenderCount := m.panelElementHeight()` - Similiar changes in all places to not depend on ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
phase_1_detail.md
12-12: Hard tabs
Column: 1
(MD010, no-hard-tabs)
13-13: Hard tabs
Column: 1
(MD010, no-hard-tabs)
14-14: Hard tabs
Column: 1
(MD010, no-hard-tabs)
15-15: Hard tabs
Column: 1
(MD010, no-hard-tabs)
16-16: Hard tabs
Column: 1
(MD010, no-hard-tabs)
23-23: Hard tabs
Column: 1
(MD010, no-hard-tabs)
24-24: Hard tabs
Column: 1
(MD010, no-hard-tabs)
25-25: Hard tabs
Column: 1
(MD010, no-hard-tabs)
26-26: Hard tabs
Column: 1
(MD010, no-hard-tabs)
27-27: Hard tabs
Column: 1
(MD010, no-hard-tabs)
28-28: Hard tabs
Column: 1
(MD010, no-hard-tabs)
29-29: Hard tabs
Column: 1
(MD010, no-hard-tabs)
30-30: Hard tabs
Column: 1
(MD010, no-hard-tabs)
56-56: Hard tabs
Column: 1
(MD010, no-hard-tabs)
57-57: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1
(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1
(MD010, no-hard-tabs)
61-61: Hard tabs
Column: 1
(MD010, no-hard-tabs)
62-62: Hard tabs
Column: 1
(MD010, no-hard-tabs)
63-63: Hard tabs
Column: 1
(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1
(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1
(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1
(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1
(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1
(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1
(MD010, no-hard-tabs)
105-105: Hard tabs
Column: 1
(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1
(MD010, no-hard-tabs)
107-107: Hard tabs
Column: 1
(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1
(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1
(MD010, no-hard-tabs)
120-120: Hard tabs
Column: 1
(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1
(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1
(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1
(MD010, no-hard-tabs)
130-130: Hard tabs
Column: 1
(MD010, no-hard-tabs)
131-131: Hard tabs
Column: 1
(MD010, no-hard-tabs)
132-132: Hard tabs
Column: 1
(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1
(MD010, no-hard-tabs)
134-134: Hard tabs
Column: 1
(MD010, no-hard-tabs)
147-147: Hard tabs
Column: 1
(MD010, no-hard-tabs)
149-149: Hard tabs
Column: 1
(MD010, no-hard-tabs)
150-150: Hard tabs
Column: 1
(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 1
(MD010, no-hard-tabs)
152-152: Hard tabs
Column: 1
(MD010, no-hard-tabs)
154-154: Hard tabs
Column: 1
(MD010, no-hard-tabs)
161-161: Hard tabs
Column: 1
(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1
(MD010, no-hard-tabs)
164-164: Hard tabs
Column: 1
(MD010, no-hard-tabs)
165-165: Hard tabs
Column: 1
(MD010, no-hard-tabs)
166-166: Hard tabs
Column: 1
(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1
(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 1
(MD010, no-hard-tabs)
201-201: Hard tabs
Column: 1
(MD010, no-hard-tabs)
217-217: Hard tabs
Column: 1
(MD010, no-hard-tabs)
218-218: Hard tabs
Column: 1
(MD010, no-hard-tabs)
223-223: Hard tabs
Column: 1
(MD010, no-hard-tabs)
224-224: Hard tabs
Column: 1
(MD010, no-hard-tabs)
232-232: Hard tabs
Column: 1
(MD010, no-hard-tabs)
233-233: Hard tabs
Column: 1
(MD010, no-hard-tabs)
238-238: Hard tabs
Column: 1
(MD010, no-hard-tabs)
239-239: Hard tabs
Column: 1
(MD010, no-hard-tabs)
295-295: Hard tabs
Column: 1
(MD010, no-hard-tabs)
296-296: Hard tabs
Column: 1
(MD010, no-hard-tabs)
300-300: Hard tabs
Column: 1
(MD010, no-hard-tabs)
301-301: Hard tabs
Column: 1
(MD010, no-hard-tabs)
308-308: Hard tabs
Column: 1
(MD010, no-hard-tabs)
309-309: Hard tabs
Column: 1
(MD010, no-hard-tabs)
313-313: Hard tabs
Column: 1
(MD010, no-hard-tabs)
314-314: Hard tabs
Column: 1
(MD010, no-hard-tabs)
331-331: Hard tabs
Column: 1
(MD010, no-hard-tabs)
filepanel_dimension_fix.md
11-11: Spaces inside code span elements
(MD038, no-space-in-code)
⏰ 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: Cloudflare Pages
- GitHub Check: Analyze (go)
🔇 Additional comments (37)
src/internal/ui/filepanel/types.go (1)
19-21: LGTM! Clean encapsulation of dimension fields.The addition of unexported
widthandheightfields to the Model struct properly encapsulates dimension management. The comments clearly indicate these include borders, which helps prevent confusion about their semantics.src/internal/wheel_function.go (1)
22-22: LGTM! Navigation calls updated consistently.The wheel navigation for file panels now uses the parameter-free
ListUp()andListDown()methods, correctly aligning with the dimension encapsulation refactor. The sidebar and process bar navigation still pass height parameters as expected since they haven't been refactored yet.Also applies to: 34-34
src/internal/ui/filepanel/misc.go (1)
5-14: LGTM! Clean String() implementation for PanelMode.The String() method provides a clear string representation for the PanelMode enum, with proper handling of invalid values by returning
common.InvalidTypeString. This follows Go conventions for providing string representations of custom types.src/internal/ui/filepanel/get_elements_test.go (1)
207-276: LGTM! Excellent test coverage for SingleItemSelect.The test comprehensively covers important edge cases including:
- Toggle behavior (select/deselect)
- Out-of-bounds cursor values (negative and beyond array length)
- Empty element list
The table-driven test structure makes it easy to understand and maintain.
src/internal/key_function.go (3)
34-34: LGTM! List navigation updated consistently.The
ListUp()andListDown()calls for file panels now correctly use the parameter-free API, relying on internal dimension management.Also applies to: 47-47
51-51: LGTM! Page navigation updated correctly.The
PgUp()andPgDown()calls now use the updated API without themainPanelHeightparameter, consistent with the dimension encapsulation refactor.Also applies to: 54-54
154-154: LGTM! Item selection navigation updated correctly.The
ItemSelectUp()andItemSelectDown()calls in select mode now correctly use the parameter-free API, consistent with the refactored navigation methods.Also applies to: 156-156
src/internal/ui/filepanel/sort.go (1)
140-147: LGTM! Clean page scroll size calculation with proper fallback.The
getPageScrollSize()method correctly:
- Uses the configured
PageScrollSizewhen positive- Falls back to
PanelElementHeight()for default behavior- Handles both zero and negative values with the
<= 0checkThis provides a good balance between configurability and sensible defaults.
src/internal/model_render.go (1)
58-60: LGTM! Clean dimension management refactor.The changes successfully encapsulate dimension handling within the file panel model. Calling
UpdateDimensionsbeforeRenderensures the panel has current dimensions, and the simplifiedRender(focussed bool)signature is much cleaner than passing dimensions explicitly.src/internal/ui/filepanel/model.go (1)
11-61: LGTM! Well-structured model constructors.The constructor functions are cleanly organized with good separation of concerns:
FilePanelSlicefor batch creationdefaultFilePanelfor default configurationNewfor the base constructorThe file vs directory detection (lines 26-29) correctly extracts the target filename when a file path is provided, and the default sort options properly initialize from
common.Config.src/internal/ui/filepanel/get_elements.go (3)
18-36: LGTM! Clean directory element retrieval.The function correctly reads the directory, filters entries based on errors and dotfile visibility, and returns sorted elements.
39-76: LGTM! FZF search integration is correct.The search-based element retrieval correctly builds a map, performs FZF search, and reconstructs the sorted element list. The blocking FZF call concern is already documented in the existing TODO (line 68) and in the utils function itself.
96-109: LGTM! Well-structured conditional update logic.The function correctly checks throttling, updates elements, records the timestamp, and applies target file cursor positioning.
src/internal/ui/filepanel/update.go (2)
11-21: LGTM! Correct mode switching logic.The function properly toggles between panel modes and clears the selection when exiting SelectMode. Error logging for unexpected modes is appropriate.
24-70: LGTM! Solid directory navigation with known cache limitation.The implementation correctly:
- Resolves paths to absolute form
- Avoids redundant updates
- Caches cursor/render state per directory
- Validates the destination is an existing directory
- Resets the search bar
The TODO (lines 50-53) correctly identifies that the DirectoryRecords cache can become invalid if files are deleted externally. This is acceptable for now as noted in the comment.
src/internal/ui/filepanel/navigation_test.go (7)
11-18: LGTM! Clean test helper.The
testModelhelper provides a simple, focused way to construct test fixtures for navigation testing.
20-97: LGTM! Comprehensive up/down navigation tests.Excellent coverage of:
- Movement within renderable range
- Scrolling behavior at boundaries
- Wrap-around at top and bottom
- Empty panel handling
99-195: LGTM! Thorough page navigation tests.The test cases cover all critical scenarios:
- Full page scrolling
- Wrap-around at boundaries
- Small element counts
- Empty panels
197-331: LGTM! Excellent selection interaction tests.Comprehensive coverage of:
- Select/deselect behavior
- Movement with selection
- Wrap-around with selection
- Selection persistence
- Empty panel handling
333-424: LGTM! Solid scroll-to-cursor tests.Tests correctly validate:
- No-op when cursor already visible
- Scrolling when jumping above/below view
- Boundary conditions
- Invalid cursor handling
426-453: LGTM! Target file cursor test is thorough.Validates that
applyTargetFileCursorcorrectly positions the cursor and is idempotent on subsequent calls.
455-521: LGTM! Excellent config-driven paging tests.The tests properly:
- Back up and restore the original configuration
- Validate default behavior (PageScrollSize = 0)
- Validate custom scroll sizes
- Cover both PgUp and PgDown
This demonstrates good integration testing of the configurable page scroll feature.
src/internal/ui/filepanel/utils.go (3)
7-24: LGTM! Solid utility methods with proper bounds checking.All three methods (
GetSelectedItem,ResetSelected,GetSelectedItemPtr) correctly handle boundary conditions and invalid cursor positions.
27-41: LGTM! Correct toggle logic with known performance limitation.The selection toggle correctly handles adding/removing items with proper bounds checking. The TODO (lines 32-35) accurately identifies the O(n) performance issue when the
Selectedslice grows large. Converting tomap[string]struct{}would make this O(1), but the current implementation is acceptable for typical use cases.
43-49: LGTM! Simple, clear helper methods.Both
ElemCountandEmptyare straightforward and correct.src/internal/ui/filepanel/render.go (3)
17-26: LGTM! Clean signature simplification.The new
Render(focussed bool)signature is much cleaner than the previous version. The rendering now correctly derives dimensions from the model's internal state (m.height,m.width), eliminating the need to thread these values through the call chain.
28-33: LGTM! Top bar rendering uses internal dimensions.The simplified signature and use of
m.GetContentWidth()for truncation width calculation is correct and cleaner than passingfilePanelWidthas a parameter.
64-104: LGTM! File entries rendering correctly uses model dimensions.The rendering logic now properly derives:
- End index from
m.PanelElementHeight()(line 70)- Name width from
m.GetContentWidth()(line 95)This is consistent with the dimension encapsulation refactor and maintains correct rendering behavior.
src/internal/model.go (5)
63-64: LGTM - Clean command-based refactoring.The introduction of
resizeCmdand its integration into the command flow properly separates resize effects from inline state changes, aligning well with Bubble Tea's architecture.Also applies to: 78-78, 112-113
209-236: Good refactoring with acknowledged tech debt.The separation of height calculations into
setHeightValuesimproves clarity. The footer height magic numbers (6-10) are noted for future improvement via thenolintcomment - this is acceptable given the focus on dimension management refactoring in this PR.
238-258: Well-structured dimension update flow.The method properly centralizes component sizing and conditionally triggers file preview refresh when the preview panel is open. The
forcePreviewRenderflag ensures content is re-rendered with the new dimensions.
275-284: LGTM - Dimension delegation to file panels.The refactoring properly delegates dimension management to individual file panels via
UpdateDimensions, maintaining separation of concerns.
699-701: Signature verified as consistent.The
UpdateElementsIfNeededcall matches the function definition insrc/internal/ui/filepanel/get_elements.go:96with parameters in the correct order:focusPanelReRender,toggleDotFile, andupdatedToggleDotFile.src/internal/ui/filepanel/dimension.go (4)
1-8: LGTM - Clean dimension management API.The
UpdateDimensionsmethod provides a convenient atomic update for both width and height.
10-16: Proper width clamping and SearchBar sync.The minimum width enforcement and SearchBar width synchronization ensure consistent rendering regardless of terminal size.
27-46: Clear dimension accessors with proper calculations.The hierarchy of dimension accessors is well-structured:
GetWidth/GetHeight→ total dimensions including bordersGetMainPanelHeight/GetContentWidth→ inner dimensions minus border paddingPanelElementHeight→ usable height for file list elementsThis encapsulates the dimension math that was previously scattered throughout the codebase.
18-25: Good scroll adjustment on height change.Calling
scrollToCursorafter height changes ensures the cursor remains visible when the panel shrinks. This prevents the cursor from being off-screen after terminal resize.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/internal/type.go (1)
130-130: Consider removing orphaned comment.The
/* FILE WINDOWS TYPE END*/comment appears to be a leftover from the removed/* FILE WINDOWS TYPE START*/block that previously defined the internalfileModelstruct.🔎 Suggested fix
-/* FILE WINDOWS TYPE END*/ - type editorFinishedMsg struct{ err error }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
src/internal/common/ui_consts.gosrc/internal/default_config.gosrc/internal/function.gosrc/internal/handle_file_operations.gosrc/internal/handle_modal.gosrc/internal/handle_panel_movement.gosrc/internal/handle_panel_navigation.gosrc/internal/key_function.gosrc/internal/model.gosrc/internal/model_msg.gosrc/internal/model_prompt_test.gosrc/internal/model_render.gosrc/internal/model_render_test.gosrc/internal/model_render_unix_test.gosrc/internal/model_test.gosrc/internal/type.gosrc/internal/type_utils.gosrc/internal/ui/filemodel/consts.gosrc/internal/ui/filemodel/dimensions.gosrc/internal/ui/filemodel/navigation.gosrc/internal/ui/filemodel/render.gosrc/internal/ui/filemodel/type.gosrc/internal/ui/filemodel/update.gosrc/internal/ui/filemodel/utils.gosrc/internal/ui/filepanel/consts.gosrc/internal/ui/filepanel/dimension.gosrc/internal/ui/filepanel/model.gosrc/internal/validation.gosrc/internal/wheel_function.gosrc/superfile_config/config.toml
💤 Files with no reviewable changes (3)
- src/internal/function.go
- src/internal/common/ui_consts.go
- src/internal/type_utils.go
🚧 Files skipped from review as they are similar to previous changes (2)
- src/internal/wheel_function.go
- src/internal/ui/filepanel/model.go
🧰 Additional context used
🧠 Learnings (36)
📓 Common learnings
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: 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 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, 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: 0
File: :0-0
Timestamp: 2025-04-12T12:25:06.352Z
Learning: PR #770 "Sidebar code separation" is a refactoring and code reorganization effort with no new features. It involves moving code, updating import paths, renaming functions to follow PascalCase convention, and consolidating sidebar-related functionality into a dedicated package.
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: 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/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: 730
File: src/internal/model_render.go:653-654
Timestamp: 2025-03-29T10:02:04.260Z
Learning: A panel validation function should be implemented to ensure panel.cursor doesn't become negative or go out of bounds, but this should be addressed in a separate PR - not as part of the PR fixing crashes when opening files with the editor on an empty panel.
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-04-12T14:02:46.575Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:16-16
Timestamp: 2025-04-12T14:02:46.575Z
Learning: In Go packages, constants and variables should only be exported (capitalized) if they need to be accessed from outside their package. In the sidebar package, constants like `sideBarInitialHeight` should remain unexported as they're only used within the package.
Applied to files:
src/internal/ui/filemodel/consts.gosrc/internal/ui/filepanel/consts.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/ui/filemodel/render.gosrc/internal/model_test.gosrc/internal/model_render_test.gosrc/internal/default_config.gosrc/internal/handle_panel_navigation.gosrc/internal/model_render_unix_test.gosrc/internal/type.gosrc/internal/handle_file_operations.gosrc/internal/handle_modal.gosrc/internal/model_prompt_test.gosrc/internal/model_msg.gosrc/internal/model.gosrc/internal/model_render.go
📚 Learning: 2025-08-29T13:56:33.955Z
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.
Applied to files:
src/internal/key_function.gosrc/internal/ui/filemodel/navigation.gosrc/internal/handle_panel_movement.gosrc/internal/ui/filepanel/consts.gosrc/internal/handle_panel_navigation.go
📚 Learning: 2025-09-12T05:16:10.488Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/handle_modal.go:253-253
Timestamp: 2025-09-12T05:16:10.488Z
Learning: lazysegtree identified that the fuzzy search function in src/internal/handle_modal.go for the help menu only searches on item.description but should also include item.key in the search haystack to provide comprehensive search functionality, as users expect to find hotkeys by searching for their key names.
Applied to files:
src/internal/key_function.gosrc/internal/handle_panel_movement.go
📚 Learning: 2025-11-03T01:53:26.636Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1113
File: src/superfile_config/vimHotkeys.toml:66-67
Timestamp: 2025-11-03T01:53:26.636Z
Learning: In yorukot/superfile, modal-specific hotkeys (like nav_bulk_rename and rev_nav_bulk_rename using tab/shift+tab in the bulk rename modal) can safely conflict with global hotkey bindings (like next_file_panel and previous_file_panel) because the modal captures input when focused, and the global bindings remain active when the modal is closed. This context-specific key handling is intentional and not considered a conflict.
Applied to files:
src/internal/key_function.gosrc/internal/handle_modal.go
📚 Learning: 2025-09-05T08:04:14.324Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1040
File: src/internal/function.go:58-58
Timestamp: 2025-09-05T08:04:14.324Z
Learning: lazysegtree asked about "dual-panel functionality" and "secondary focus" in the superfile codebase. The system actually supports multiple file panels (not specifically dual-panel), where users can create/close panels and navigate between them. The old `secondFocus` state was used to mark non-active panels when multiple panels were open, not for a specific dual-panel feature. The focus states were: noneFocus (no file panel focused), focus (active panel), and secondFocus (inactive panels).
Applied to files:
src/internal/key_function.gosrc/internal/handle_panel_navigation.gosrc/internal/handle_file_operations.gosrc/internal/handle_modal.go
📚 Learning: 2025-04-12T13:51:24.691Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/render.go:0-0
Timestamp: 2025-04-12T13:51:24.691Z
Learning: In the sidebar component of yorukot/superfile, the partial list rendering in render.go is intentional. The sidebar implements scrolling functionality where ListUp/ListDown methods in navigation.go update cursor position and renderIndex to show the appropriate portion of the directory list based on available space.
Applied to files:
src/internal/key_function.gosrc/internal/validation.gosrc/internal/ui/filepanel/consts.gosrc/internal/handle_panel_navigation.gosrc/internal/model.gosrc/internal/model_render.go
📚 Learning: 2025-04-12T12:00:32.688Z
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.
Applied to files:
src/internal/key_function.gosrc/internal/handle_panel_movement.gosrc/internal/ui/filepanel/consts.gosrc/internal/handle_panel_navigation.gosrc/internal/handle_file_operations.go
📚 Learning: 2025-09-06T13:42:44.590Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1039
File: src/internal/ui/zoxide/model.go:53-54
Timestamp: 2025-09-06T13:42:44.590Z
Learning: The zoxide modal in src/internal/ui/zoxide/model.go is missing handling for common.Hotkeys.Quit when zClient is nil (lines 52-54), only handling ConfirmTyping and CancelTyping. This creates inconsistency with other modals like sort options menu, help menu, and notify model which all properly handle the Quit hotkey. The prompt modal has the same inconsistency.
Applied to files:
src/internal/key_function.gosrc/internal/model_test.gosrc/internal/type.gosrc/internal/model_prompt_test.go
📚 Learning: 2025-07-27T15:35:25.617Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:35:25.617Z
Learning: In yorukot/superfile, deleteItemWarn() only displays a warning modal via the channelMessage system and does not perform actual file deletion. The actual deletion happens when the user confirms the modal, which triggers getDeleteCmd() that returns a tea.Cmd. The architecture intentionally separates UI modal operations (using channelMessage/goroutines) from file I/O operations (using tea.Cmd pattern), creating clean separation of concerns between UI state management and file operations.
Applied to files:
src/internal/key_function.gosrc/internal/type.gosrc/internal/model_prompt_test.go
📚 Learning: 2025-07-27T15:35:25.617Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:35:25.617Z
Learning: deleteItemWarn() in yorukot/superfile only displays a warning modal via channel messaging and does not perform actual file deletion. The actual deletion happens through getDeleteCmd() which returns a tea.Cmd. The architectural pattern separates UI modal operations (using channels/goroutines) from file I/O operations (using tea.Cmd), which is intentional and appropriate.
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-08-03T14:49:03.094Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_test.go:64-64
Timestamp: 2025-08-03T14:49:03.094Z
Learning: In Go, unexported fields (lowercase names) in structs are accessible within the same package, so test files in the same package as the struct definitions can access and test these unexported fields directly without needing them to be exported.
Applied to files:
src/internal/model_test.gosrc/internal/model_render_test.gosrc/internal/model_render_unix_test.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/validation.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/model_render_test.gosrc/internal/ui/filepanel/consts.gosrc/internal/model_render_unix_test.gosrc/internal/handle_modal.gosrc/internal/model_msg.gosrc/internal/model.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/model_render_test.gosrc/internal/default_config.gosrc/internal/model_render_unix_test.gosrc/internal/model_msg.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/internal/model_render_test.gosrc/internal/model_render_unix_test.go
📚 Learning: 2025-08-03T15:07:56.341Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model.go:118-120
Timestamp: 2025-08-03T15:07:56.341Z
Learning: In the processbar package (src/internal/ui/processbar/model.go), the Progress.Width field is modified temporarily during rendering on a copy of the process data, and this modification is intentionally not persisted to the stored process data since it's only needed for display purposes during the render call.
Applied to files:
src/internal/model_render_test.gosrc/internal/ui/filemodel/dimensions.gosrc/internal/ui/filepanel/dimension.gosrc/internal/model.gosrc/internal/model_render.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/model_render_test.gosrc/internal/model_render_unix_test.go
📚 Learning: 2025-04-12T14:00:49.244Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:10-13
Timestamp: 2025-04-12T14:00:49.244Z
Learning: In the sidebar package, variables like `pinnedDividerDir` and `diskDividerDir` should remain unexported as they're only used within the package, even though their struct fields are exported with PascalCase (Name, Location) for JSON serialization purposes.
Applied to files:
src/internal/ui/filepanel/consts.gosrc/internal/handle_panel_navigation.go
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/internal/ui/filepanel/consts.gosrc/internal/handle_panel_navigation.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/ui/filepanel/consts.go
📚 Learning: 2025-08-24T03:25:10.117Z
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.
Applied to files:
src/internal/ui/filepanel/consts.go
📚 Learning: 2025-04-12T14:01:13.080Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/render.go:45-45
Timestamp: 2025-04-12T14:01:13.080Z
Learning: In Go, identifiers should remain unexported (lowercase) when they're only used within their own package, even if other similar identifiers are exported. Exported (capitalized) identifiers should only be used when the element needs to be accessible from outside its package.
Applied to files:
src/internal/ui/filepanel/consts.go
📚 Learning: 2025-08-06T10:54:31.444Z
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.
Applied to files:
src/internal/ui/filepanel/consts.go
📚 Learning: 2025-04-28T03:48:46.327Z
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.
Applied to files:
src/internal/ui/filepanel/consts.go
📚 Learning: 2025-04-28T04:27:33.074Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/model_render.go:254-256
Timestamp: 2025-04-28T04:27:33.074Z
Learning: When race conditions or other issues that are unrelated to the current PR's focus are identified during review, lazysegtree prefers creating separate GitHub issues to track them rather than addressing them in the current PR. This helps maintain PR focus and scope.
Applied to files:
src/internal/ui/filepanel/consts.go
📚 Learning: 2025-07-27T15:32:06.922Z
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.
Applied to files:
src/internal/ui/filepanel/consts.go
📚 Learning: 2025-08-06T10:27:20.420Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:27:20.420Z
Learning: lazysegtree specifically asked for a "strict review to find bad stuff" in PR reviews, indicating he wants comprehensive analysis that identifies all potential issues regardless of scope, not just issues within the current PR's scope.
Applied to files:
src/internal/ui/filepanel/consts.go
📚 Learning: 2025-09-04T07:24:30.872Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1041
File: src/internal/default_config.go:38-38
Timestamp: 2025-09-04T07:24:30.872Z
Learning: In the superfile codebase, the main model struct has a zClient field for zoxide directory tracking, and the trackDirectoryWithZoxide function in model.go checks if m.zClient is nil before proceeding. When reviewing model initialization functions like defaultModelConfig, ensure all struct fields are properly assigned, especially external service clients like zClient that enable core functionality.
Applied to files:
src/internal/default_config.gosrc/internal/type.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/default_config.gosrc/internal/type.gosrc/internal/handle_modal.gosrc/internal/model.go
📚 Learning: 2025-04-12T12:25:06.352Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:25:06.352Z
Learning: PR #770 "Sidebar code separation" is a refactoring and code reorganization effort with no new features. It involves moving code, updating import paths, renaming functions to follow PascalCase convention, and consolidating sidebar-related functionality into a dedicated package.
Applied to files:
src/internal/handle_panel_navigation.go
📚 Learning: 2025-04-03T16:05:49.630Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 752
File: src/internal/common/load_config.go:27-37
Timestamp: 2025-04-03T16:05:49.630Z
Learning: In the superfile project, configuration struct fields use PascalCase (e.g., "ShellCloseOnSuccess") while their corresponding TOML field names use snake_case (e.g., "shell_close_on_success") as defined in the struct tags. Boolean configuration fields typically don't require explicit validation in the LoadConfigFile function, unlike numeric fields which have range validations.
Applied to files:
src/superfile_config/config.toml
📚 Learning: 2025-08-05T11:51:37.645Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:44-44
Timestamp: 2025-08-05T11:51:37.645Z
Learning: In Bubble Tea applications, when functions like warnModalForRenaming() capture state (like m.ioReqCnt) and increment it before returning a tea.Cmd, the increment happens synchronously in the calling context (typically Update()), not inside the returned tea.Cmd function. Since Update() is called synchronously by the Bubble Tea runtime, there are no race conditions in this pattern.
Applied to files:
src/internal/model.go
📚 Learning: 2025-06-05T12:42:45.139Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 821
File: src/internal/file_operations_compress.go:0-0
Timestamp: 2025-06-05T12:42:45.139Z
Learning: Pattern to watch for: defer file.Close() inside loops (for, range, filepath.Walk callbacks) causes resource leaks by accumulating file descriptors until function exits. Should be replaced with explicit Close() calls immediately after file operations with proper error handling.
Applied to files:
src/internal/model.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/model_render.go
🧬 Code graph analysis (16)
src/internal/ui/filemodel/utils.go (1)
src/internal/ui/filemodel/type.go (1)
FileModel(10-19)
src/internal/ui/filemodel/consts.go (2)
src/internal/ui/filepanel/consts.go (2)
MinHeight(7-7)MinWidth(8-8)src/internal/ui/filepanel/model.go (1)
New(47-61)
src/internal/ui/filemodel/render.go (1)
src/internal/ui/filemodel/type.go (1)
FileModel(10-19)
src/internal/ui/filemodel/navigation.go (1)
src/internal/ui/filemodel/type.go (1)
FileModel(10-19)
src/internal/handle_panel_movement.go (1)
src/internal/common/ui_consts.go (1)
InnerPadding(12-12)
src/internal/ui/filemodel/update.go (2)
src/internal/ui/filemodel/type.go (1)
FileModel(10-19)src/internal/ui/filemodel/consts.go (1)
ErrMaximumPanelCount(17-17)
src/internal/validation.go (3)
src/internal/common/predefined_variable.go (2)
MinFooterHeight(36-36)MinimumWidth(32-32)src/internal/common/ui_consts.go (2)
BorderPadding(11-11)InnerPadding(12-12)src/internal/utils/ui_utils.go (2)
FullFooterHeight(17-22)FooterWidth(12-14)
src/internal/ui/filepanel/consts.go (1)
src/internal/common/ui_consts.go (1)
BorderPadding(11-11)
src/internal/ui/filepanel/dimension.go (4)
src/internal/ui/filepanel/types.go (1)
Model(14-34)src/internal/ui/filepanel/consts.go (2)
MinWidth(8-8)MinHeight(7-7)src/internal/common/ui_consts.go (2)
InnerPadding(12-12)BorderPadding(11-11)src/config/icon/icon.go (1)
Cursor(31-31)
src/internal/default_config.go (5)
src/internal/ui/preview/model.go (1)
New(43-56)src/internal/ui/sidebar/sidebar.go (1)
New(107-122)src/internal/ui/filemodel/type.go (1)
FileModel(10-19)src/internal/ui/filepanel/model.go (1)
FilePanelSlice(11-19)src/internal/common/ui_consts.go (1)
DefaultFilePanelWidth(15-15)
src/internal/type.go (1)
src/internal/ui/filemodel/type.go (1)
FileModel(10-19)
src/internal/handle_file_operations.go (2)
src/internal/common/style_function.go (1)
GenerateRenameTextInput(279-295)src/internal/common/ui_consts.go (1)
InnerPadding(12-12)
src/internal/handle_modal.go (1)
src/internal/ui/filepanel/types.go (1)
Model(14-34)
src/internal/model_prompt_test.go (3)
src/internal/test_utils.go (1)
TeaUpdate(63-66)src/internal/utils/tea_utils.go (1)
TeaRuneKeyMsg(5-10)src/internal/ui/prompt/consts.go (2)
SplitCommand(11-11)OpenCommand(10-10)
src/internal/model.go (5)
src/internal/ui/filepanel/types.go (2)
Element(58-64)PanelMode(67-67)src/internal/common/ui_consts.go (5)
HeightBreakA(19-19)HeightBreakB(20-20)HeightBreakC(21-21)HeightBreakD(22-22)BorderPadding(11-11)src/internal/utils/ui_utils.go (1)
FullFooterHeight(17-22)src/internal/utils/file_utils.go (1)
ResolveAbsPath(190-203)src/internal/ui/filepanel/consts.go (1)
MinWidth(8-8)
src/internal/model_render.go (3)
src/internal/ui/spf_renderers.go (1)
ClipboardRenderer(147-151)src/internal/common/ui_consts.go (2)
BorderPadding(11-11)FooterGroupCols(13-13)src/internal/utils/ui_utils.go (1)
FooterWidth(12-14)
⏰ 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
🔇 Additional comments (47)
src/internal/ui/filemodel/utils.go (1)
5-7: LGTM!The accessor centralizes focused panel access, aligning with the broader refactor. The invariant that
FocusedPanelIndexis valid is maintained by callers likeMoveFocusedPanelBywhich checksPanelCount() == 0before accessing. Based on learnings, this approach of maintaining correctness through design invariants rather than defensive checks at every access point is the preferred pattern.src/internal/handle_panel_movement.go (3)
127-132: LGTM!The refactoring from direct indexing to
m.getFocusedFilePanel()is consistent with the centralized accessor pattern introduced across the codebase.
145-153: LGTM!The simplification of
toggleFooterControllerproperly delegates tosetHeightValues()andupdateComponentDimensions(), aligning with the centralized dimension management approach in this PR.
156-167: LGTM!The search bar focus logic correctly uses the focused panel accessor and references
SinglePanelWidthfrom the file model for consistent width calculations.src/superfile_config/config.toml (1)
71-74: LGTM!The new
page_scroll_sizeconfiguration option is well-documented with its default behavior (0 = full page). This aligns with the navigation test coverage added in the PR.src/internal/ui/filemodel/navigation.go (2)
5-11: LGTM!Simple wrappers that delegate to
MoveFocusedPanelBywith appropriate delta values.
13-21: LGTM!The wrap-around logic using
(index + delta + count) % countcorrectly handles both forward and backward navigation. The guard against zero panels prevents division-by-zero and invalid slice access.src/internal/model_render_unix_test.go (1)
25-28: LGTM!The test correctly references the now-exported
FilePreviewfield, consistent with the broader refactoring that exposes file model state publicly.src/internal/ui/filepanel/consts.go (1)
1-12: LGTM!The constants are appropriately scoped:
contentPaddingandsortOptionsDefault*are unexported for internal use, whileMinHeightandMinWidthare exported as they're referenced by thefilemodelpackage for dimension calculations.src/internal/ui/filemodel/render.go (1)
5-14: LGTM!The render logic correctly composes file panels horizontally and conditionally includes the preview. The use of
lipgloss.JoinHorizontal(lipgloss.Top, ...)properly aligns panels at the top.Minor note: when the preview is closed,
f[m.PanelCount()]will be an empty string passed toJoinHorizontal. This is harmless as lipgloss handles empty strings gracefully, but you could optionally trim the slice if the preview is closed.src/internal/model_render_test.go (1)
145-147: LGTM!The test correctly accesses the now-exported
FilePreviewfield and uses its public API methods (SetWidth,SetHeight,RenderWithPath). This aligns with the refactoring to use the publicFileModelstructure.src/internal/model_msg.go (1)
188-199: LGTM!The field access change to
m.fileModel.FilePreview.SetContent()is consistent with the refactoring to use exportedFileModelfields. The guard clauses for nil checks and location validation remain properly in place.src/internal/model_test.go (1)
125-132: LGTM!The test correctly uses the now-exported
FilePanelsfield to verify panel initialization. The assertions properly validate cursor positioning and render index calculations for files at different positions in the directory listing.src/internal/ui/filemodel/consts.go (1)
1-17: LGTM!The package constants are well-structured:
- The aliased dimension constants maintain semantic separation while deriving from the filepanel source values, as documented in the comment.
ErrMaximumPanelCountprovides a proper sentinel error for panel lifecycle management.src/internal/ui/filemodel/type.go (1)
8-19: LGTM!The
FileModelstruct cleanly aggregates the file panel state with all necessary fields for multi-panel management. The exported fields enable proper access from the maininternalpackage while centralizing the file model concerns in a dedicated package.src/internal/type.go (1)
55-55: LGTM!The
fileModelfield correctly references the newfilemodel.FileModeltype, centralizing file panel state in the dedicated package.src/internal/handle_panel_navigation.go (2)
11-11: LGTM!Using the
getFocusedFilePanel()accessor is cleaner than direct array indexing and centralizes the focus logic.
25-28: No issues found:getFocusedFilePanel()returns*filepanel.Model, correctly enabling mutation.src/internal/key_function.go (3)
34-34: LGTM! Consistent migration to focused-panel accessor.The navigation methods (
ListUp,ListDown,PgUp,PgDown) now correctly usegetFocusedFilePanel()instead of direct indexing. The removal of height parameters aligns with the centralized dimension management introduced in this PR.Also applies to: 47-47, 51-54
59-72: LGTM! Panel management correctly routed through FileModel.Panel operations (
NextFilePanel,PreviousFilePanel,CloseFilePanel,CreateNewFilePanel) are now properly delegated tom.fileModel, which centralizes panel lifecycle management in the newfilemodelpackage.
151-156: LGTM! Selection operations use focused panel accessor.
SingleItemSelect,ItemSelectUp, andItemSelectDownnow correctly operate on the focused panel via the accessor method.src/internal/handle_modal.go (2)
58-63: LGTM! Rename cancellation properly updates both state flags.The
cancelRenamemethod correctly updates bothpanel.Renamingandm.fileModel.Renaming, maintaining consistency between the panel-level and model-level renaming state.
294-302: LGTM! Clean helper methods for focused panel access and preview toggle.The
toggleFilePreviewPanel()andgetFocusedFilePanel()helpers centralize common operations, reducing code duplication across the codebase. The preview toggle correctly updates child component widths after state change.src/internal/handle_file_operations.go (2)
33-33: LGTM! Consistent use of focused panel accessor.File creation, rename conflict checking, and rename initiation all correctly use
getFocusedFilePanel()for panel access.Also applies to: 44-44, 81-81
99-109: LGTM! Rename width calculation uses correct public field.The rename text input width is correctly calculated using
m.fileModel.SinglePanelWidth - common.InnerPadding, which aligns with the new publicSinglePanelWidthfield introduced inFileModel.src/internal/ui/filemodel/update.go (2)
11-33: LGTM! Well-structured panel creation with proper validation.
CreateNewFilePanelcorrectly:
- Validates maximum panel count
- Checks location accessibility via
os.Stat- Inherits sort options from current focused panel
- Updates focus state and propagates height
- Triggers width recalculation
35-50: LGTM! Safe panel closure with proper cleanup.
CloseFilePanelcorrectly handles edge cases (last panel), updates the focus index, and triggers layout recalculation.src/internal/model_render.go (3)
22-25: LGTM! Sidebar render correctly uses focused panel location.The sidebar render now obtains the current panel's location through the focused panel accessor, keeping it consistent with the rest of the refactoring.
62-67: LGTM! Centralized clipboard width calculation.The
getClipboardWidth()helper encapsulates the footer width calculation logic, handling the modulo-3 case for proper width distribution across footer components.
273-274: LGTM! Sort options render uses focused panel accessor.Consistent with the broader refactoring pattern.
src/internal/model_prompt_test.go (2)
133-148: LGTM! Tests correctly updated for public API.The split panel test now uses
m.fileModel.FilePanelsandm.fileModel.MaxFilePanelconsistently with the exported field naming convention.
194-195: LGTM! CloseFilePanel calls updated to use FileModel method.All
CloseFilePanel()invocations correctly route throughm.fileModel.CloseFilePanel().Also applies to: 203-203, 210-210, 221-221, 229-229, 339-339
src/internal/ui/filemodel/dimensions.go (3)
10-28: LGTM! Dimension setters with minimum enforcement.
SetHeightandSetWidthcorrectly enforce minimum dimensions and propagate updates to child components.PanelCount()is a clean accessor.
62-68: LGTM! Width distribution across panels.The loop correctly uses Go 1.22+ range-over-integer syntax to iterate panels. The last panel receives any remaining width to handle non-divisible cases.
49-54: The implementation ofFilePreviewWidthis correct. According to the configuration documentation,FilePreviewWidthallows '0' (same as file panel) or values from 2 to 10, where the value represents "one xth of the total width." Line 53's calculation (previewWidth = m.Width / common.Config.FilePreviewWidth) correctly treatsFilePreviewWidthas a divisor, which aligns with both the configuration comment and the validation logic in the codebase.src/internal/default_config.go (1)
32-36: LGTM! Clean initialization of public FileModel.The
fileModelfield correctly uses the exportedfilemodel.FileModeltype with:
FilePanelsviafilepanel.FilePanelSliceFilePreviewviapreview.New()(single instance, avoiding the duplicate ImagePreviewer issue from PR #1021)SinglePanelWidthset tocommon.DefaultFilePanelWidthBased on learnings, this single
preview.New()call is the correct pattern.src/internal/ui/filepanel/dimension.go (1)
1-46: LGTM! Clean dimension management implementation.The dimension management methods are well-structured with appropriate clamping to minimum values and proper side-effect handling (updating
SearchBar.Widthon width change, triggeringscrollToCursoron height change). The getter methods provide consistent access to computed layout values.src/internal/validation.go (3)
14-120: Comprehensive layout validation with clear invariant checks.The
validateLayoutfunction provides thorough validation of UI model invariants including footer state consistency, dimension calculations, panel counts, and overlay bounds. The//nolint:gocognitannotation is appropriate given the cumulative nature of validation checks.
122-193: Well-structured render validation with border checking.The
validateRenderfunction correctly usesansi.Stripbefore dimension checks and validates border characters when required. The check for minimum lines before border validation (line 149) prevents index-out-of-bounds errors.
195-263: Good component-level and final render validation.The component validation flow properly validates sidebar, file panels, file model, and footer components with appropriate dimensions. The final render validation includes placement checks for file panels with the noted overlay limitation documented inline.
src/internal/model.go (7)
64-65: Good integration of resize command into update flow.The resize handling now properly returns a
tea.Cmdthat triggers file preview re-rendering with updated dimensions. This ensures the preview content is refreshed when terminal dimensions change.Also applies to: 78-79, 113-114
217-237: Clean extraction of height calculation logic.The
setHeightValuesfunction properly separates footer height calculation based on screen size breakpoints and computesmainPanelHeightconsistent with the validation invariant invalidateLayout.
239-256: Proper dimension propagation to all components.The
updateComponentDimensionsfunction ensures all UI components are resized consistently after a window resize event, with the file model dimensions correctly accounting for sidebar width and border padding.
522-581: Well-integrated view rendering with debug validation.The
Viewfunction properly uses the newfileModel.Render()method and includes comprehensive debug validation that's appropriately guarded behindcommon.Config.Debug. The validation errors are logged rather than panicking, allowing debug insight without breaking the application.
140-167: Proper use of FilePreview accessor methods.The
getFilePreviewCmdfunction correctly uses the new accessor pattern (IsOpen(),GetLocation(),SetLocation(),SetContentWithRenderText(),RenderWithPath()) for file preview operations.
306-322: Consistent migration to public field names in debug logging.The debug logging now uses the exported field names (
FocusedPanelIndex,FilePanels,Renaming) which aligns with the PR's refactoring to expose these fields publicly on the file model.
667-676: Correct iteration over FilePanels collection.The loop correctly iterates using the index to access and modify
FilePanels[i]in place, callingUpdateElementsIfNeededon each panel.
|
Next step will be to add more unit test to ensure model-wide dimension handling on resize, footer toogle, etc. Another PR. This is getting too big. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/internal/ui/filepanel/model.go (1)
14-14: Fix comment typo.Minor typo: "focussed" should be "focused".
🔎 Proposed fix
- // Making the first panel as the focussed + // Making the first panel as the focused
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/internal/ui/filepanel/consts.gosrc/internal/ui/filepanel/dimension.gosrc/internal/ui/filepanel/model.gosrc/internal/validation.gosrc/superfile_config/config.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- src/internal/ui/filepanel/dimension.go
- src/internal/ui/filepanel/consts.go
- src/superfile_config/config.toml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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: 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 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, 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: 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: 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: 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: 0
File: :0-0
Timestamp: 2025-04-12T12:25:06.352Z
Learning: PR #770 "Sidebar code separation" is a refactoring and code reorganization effort with no new features. It involves moving code, updating import paths, renaming functions to follow PascalCase convention, and consolidating sidebar-related functionality into a dedicated package.
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: 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: 1040
File: src/internal/function.go:58-58
Timestamp: 2025-09-05T08:04:14.324Z
Learning: lazysegtree asked about "dual-panel functionality" and "secondary focus" in the superfile codebase. The system actually supports multiple file panels (not specifically dual-panel), where users can create/close panels and navigate between them. The old `secondFocus` state was used to mark non-active panels when multiple panels were open, not for a specific dual-panel feature. The focus states were: noneFocus (no file panel focused), focus (active panel), and secondFocus (inactive panels).
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-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/validation.go
📚 Learning: 2025-03-29T13:20:46.467Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 730
File: src/internal/function.go:211-211
Timestamp: 2025-03-29T13:20:46.467Z
Learning: Empty panel.element checks should be added at the beginning of functions that access panel.element to prevent crashes when a panel has no elements, similar to using `if len(panel.element) == 0 { return }`.
Applied to files:
src/internal/validation.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/validation.go
📚 Learning: 2025-04-12T13:51:24.691Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/render.go:0-0
Timestamp: 2025-04-12T13:51:24.691Z
Learning: In the sidebar component of yorukot/superfile, the partial list rendering in render.go is intentional. The sidebar implements scrolling functionality where ListUp/ListDown methods in navigation.go update cursor position and renderIndex to show the appropriate portion of the directory list based on available space.
Applied to files:
src/internal/validation.go
🧬 Code graph analysis (2)
src/internal/validation.go (4)
src/internal/common/predefined_variable.go (2)
MinFooterHeight(36-36)MinimumWidth(32-32)src/internal/ui/filepanel/model.go (1)
New(47-61)src/internal/common/ui_consts.go (2)
BorderPadding(11-11)InnerPadding(12-12)src/internal/utils/ui_utils.go (2)
FullFooterHeight(17-22)FooterWidth(12-14)
src/internal/ui/filepanel/model.go (4)
src/internal/ui/filepanel/types.go (2)
Model(14-34)PanelMode(67-67)src/internal/common/default_config.go (1)
Config(11-11)src/internal/common/style_function.go (1)
GenerateSearchBar(237-249)src/internal/ui/filepanel/consts.go (2)
MinWidth(8-8)MinHeight(7-7)
⏰ 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 (3)
src/internal/ui/filepanel/model.go (2)
21-45: LGTM! Well-structured file path handling.The function correctly handles both file and directory paths. The error handling on Line 26 is safe—
statis only accessed whenerr == nil, preventing nil dereference. The silent error fallback (using the path as-is when stat fails) is appropriate graceful degradation for a constructor function.
47-61: LGTM! Clean constructor implementation.The
Newfunction properly initializes all Model fields with appropriate defaults. The emptyDirectoryRecordsmap is correctly allocated (not nil), and the use ofcommon.GenerateSearchBar()ensures consistent search bar configuration across panels.src/internal/validation.go (1)
8-8: ansi.Strip, ansi.StringWidth, and ansi.Cut functions are available in v0.9.3.The Cut function is ANSI-aware and accounts for wide-characters. All three functions are documented as available in the github.com/charmbracelet/x/ansi package.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/internal/ui/processbar/model_utils.go (2)
26-28: LGTM! Simple and correct dimension accessor.The getter correctly exposes the raw
heightfield, complementing the existingviewHeight()method that returns the adjusted dimension.Optionally, consider adding a documentation comment for this exported method following Go conventions:
💡 Optional: Add godoc comment
+// GetHeight returns the total height of the process bar including borders. func (m *Model) GetHeight() int { return m.height }
30-32: LGTM! Consistent with GetHeight() implementation.The getter correctly exposes the raw
widthfield, maintaining symmetry withGetHeight()and complementing the existingviewWidth()method.Optionally, consider adding a documentation comment for this exported method following Go conventions:
💡 Optional: Add godoc comment
+// GetWidth returns the total width of the process bar including borders. func (m *Model) GetWidth() int { return m.width }src/internal/ui/clipboard/model.go (1)
70-72: Consider making a defensive copy inSetItemsfor consistency.
GetItems()returns a copy to prevent external mutation, butSetItems()directly assigns the slice reference. This inconsistency could allow external code to mutate the internal state after callingSetItems.🔎 Proposed fix
func (m *Model) SetItems(items []string) { - m.items.items = items + m.items.items = make([]string, len(items)) + copy(m.items.items, items) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/internal/common/predefined_variable.gosrc/internal/default_config.gosrc/internal/handle_file_operation_test.gosrc/internal/handle_file_operations.gosrc/internal/model.gosrc/internal/model_file_operations_test.gosrc/internal/model_msg.gosrc/internal/model_render.gosrc/internal/test_utils.gosrc/internal/type.gosrc/internal/type_utils.gosrc/internal/ui/clipboard/model.gosrc/internal/ui/clipboard/model_test.gosrc/internal/ui/filemodel/dimensions.gosrc/internal/ui/filemodel/navigation.gosrc/internal/ui/filemodel/render.gosrc/internal/ui/filemodel/type.gosrc/internal/ui/filemodel/update.gosrc/internal/ui/filemodel/utils.gosrc/internal/ui/metadata/model.gosrc/internal/ui/processbar/model_utils.gosrc/internal/utils/ui_utils.gosrc/internal/validation.go
💤 Files with no reviewable changes (1)
- src/internal/type_utils.go
✅ Files skipped from review due to trivial changes (1)
- src/internal/ui/clipboard/model_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
- src/internal/handle_file_operations.go
- src/internal/ui/filemodel/dimensions.go
- src/internal/ui/filemodel/utils.go
- src/internal/ui/filemodel/render.go
- src/internal/model_render.go
- src/internal/ui/filemodel/navigation.go
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
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: 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 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, 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: 0
File: :0-0
Timestamp: 2025-04-12T12:25:06.352Z
Learning: PR #770 "Sidebar code separation" is a refactoring and code reorganization effort with no new features. It involves moving code, updating import paths, renaming functions to follow PascalCase convention, and consolidating sidebar-related functionality into a dedicated package.
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: 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: 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: 1040
File: src/internal/function.go:58-58
Timestamp: 2025-09-05T08:04:14.324Z
Learning: lazysegtree asked about "dual-panel functionality" and "secondary focus" in the superfile codebase. The system actually supports multiple file panels (not specifically dual-panel), where users can create/close panels and navigate between them. The old `secondFocus` state was used to mark non-active panels when multiple panels were open, not for a specific dual-panel feature. The focus states were: noneFocus (no file panel focused), focus (active panel), and secondFocus (inactive panels).
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-08-03T15:07:56.341Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model.go:118-120
Timestamp: 2025-08-03T15:07:56.341Z
Learning: In the processbar package (src/internal/ui/processbar/model.go), the Progress.Width field is modified temporarily during rendering on a copy of the process data, and this modification is intentionally not persisted to the stored process data since it's only needed for display purposes during the render call.
Applied to files:
src/internal/ui/metadata/model.gosrc/internal/ui/processbar/model_utils.gosrc/internal/model.go
📚 Learning: 2025-08-02T16:56:18.390Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 970
File: src/internal/ui/processbar/model_test.go:67-67
Timestamp: 2025-08-02T16:56:18.390Z
Learning: The `AddOrUpdateProcess` method in the processbar.Model type (src/internal/ui/processbar/model.go) has a void return type and doesn't return any values, so there's no return value to check in tests or other code.
Applied to files:
src/internal/ui/processbar/model_utils.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/validation.go
📚 Learning: 2025-08-06T10:54:31.444Z
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.
Applied to files:
src/internal/validation.go
📚 Learning: 2025-08-24T03:25:10.117Z
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.
Applied to files:
src/internal/validation.gosrc/internal/default_config.go
📚 Learning: 2025-04-28T03:48:46.327Z
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.
Applied to files:
src/internal/validation.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/internal/validation.go
📚 Learning: 2025-04-28T04:27:33.074Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/model_render.go:254-256
Timestamp: 2025-04-28T04:27:33.074Z
Learning: When race conditions or other issues that are unrelated to the current PR's focus are identified during review, lazysegtree prefers creating separate GitHub issues to track them rather than addressing them in the current PR. This helps maintain PR focus and scope.
Applied to files:
src/internal/validation.go
📚 Learning: 2025-07-27T15:32:06.922Z
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.
Applied to files:
src/internal/validation.go
📚 Learning: 2025-08-06T10:27:20.420Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:27:20.420Z
Learning: lazysegtree specifically asked for a "strict review to find bad stuff" in PR reviews, indicating he wants comprehensive analysis that identifies all potential issues regardless of scope, not just issues within the current PR's scope.
Applied to files:
src/internal/validation.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/validation.gosrc/internal/default_config.gosrc/internal/model_msg.gosrc/internal/model.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/validation.gosrc/internal/default_config.gosrc/internal/model.go
📚 Learning: 2025-08-29T13:56:33.955Z
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.
Applied to files:
src/internal/validation.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/validation.go
📚 Learning: 2025-04-12T13:51:24.691Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/render.go:0-0
Timestamp: 2025-04-12T13:51:24.691Z
Learning: In the sidebar component of yorukot/superfile, the partial list rendering in render.go is intentional. The sidebar implements scrolling functionality where ListUp/ListDown methods in navigation.go update cursor position and renderIndex to show the appropriate portion of the directory list based on available space.
Applied to files:
src/internal/validation.gosrc/internal/model.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/default_config.go
📚 Learning: 2025-04-12T12:00:32.688Z
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.
Applied to files:
src/internal/default_config.gosrc/internal/model.go
📚 Learning: 2025-09-04T07:24:30.872Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1041
File: src/internal/default_config.go:38-38
Timestamp: 2025-09-04T07:24:30.872Z
Learning: In the superfile codebase, the main model struct has a zClient field for zoxide directory tracking, and the trackDirectoryWithZoxide function in model.go checks if m.zClient is nil before proceeding. When reviewing model initialization functions like defaultModelConfig, ensure all struct fields are properly assigned, especially external service clients like zClient that enable core functionality.
Applied to files:
src/internal/default_config.gosrc/internal/type.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/default_config.gosrc/internal/type.gosrc/internal/model.go
📚 Learning: 2025-04-12T14:02:46.575Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:16-16
Timestamp: 2025-04-12T14:02:46.575Z
Learning: In Go packages, constants and variables should only be exported (capitalized) if they need to be accessed from outside their package. In the sidebar package, constants like `sideBarInitialHeight` should remain unexported as they're only used within the package.
Applied to files:
src/internal/utils/ui_utils.go
📚 Learning: 2025-09-06T13:42:44.590Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1039
File: src/internal/ui/zoxide/model.go:53-54
Timestamp: 2025-09-06T13:42:44.590Z
Learning: The zoxide modal in src/internal/ui/zoxide/model.go is missing handling for common.Hotkeys.Quit when zClient is nil (lines 52-54), only handling ConfirmTyping and CancelTyping. This creates inconsistency with other modals like sort options menu, help menu, and notify model which all properly handle the Quit hotkey. The prompt modal has the same inconsistency.
Applied to files:
src/internal/type.go
📚 Learning: 2025-08-05T11:51:37.645Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:44-44
Timestamp: 2025-08-05T11:51:37.645Z
Learning: In Bubble Tea applications, when functions like warnModalForRenaming() capture state (like m.ioReqCnt) and increment it before returning a tea.Cmd, the increment happens synchronously in the calling context (typically Update()), not inside the returned tea.Cmd function. Since Update() is called synchronously by the Bubble Tea runtime, there are no race conditions in this pattern.
Applied to files:
src/internal/model.go
📚 Learning: 2025-06-05T12:42:45.139Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 821
File: src/internal/file_operations_compress.go:0-0
Timestamp: 2025-06-05T12:42:45.139Z
Learning: Pattern to watch for: defer file.Close() inside loops (for, range, filepath.Walk callbacks) causes resource leaks by accumulating file descriptors until function exits. Should be replaced with explicit Close() calls immediately after file operations with proper error handling.
Applied to files:
src/internal/model.go
🧬 Code graph analysis (9)
src/internal/ui/metadata/model.go (4)
src/internal/ui/clipboard/model.go (1)
Model(13-17)src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/preview/model.go (1)
Model(32-41)src/internal/ui/processbar/model.go (1)
Model(12-26)
src/internal/ui/processbar/model_utils.go (1)
src/internal/ui/processbar/model.go (1)
Model(12-26)
src/internal/validation.go (4)
src/internal/common/predefined_variable.go (2)
MinFooterHeight(36-36)MinimumWidth(32-32)src/internal/ui/filepanel/model.go (1)
New(47-61)src/internal/common/ui_consts.go (2)
BorderPadding(11-11)InnerPadding(12-12)src/internal/utils/ui_utils.go (1)
FullFooterHeight(8-13)
src/internal/default_config.go (3)
src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/filepanel/model.go (1)
FilePanelSlice(11-19)src/internal/common/ui_consts.go (1)
DefaultFilePanelWidth(15-15)
src/internal/common/predefined_variable.go (1)
src/config/icon/icon.go (2)
Error(36-36)Space(10-10)
src/internal/model_msg.go (1)
src/internal/ui/processbar/process.go (2)
Failed(43-43)Successful(41-41)
src/internal/type.go (3)
src/internal/ui/clipboard/model.go (1)
Model(13-17)src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/filepanel/types.go (1)
Model(14-34)
src/internal/ui/filemodel/update.go (2)
src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/filemodel/consts.go (1)
ErrMaximumPanelCount(17-17)
src/internal/model.go (5)
src/internal/ui/filepanel/types.go (2)
Element(58-64)PanelMode(67-67)src/internal/common/ui_consts.go (5)
HeightBreakA(19-19)HeightBreakB(20-20)HeightBreakC(21-21)HeightBreakD(22-22)BorderPadding(11-11)src/internal/utils/ui_utils.go (2)
FullFooterHeight(8-13)CntFooterPanels(3-3)src/internal/utils/file_utils.go (1)
ResolveAbsPath(190-203)src/internal/ui/filepanel/consts.go (1)
MinWidth(8-8)
⏰ 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 (26)
src/internal/ui/metadata/model.go (1)
32-38: LGTM! Dimension getters align with project-wide refactoring.The addition of
GetHeight()andGetWidth()getter methods is consistent with the broader effort to centralize dimension handling across panel models (similar patterns exist in clipboard, filemodel, preview, and processbar modules). The implementation is clean and follows Go conventions.src/internal/utils/ui_utils.go (1)
3-3: The constant name follows the established codebase convention for "Count" abbreviations, as seen with theCntLines()method in the rendering package. The semantic change fromFilePanelMaxtoCntFooterPanelsis correct—the constant is used as a divisor to distribute footer width among three panels, not as a maximum limit. No changes needed.Likely an incorrect or invalid review comment.
src/internal/model_file_operations_test.go (1)
43-44: LGTM! Clean migration to public clipboard API.The test correctly uses the public clipboard API (
IsCut()andGetFirstItem()) instead of directly accessing internal fields, aligning with the broader refactor to expose clipboard state publicly.Also applies to: 54-55
src/internal/test_utils.go (1)
118-123: LGTM! Test utilities correctly use public clipboard API.The helper functions now validate clipboard state through the public API methods (
IsCut()andGetItems()), ensuring tests exercise the same interface as production code.Also applies to: 166-166
src/internal/common/predefined_variable.go (1)
48-48: LGTM! Consistent empty-state text addition.The
ClipboardNoneTextvariable follows the established pattern for empty-state messages (matchingProcessBarNoneText,SideBarNoneText, etc.) and is properly initialized inLoadPrerenderedVariables.Also applies to: 126-126
src/internal/handle_file_operation_test.go (1)
270-272: LGTM! Test assertions correctly use public clipboard API.The clipboard state verification now uses
GetItems()andReset(false), properly aligning with the public clipboard interface introduced in this refactor.Also applies to: 285-285
src/internal/ui/filemodel/type.go (1)
1-19: LGTM! Clean public API introduction for file model.The new
filemodel.Modelstruct properly exports all fields, establishing a clear public API surface for the file window system. Field names follow Go conventions and align with the broader refactoring to expose UI state publicly.src/internal/model_msg.go (2)
42-43: LGTM! Clipboard state access migrated to public API.The paste operation message handler correctly uses
clipboard.IsCut()andclipboard.Reset(false)instead of accessing internalcopyItemsfields.
198-198: LGTM! FilePreview field access updated to public API.The change from
fileModel.filePreviewtofileModel.FilePreviewcorrectly uses the exported field from the new publicfilemodel.Modelstruct.src/internal/ui/filemodel/update.go (2)
11-33: LGTM! CreateNewFilePanel correctly manages panel lifecycle.The method properly validates the maximum panel count and location accessibility before creating a new panel. Focus state transitions are handled correctly:
- Previous focused panel loses focus
- New panel becomes focused and inherits sort options
- Layout is recomputed via
UpdateChildComponentWidth()
35-50: LGTM! CloseFilePanel correctly handles panel removal and focus.The focus index management is correct:
- Guards against closing the last panel
- Properly removes the focused panel from the slice
- Correctly adjusts
FocusedPanelIndex(decrements when not at 0)- Sets focus on the new focused panel
- Recomputes layout via
UpdateChildComponentWidth()The logic correctly handles edge cases (removing first panel, removing last panel, removing middle panel) through the conditional decrement pattern.
src/internal/default_config.go (1)
8-9: LGTM! Clean migration to public filemodel.Model.The initialization correctly uses the new public
filemodel.Modelstruct with exported fields (FilePanels,FilePreview,SinglePanelWidth). The field values are properly sourced fromfilepanel.FilePanelSlice(),preview.New(), and the default width constant. Thepreview.New()function creates a singleImagePreviewerinstance with no duplicate initialization patterns.src/internal/type.go (2)
6-8: Good separation of concerns with external model packages.The refactoring to use
filemodel.Modelandclipboard.Modelfrom dedicated packages improves code organization and encapsulation. The removal of internalfileModelstruct andcopyItemsfield in favor of these external types aligns with the PR's modularization goals.Also applies to: 57-60
55-98: LGTM!The main
modelstruct is well-organized with logical groupings (Main Panels, Modals, state fields). The existing TODO comment appropriately documents the potential optimization opportunity.src/internal/ui/clipboard/model.go (3)
30-55: LGTM!The
Rendermethod handles edge cases well: empty state display, overflow indication with remaining item count, and graceful error handling for inaccessible files. The use ofos.Lstatcorrectly handles symlinks.
61-64: LGTM!Using
m.items.items[:0]to reset the slice is an efficient Go idiom that preserves the underlying array capacity for reuse.
74-98: LGTM!The accessor methods are well-implemented with appropriate defensive practices:
GetItems()returns a copy, andGetFirstItem()safely handles the empty case.src/internal/model.go (8)
62-115: LGTM!The
Updatefunction properly handles the newresizeCmdreturn value fromhandleWindowResizeand batches all commands correctly. The variable declarations are appropriately updated.
209-255: LGTM!The dimension handling is well-structured.
handleWindowResizeproperly chainssetHeightValues→updateComponentDimensions→getFilePreviewCmd(true), ensuring preview content is re-rendered with new dimensions after resize.
290-298: LGTM!The footer component sizing correctly distributes width across the three panels, with the remainder going to the clipboard panel. This ensures no pixels are wasted.
518-578: LGTM!The
View()method cleanly integrates the newfileModel.Render()andclipboard.Render()calls. The debug-only validation is a good practice for catching layout issues during development without impacting production performance.
663-672: LGTM!The
getFilePanelItemsmethod correctly iterates using index-based access to allow direct modification ofFilePanels[i], and properly propagates the dot file toggle state.
676-696: LGTM!The
quitSuperfilemethod properly cleans up resources using the newfileModel.FilePreview.CleanUp()and correctly retrieves the current directory for the cd-on-quit feature.
476-483: LGTM!The panel creation methods properly delegate to
fileModel.CreateNewFilePanel(), maintaining good encapsulation. Path resolution correctly usesutils.ResolveAbsPath.
140-167: LGTM! ThegetFilePreviewCmdmethod correctly uses the centralizedgetFocusedFilePanel()accessor and properly handles the file preview update flow with the newfileModel.FilePreviewfield. All referenced methods (SetContentWithRenderText,SetLocation,GetLocation,RenderWithPath) are properly defined in the preview package.src/internal/validation.go (1)
1-302: Comprehensive validation infrastructure looks solid.The validation subsystem effectively enforces UI rendering invariants across layout, rendering, component placement, and dimensional consistency. The logic correctly validates footer dimensions, panel counts, focus bounds, border styles, and component placement. Error messages are descriptive and will aid debugging. The code uses Go 1.22+ integer range syntax (
for i := range cntRowat line 290), and the project's go.mod specifies Go 1.25.0, which is compatible.
|
Fixed one nitpick comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/internal/default_config.go (1)
32-36: Consider explicitly initializing FocusedPanelIndex to 0.The
filemodel.Modelinitialization correctly sets upFilePanels,FilePreview, andSinglePanelWidth. However,FocusedPanelIndexrelies on the zero value (0) to match the first panel thatFilePanelSlicemarks as focused. While the zero value is correct here, an explicitFocusedPanelIndex: 0would improve clarity and make the intent more obvious.🔎 Proposed enhancement for clarity
fileModel: filemodel.Model{ FilePanels: filepanel.FilePanelSlice(firstPanelPaths), FilePreview: preview.New(), SinglePanelWidth: common.DefaultFilePanelWidth, + FocusedPanelIndex: 0, },src/internal/ui/filemodel/dimensions.go (2)
62-68: Use idiomatic loop syntax for clarity.The loop
for i := range panelCountworks correctly but is less clear thanfor i := range m.FilePanels. Using the slice directly makes the intent obvious and avoids subtle dependencies on thepanelCountvariable remaining synchronized.🔎 Proposed refactor for idiomatic loop
- for i := range panelCount { + for i := range m.FilePanels { if i == panelCount-1 { m.FilePanels[i].SetWidth(lastPanelWidth) } else { m.FilePanels[i].SetWidth(panelWidth) } }Note: You'll still need the
panelCountvariable for the width calculations (lines 59-60), but using the slice for iteration is clearer.
46-57: FilePreviewWidth naming could be clearer—it's documented as a divisor, not an absolute width value.The configuration documentation in
src/internal/common/config_type.gocorrectly explains thatFilePreviewWidthrepresents "one xth of the total width" (a divisor, not pixels), with valid values 0 (equal split) or 2–10. However, the field nameFilePreviewWidthdoesn't immediately convey this divisor semantics. Consider renaming toFilePreviewDivisor,FilePreviewRatio, or similar to reduce confusion for developers who might not read the configuration comment.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
src/internal/common/predefined_variable.gosrc/internal/default_config.gosrc/internal/handle_file_operation_test.gosrc/internal/handle_file_operations.gosrc/internal/key_function.gosrc/internal/model.gosrc/internal/model_file_operations_test.gosrc/internal/model_msg.gosrc/internal/model_render.gosrc/internal/test_utils.gosrc/internal/type.gosrc/internal/type_utils.gosrc/internal/ui/clipboard/model.gosrc/internal/ui/clipboard/model_test.gosrc/internal/ui/filemodel/dimensions.gosrc/internal/ui/filemodel/navigation.gosrc/internal/ui/filemodel/render.gosrc/internal/ui/filemodel/type.gosrc/internal/ui/filemodel/update.gosrc/internal/ui/filemodel/utils.gosrc/internal/ui/metadata/model.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/processbar/model_navigation_test.gosrc/internal/ui/processbar/model_utils.gosrc/internal/ui/sidebar/consts.gosrc/internal/ui/sidebar/directory_utils.gosrc/internal/ui/sidebar/navigation.gosrc/internal/ui/sidebar/navigation_test.gosrc/internal/ui/sidebar/render.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/ui/sidebar/type.gosrc/internal/ui/sidebar/utils.gosrc/internal/utils/ui_utils.gosrc/internal/validation.gosrc/internal/wheel_function.go
💤 Files with no reviewable changes (1)
- src/internal/type_utils.go
🚧 Files skipped from review as they are similar to previous changes (11)
- src/internal/ui/filemodel/utils.go
- src/internal/wheel_function.go
- src/internal/test_utils.go
- src/internal/ui/clipboard/model_test.go
- src/internal/utils/ui_utils.go
- src/internal/ui/clipboard/model.go
- src/internal/ui/filemodel/update.go
- src/internal/ui/processbar/model_utils.go
- src/internal/common/predefined_variable.go
- src/internal/handle_file_operations.go
- src/internal/ui/filemodel/render.go
🧰 Additional context used
🧠 Learnings (34)
📓 Common learnings
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 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.
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: 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: 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: 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: 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/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:25:06.352Z
Learning: PR #770 "Sidebar code separation" is a refactoring and code reorganization effort with no new features. It involves moving code, updating import paths, renaming functions to follow PascalCase convention, and consolidating sidebar-related functionality into a dedicated package.
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: 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: 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.
📚 Learning: 2025-08-03T15:07:56.341Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model.go:118-120
Timestamp: 2025-08-03T15:07:56.341Z
Learning: In the processbar package (src/internal/ui/processbar/model.go), the Progress.Width field is modified temporarily during rendering on a copy of the process data, and this modification is intentionally not persisted to the stored process data since it's only needed for display purposes during the render call.
Applied to files:
src/internal/ui/sidebar/type.gosrc/internal/ui/filemodel/dimensions.gosrc/internal/ui/processbar/model_navigation_test.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/metadata/model.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/model_render.gosrc/internal/model.go
📚 Learning: 2025-04-12T14:02:46.575Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:16-16
Timestamp: 2025-04-12T14:02:46.575Z
Learning: In Go packages, constants and variables should only be exported (capitalized) if they need to be accessed from outside their package. In the sidebar package, constants like `sideBarInitialHeight` should remain unexported as they're only used within the package.
Applied to files:
src/internal/ui/sidebar/type.gosrc/internal/ui/sidebar/consts.gosrc/internal/ui/sidebar/utils.gosrc/internal/ui/sidebar/render.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/sidebar/navigation.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/ui/sidebar/navigation_test.go
📚 Learning: 2025-04-12T14:00:49.244Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:10-13
Timestamp: 2025-04-12T14:00:49.244Z
Learning: In the sidebar package, variables like `pinnedDividerDir` and `diskDividerDir` should remain unexported as they're only used within the package, even though their struct fields are exported with PascalCase (Name, Location) for JSON serialization purposes.
Applied to files:
src/internal/ui/sidebar/consts.gosrc/internal/ui/sidebar/utils.gosrc/internal/ui/sidebar/directory_utils.go
📚 Learning: 2025-04-12T13:51:24.691Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/render.go:0-0
Timestamp: 2025-04-12T13:51:24.691Z
Learning: In the sidebar component of yorukot/superfile, the partial list rendering in render.go is intentional. The sidebar implements scrolling functionality where ListUp/ListDown methods in navigation.go update cursor position and renderIndex to show the appropriate portion of the directory list based on available space.
Applied to files:
src/internal/ui/sidebar/consts.gosrc/internal/ui/sidebar/render.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/sidebar/navigation.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/model_render.gosrc/internal/key_function.gosrc/internal/ui/sidebar/navigation_test.gosrc/internal/model.go
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/internal/ui/sidebar/consts.gosrc/internal/ui/sidebar/directory_utils.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/default_config.gosrc/internal/validation.gosrc/internal/model_render.gosrc/internal/type.gosrc/internal/model_msg.gosrc/internal/model.go
📚 Learning: 2025-04-12T12:00:32.688Z
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.
Applied to files:
src/internal/default_config.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/default_config.go
📚 Learning: 2025-09-04T07:24:30.872Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1041
File: src/internal/default_config.go:38-38
Timestamp: 2025-09-04T07:24:30.872Z
Learning: In the superfile codebase, the main model struct has a zClient field for zoxide directory tracking, and the trackDirectoryWithZoxide function in model.go checks if m.zClient is nil before proceeding. When reviewing model initialization functions like defaultModelConfig, ensure all struct fields are properly assigned, especially external service clients like zClient that enable core functionality.
Applied to files:
src/internal/default_config.gosrc/internal/type.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/default_config.gosrc/internal/type.gosrc/internal/model.go
📚 Learning: 2025-08-29T13:56:33.955Z
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.
Applied to files:
src/internal/ui/processbar/model_navigation_test.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/validation.gosrc/internal/ui/sidebar/navigation.gosrc/internal/key_function.gosrc/internal/ui/sidebar/navigation_test.gosrc/internal/ui/filemodel/navigation.go
📚 Learning: 2025-08-02T16:56:18.390Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 970
File: src/internal/ui/processbar/model_test.go:67-67
Timestamp: 2025-08-02T16:56:18.390Z
Learning: The `AddOrUpdateProcess` method in the processbar.Model type (src/internal/ui/processbar/model.go) has a void return type and doesn't return any values, so there's no return value to check in tests or other code.
Applied to files:
src/internal/ui/processbar/model_navigation_test.gosrc/internal/ui/processbar/model_navigation.go
📚 Learning: 2025-04-12T12:25:06.352Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:25:06.352Z
Learning: PR #770 "Sidebar code separation" is a refactoring and code reorganization effort with no new features. It involves moving code, updating import paths, renaming functions to follow PascalCase convention, and consolidating sidebar-related functionality into a dedicated package.
Applied to files:
src/internal/ui/sidebar/render.gosrc/internal/ui/sidebar/navigation.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/ui/sidebar/navigation_test.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/sidebar/render.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/sidebar/navigation.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/validation.gosrc/internal/ui/sidebar/navigation_test.go
📚 Learning: 2025-08-06T10:54:31.444Z
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.
Applied to files:
src/internal/validation.go
📚 Learning: 2025-08-24T03:25:10.117Z
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.
Applied to files:
src/internal/validation.go
📚 Learning: 2025-04-28T03:48:46.327Z
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.
Applied to files:
src/internal/validation.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/internal/validation.go
📚 Learning: 2025-04-28T04:27:33.074Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/model_render.go:254-256
Timestamp: 2025-04-28T04:27:33.074Z
Learning: When race conditions or other issues that are unrelated to the current PR's focus are identified during review, lazysegtree prefers creating separate GitHub issues to track them rather than addressing them in the current PR. This helps maintain PR focus and scope.
Applied to files:
src/internal/validation.go
📚 Learning: 2025-07-27T15:32:06.922Z
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.
Applied to files:
src/internal/validation.go
📚 Learning: 2025-08-06T10:27:20.420Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:27:20.420Z
Learning: lazysegtree specifically asked for a "strict review to find bad stuff" in PR reviews, indicating he wants comprehensive analysis that identifies all potential issues regardless of scope, not just issues within the current PR's scope.
Applied to files:
src/internal/validation.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/validation.gosrc/internal/model.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/validation.go
📚 Learning: 2025-04-27T17:04:47.888Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 775
File: src/internal/common/style_function.go:240-243
Timestamp: 2025-04-27T17:04:47.888Z
Learning: lazysegtree plans to refactor textinput.Model creation in the future to fix the code duplication issue that caused a bug where the sidebar search model was created without setting the Width property.
Applied to files:
src/internal/ui/sidebar/sidebar.go
📚 Learning: 2025-09-06T13:42:44.590Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1039
File: src/internal/ui/zoxide/model.go:53-54
Timestamp: 2025-09-06T13:42:44.590Z
Learning: The zoxide modal in src/internal/ui/zoxide/model.go is missing handling for common.Hotkeys.Quit when zClient is nil (lines 52-54), only handling ConfirmTyping and CancelTyping. This creates inconsistency with other modals like sort options menu, help menu, and notify model which all properly handle the Quit hotkey. The prompt modal has the same inconsistency.
Applied to files:
src/internal/type.gosrc/internal/key_function.go
📚 Learning: 2025-11-03T01:53:26.636Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1113
File: src/superfile_config/vimHotkeys.toml:66-67
Timestamp: 2025-11-03T01:53:26.636Z
Learning: In yorukot/superfile, modal-specific hotkeys (like nav_bulk_rename and rev_nav_bulk_rename using tab/shift+tab in the bulk rename modal) can safely conflict with global hotkey bindings (like next_file_panel and previous_file_panel) because the modal captures input when focused, and the global bindings remain active when the modal is closed. This context-specific key handling is intentional and not considered a conflict.
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-09-05T08:04:14.324Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1040
File: src/internal/function.go:58-58
Timestamp: 2025-09-05T08:04:14.324Z
Learning: lazysegtree asked about "dual-panel functionality" and "secondary focus" in the superfile codebase. The system actually supports multiple file panels (not specifically dual-panel), where users can create/close panels and navigate between them. The old `secondFocus` state was used to mark non-active panels when multiple panels were open, not for a specific dual-panel feature. The focus states were: noneFocus (no file panel focused), focus (active panel), and secondFocus (inactive panels).
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-09-12T05:16:10.488Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/handle_modal.go:253-253
Timestamp: 2025-09-12T05:16:10.488Z
Learning: lazysegtree identified that the fuzzy search function in src/internal/handle_modal.go for the help menu only searches on item.description but should also include item.key in the search haystack to provide comprehensive search functionality, as users expect to find hotkeys by searching for their key names.
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-07-27T15:35:25.617Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:35:25.617Z
Learning: In yorukot/superfile, deleteItemWarn() only displays a warning modal via the channelMessage system and does not perform actual file deletion. The actual deletion happens when the user confirms the modal, which triggers getDeleteCmd() that returns a tea.Cmd. The architecture intentionally separates UI modal operations (using channelMessage/goroutines) from file I/O operations (using tea.Cmd pattern), creating clean separation of concerns between UI state management and file operations.
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-07-27T15:35:25.617Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:35:25.617Z
Learning: deleteItemWarn() in yorukot/superfile only displays a warning modal via channel messaging and does not perform actual file deletion. The actual deletion happens through getDeleteCmd() which returns a tea.Cmd. The architectural pattern separates UI modal operations (using channels/goroutines) from file I/O operations (using tea.Cmd), which is intentional and appropriate.
Applied to files:
src/internal/key_function.go
📚 Learning: 2025-08-05T11:51:37.645Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:44-44
Timestamp: 2025-08-05T11:51:37.645Z
Learning: In Bubble Tea applications, when functions like warnModalForRenaming() capture state (like m.ioReqCnt) and increment it before returning a tea.Cmd, the increment happens synchronously in the calling context (typically Update()), not inside the returned tea.Cmd function. Since Update() is called synchronously by the Bubble Tea runtime, there are no race conditions in this pattern.
Applied to files:
src/internal/model.go
📚 Learning: 2025-06-05T12:42:45.139Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 821
File: src/internal/file_operations_compress.go:0-0
Timestamp: 2025-06-05T12:42:45.139Z
Learning: Pattern to watch for: defer file.Close() inside loops (for, range, filepath.Walk callbacks) causes resource leaks by accumulating file descriptors until function exits. Should be replaced with explicit Close() calls immediately after file operations with proper error handling.
Applied to files:
src/internal/model.go
🧬 Code graph analysis (14)
src/internal/default_config.go (3)
src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/filepanel/model.go (1)
FilePanelSlice(11-19)src/internal/common/ui_consts.go (1)
DefaultFilePanelWidth(15-15)
src/internal/ui/filemodel/dimensions.go (3)
src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/filemodel/consts.go (2)
FileModelMinHeight(13-13)FileModelMinWidth(14-14)src/internal/ui/filepanel/consts.go (1)
MinWidth(8-8)
src/internal/ui/sidebar/utils.go (2)
src/internal/ui/sidebar/type.go (1)
Model(10-20)src/internal/ui/metadata/model.go (1)
Model(9-19)
src/internal/ui/sidebar/render.go (2)
src/internal/ui/sidebar/type.go (1)
Model(10-20)src/internal/ui/spf_renderers.go (1)
SidebarRenderer(12-32)
src/internal/ui/processbar/model_navigation.go (3)
src/internal/ui/metadata/model.go (1)
Model(9-19)src/internal/ui/processbar/model.go (1)
Model(12-26)src/internal/common/ui_consts.go (1)
BorderPadding(11-11)
src/internal/ui/metadata/model.go (4)
src/internal/ui/clipboard/model.go (1)
Model(13-17)src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/preview/model.go (1)
Model(32-41)src/internal/ui/processbar/model.go (1)
Model(12-26)
src/internal/validation.go (6)
src/internal/common/predefined_variable.go (1)
MinFooterHeight(36-36)src/internal/ui/sidebar/sidebar.go (1)
New(107-124)src/internal/ui/filepanel/model.go (1)
New(47-61)src/internal/common/ui_consts.go (2)
BorderPadding(11-11)InnerPadding(12-12)src/internal/utils/ui_utils.go (1)
FullFooterHeight(8-13)src/internal/common/default_config.go (1)
Config(11-11)
src/internal/ui/sidebar/navigation.go (2)
src/internal/ui/sidebar/type.go (1)
Model(10-20)src/internal/common/ui_consts.go (1)
BorderPadding(11-11)
src/internal/ui/sidebar/sidebar.go (2)
src/internal/common/default_config.go (1)
Config(11-11)src/internal/common/ui_consts.go (1)
BorderPadding(11-11)
src/internal/type.go (4)
src/internal/ui/clipboard/model.go (1)
Model(13-17)src/internal/ui/filemodel/type.go (1)
Model(10-19)src/internal/ui/filepanel/types.go (1)
Model(14-34)src/internal/ui/preview/model.go (1)
Model(32-41)
src/internal/model_msg.go (1)
src/internal/ui/processbar/process.go (2)
Failed(43-43)Successful(41-41)
src/internal/ui/filemodel/type.go (2)
src/internal/ui/clipboard/model.go (1)
Model(13-17)src/internal/ui/metadata/model.go (1)
Model(9-19)
src/internal/model.go (1)
src/internal/ui/filepanel/types.go (1)
Element(58-64)
src/internal/ui/filemodel/navigation.go (1)
src/internal/ui/filemodel/type.go (1)
Model(10-19)
⏰ 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: test
- GitHub Check: Build and Test (windows-latest)
🔇 Additional comments (47)
src/internal/ui/sidebar/type.go (1)
18-19: LGTM: Dimension fields added for layout management.The addition of
widthandheightfields to the Model struct supports the centralized dimension handling introduced across the PR. The fields are appropriately unexported and initialized in theNew()constructor.src/internal/ui/sidebar/consts.go (1)
23-28: LGTM: UI dimension constants added.The new constants support sidebar dimension handling and layout calculations. All constants are appropriately unexported as they're only used within the sidebar package.
src/internal/ui/metadata/model.go (1)
32-38: LGTM: Dimension getters added.The public getters provide consistent access to the model's dimensions, aligning with the centralized dimension management pattern introduced across other UI components in this PR.
src/internal/ui/sidebar/sidebar.go (1)
115-121: LGTM: Model initialization updated with dimension fields.The width and height fields are properly initialized in the constructor:
- Width accounts for the configured sidebar width plus border padding
- Height is initialized to the minimum height constant
- Search bar width is correctly calculated based on the new width field
src/internal/ui/filemodel/type.go (1)
1-19: LGTM: FileModel type definition introduced.The new
Modelstruct provides a clean public API for managing file panels, their layout, and focus state. All fields are appropriately exported to support the centralized navigation and dimension management implemented across the PR.src/internal/ui/filemodel/navigation.go (1)
13-21: Verify bounds onFocusedPanelIndexbefore callingGetFocusedFilePanel()on line 18.Line 18 calls
GetFocusedFilePanel()which directly accessesm.FilePanels[m.FocusedPanelIndex]with no bounds checking. Although line 19 normalizesFocusedPanelIndexvia wrap-around, the first call happens before that normalization. IfFocusedPanelIndexis ever out of bounds whenMoveFocusedPanelBy()is called, this will panic. The check at line 13 only validates that panels exist (m.PanelCount() == 0), not thatFocusedPanelIndexitself is valid. Consider adding an explicit bounds validation:if m.FocusedPanelIndex >= m.PanelCount() { m.FocusedPanelIndex = 0 }before line 18.src/internal/validation.go (1)
14-194: Excellent validation infrastructure for catching bugs during development.The comprehensive validation checks across layout dimensions, render output, and border structure will help catch integration issues early. The validation is appropriately gated behind debug mode to avoid production overhead.
src/internal/model.go (3)
64-65: Clean integration of file preview command handling.The addition of
filePreviewCmdproperly coordinates with the refactored file preview panel, ensuring preview updates flow through the Bubble Tea message system correctly.Also applies to: 110-110, 113-114
210-256: Well-structured resize handling refactor.Breaking down
handleWindowResizeinto focused helper functions (setHeightValues,updateComponentDimensions,setMainModelDimensions, etc.) significantly improves readability and maintainability. Each function has a clear, single responsibility.
562-576: Validation integration provides excellent debugging support.Calling validation checks only in debug mode strikes the right balance—catching layout/rendering bugs during development without impacting production performance.
src/internal/ui/sidebar/directory_utils.go (1)
88-88: Improved constant naming for clarity.The rename from
directoryCapacityExtratodirectoryCapacityForDividersmakes the preallocation logic more self-documenting.src/internal/handle_file_operation_test.go (1)
270-285: Clean migration to public clipboard API.Tests now properly use
clipboard.GetItems()andclipboard.Reset(false)instead of accessing internal fields, improving encapsulation and test maintainability.src/internal/ui/processbar/model_navigation_test.go (1)
167-170: Tests updated to match parameterless navigation API.The removal of the
footerHeightargument fromListDown()andListUp()aligns with the navigation refactoring. The model still receives height via the constructor, so behavior is preserved while the API is cleaner.src/internal/model_file_operations_test.go (1)
43-55: Proper use of clipboard public API in tests.Tests now correctly use
clipboard.IsCut()andclipboard.GetFirstItem()instead of accessing internal state, which improves test maintainability and ensures tests validate the public contract.src/internal/ui/sidebar/utils.go (1)
96-110: Good addition of dimension accessors with safety checks.The new
GetWidth(),GetHeight(), andSetHeight()methods complement the public API pattern used throughout the PR. The safety check inSetHeight()that logs an error and clamps tominHeightprevents invalid state while providing debugging visibility.src/internal/model_msg.go (2)
42-43: Consistent clipboard API usage in message handlers.The migration from
copyItems.cut/copyItems.reset()toclipboard.IsCut()/clipboard.Reset()properly uses the public clipboard interface in message handling code.
198-198: Correct use of public FilePreview field.The change from
fileModel.filePreviewtofileModel.FilePreviewaligns with the public API exposure, ensuring external components interact with FilePreview through the intended interface.src/internal/type.go (3)
6-8: LGTM!The new imports for
clipboardandfilemodelpackages align with the refactoring to use external model types for better separation of concerns.
55-61: LGTM!The transition from internal
fileModeltofilemodel.Modeland the addition of theclipboardfield consolidate UI state management into dedicated packages. This improves modularity and aligns with the centralized focused-panel access pattern introduced across the codebase.
83-84: LGTM!Retaining
disableMetadatawhile removingfilePanelFocusIndexis consistent with the refactor—focus index management is now handled withinfilemodel.Model.FocusedPanelIndex.src/internal/model_render.go (3)
19-22: LGTM!Using
getFocusedFilePanel().Locationfor sidebar rendering is a clean improvement that centralizes focused-panel access and simplifies the call site.
51-54: LGTM!Accessing
m.fileModel.FilePanelsdirectly for the terminal size warning calculation is appropriate here since this needs the total panel count, not just the focused panel.
232-233: LGTM!Using
getFocusedFilePanel()for sort options rendering is consistent with the refactored navigation and rendering patterns.src/internal/ui/processbar/model_navigation.go (4)
3-5: LGTM!Adding the
commonimport forBorderPaddingis necessary for the internal height calculation.
10-26: LGTM!The parameterless
ListUp()method correctly delegates to the newcntRenderableProcess()helper. The wrap-around logic is preserved and correctly uses the model's internal height.
29-43: LGTM!The parameterless
ListDown()method is consistent withListUp()and correctly usesm.cntRenderableProcess()for determining the visible range.
45-48: No action needed.The concern about uninitialized
m.heightis unfounded. TheSetDimensions()method enforces a minimum height of 2 (minHeight constant), and sincecommon.BorderPaddingis also 2, the minimumfooterHeightwould be 0, not -2. Additionally,m.heightis always initialized viaSetDimensions()during model construction, which occurs before any navigation methods are called.Likely an incorrect or invalid review comment.
src/internal/ui/sidebar/navigation_test.go (6)
7-8: LGTM!Adding the
commonimport forBorderPaddingenables consistent height calculation in tests.
82-90: LGTM!The test correctly sets the sidebar height via
SetHeight(tt.mainPanelHeight + common.BorderPadding)before invokinglastRenderedIndex(). This mirrors how height would be set in production code.
240-247: LGTM!Consistent test setup pattern for
firstRenderedIndex()tests.
363-376: LGTM!The
updateRenderIndex()tests correctly set height before the parameterless call.
464-479: LGTM!The
ListUp()tests follow the same pattern of setting height before the parameterless navigation call.
592-605: LGTM!The
ListDown()tests complete the consistent test pattern across all navigation methods.src/internal/ui/sidebar/render.go (3)
14-22: LGTM!The simplified
Rendersignature using model-provideds.heightands.widthis cleaner and aligns with the centralized dimension management pattern.
33-33: LGTM!Invoking
directoriesRenderwithout the height parameter is consistent with the refactored signature.
38-50: LGTM!The
directoriesRendermethod correctly derivesmainPanelHeightlocally froms.height - common.BorderPadding. This encapsulates the height calculation within the sidebar package.src/internal/ui/sidebar/navigation.go (6)
5-6: LGTM!Adding the
commonimport forBorderPaddingenables internal height derivation.
9-30: LGTM!The parameterless
ListUp()method with recursive calls for skipping dividers is clean. TheupdateRenderIndex()call correctly uses the model's internal height.
32-56: LGTM!The parameterless
ListDown()method mirrorsListUp()with consistent recursive handling for dividers.
62-74: LGTM!
lastRenderedIndex()correctly derivesmainPanelHeightfroms.height - common.BorderPaddinginternally.
78-96: LGTM!
firstRenderedIndex()follows the same pattern of internal height derivation.
98-124: LGTM!
updateRenderIndex()correctly uses the parameterlesslastRenderedIndex()andfirstRenderedIndex()helpers.src/internal/key_function.go (5)
25-35: LGTM!The parameterless
ListUp()calls for sidebar, processbar, and focused file panel are consistent with the refactored navigation API.
38-48: LGTM!The parameterless
ListDown()calls follow the same pattern asListUp().
50-54: LGTM!
PgUp()andPgDown()correctly delegate to the focused file panel.
59-72: LGTM!Panel lifecycle methods (
NextFilePanel,PreviousFilePanel,CloseFilePanel,CreateNewFilePanel) correctly delegate tofileModelwrappers, centralizing panel management.
149-157: LGTM!Select mode operations (
SingleItemSelect,ItemSelectUp,ItemSelectDown) correctly use the focused panel accessor with parameterless calls.
|
Non-squash merge used as too many things are being done. |
Fixes - #1206, #1022, #1176, #819
PRs
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.