chore: enable MND linter and fix all magic number issues #1180
Conversation
…, ops; ensure build
- Add KilobyteSize=1000, KibibyteSize=1024 for size calculations - Add TabWidth=4 for tab expansion - Apply constants in FormatFileSize and CheckAndTruncateLineLengths - Resolves 5 MND issues in string_function.go
- Add DefaultFilePanelWidth, ExtractedFileMode/DirMode, CenterDivisor constants - Replace magic numbers in default_config.go, file_operations_extract.go - Fix centering calculations in model.go using CenterDivisor - Update panel width calculations in handle_panel_navigation.go and model_render.go - Replace hardcoded padding values with constants across UI rendering
- Create constants.go with image processing constants - Add DefaultThumbnailCacheSize, HeightScaleFactor, RGBShift16/8 - Add Kitty protocol constants (HashSeed, HashPrime, MaxID, NonZeroOffset) - Apply nolint:mnd for EXIF orientation values (standard spec values) - Apply nolint:mnd for cache cleanup interval (half of expiration) - Update image RGB operations to use bit shift constants - Add separate WindowsPixelsPerColumn/Row constants (8/16) for Windows - Keep DefaultPixelsPerColumn/Row at original values (10/20) - Resolves 17 MND issues in pkg/file_preview/
- Add nolint annotations for metadata sort priority indices - Create processbar constants for dimensions and padding - Apply constants to processbar model and navigation - Fix golines formatting in image_preview.go
- Add constants for rendering/border dimensions and padding - Add constants for sidebar search bar and directory layout - Add constants for zoxide score display and input padding - Add constants for prompt input padding and expected argument count - Apply constants across all UI component files
- Add nolint annotations for metadata display indices and test dimensions - Add RGB color constants (RGBMask, AlphaOpaque) for image preview - Fix import cycle in utils by duplicating necessary constants - Add missing processbar import in metadata model - Fix golines formatting in sidebar render - Enable MND linter in golangci.yaml All 54 MND issues resolved, lint passes cleanly
- Mark progress as 100% complete (54/54 issues) - Document Phase 4 final cleanup details - Add final statistics and summary - Remove outdated TODO sections
WalkthroughThis PR is a comprehensive refactoring that centralizes hard-coded numeric literals and magic numbers into named constants across the codebase. New constant files are introduced in multiple packages, and existing files are updated to use these constants instead of inline values. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: |
5486bb7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ab6eeefb.superfile.pages.dev |
| Branch Preview URL: | https://add-mnd.superfile.pages.dev |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (16)
src/internal/ui/zoxide/constants.go (1)
4-10: Consider using unexported constants.Based on learnings from this repository, constants that are only used within their own package should remain unexported (lowercase). Since
ScoreColumnWidthandModalInputPaddingappear to only be used within thezoxidepackage, consider renaming them toscoreColumnWidthandmodalInputPadding.// UI dimension constants for zoxide modal const ( // ScoreColumnWidth is width reserved for score display (including padding and separator) - ScoreColumnWidth = 13 // borders(2) + padding(2) + score(6) + separator(3) + scoreColumnWidth = 13 // borders(2) + padding(2) + score(6) + separator(3) // ModalInputPadding is total padding for modal input fields - ModalInputPadding = 6 // 2 + 1 + 2 + 1 (borders and spacing) + modalInputPadding = 6 // 2 + 1 + 2 + 1 (borders and spacing) )src/internal/utils/ui_utils.go (1)
3-7: Clarify constant naming to avoid confusion withcommon.FilePanelMax.The
FilePanelMax = 3here represents the divisor for the three-panel footer layout, while the PR plan mentionscommon.FilePanelMax = 10as the maximum number of supported file panels. Consider renaming this constant to something more descriptive likeFooterPanelDivisororFooterColumnCountto distinguish it from the application-wide panel limit.-// FilePanelMax defines the maximum number of file panels -const FilePanelMax = 3 +// FooterColumnCount is the number of columns (panels) for footer width calculation +const FooterColumnCount = 3 -// BorderPaddingForFooter is the border padding for footer calculations -const BorderPaddingForFooter = 2 +// FooterBorderPadding is the border padding for footer calculations +const FooterBorderPadding = 2plan.md (1)
1-76: Consider removing this planning document before merge.This
plan.mdfile appears to be a working handoff document for tracking MND remediation progress. Such documents are typically not committed to the repository as they become stale after the PR is merged. Consider:
- Moving this content to the PR description for historical reference
- Removing the file before merge
Additionally, there's a duplicate heading "Golines Formatting" at lines 43-46 and 58-60.
src/internal/ui/processbar/model_navigation.go (1)
47-50: cntRenderableProcess now driven by LinesPerProcess constantUsing
LinesPerProcessinstead of the literal3keeps the renderable-process calculation configurable while preserving current behavior.You might also consider updating the preceding comment to reference
LinesPerProcessrather than “three lines” so it stays accurate if the constant ever changes.src/internal/common/style_function.go (1)
262-275: Well-scoped MND suppression for modal widthThe
//nolint:mndplus comment documentingModalWidth - 10is clear and localized, keeping the existing layout while satisfying the linter. No issues here.src/internal/handle_panel_up_down.go (1)
62-90: MND suppression on panCenter preserves clear centering logicKeeping
panCenter := panHeight / 2with a brief//nolint:mndexplanation retains the existing paging behavior while silencing the linter; no issues spotted.src/internal/ui/metadata/model.go (1)
3-8: Reusing processbar.BorderSize for metadata content heightSwitching from a hard-coded
-2tom.height - processbar.BorderSizekeeps the behavior the same while documenting the border adjustment. The only tradeoff is a small new dependency frommetadatatoprocessbar; if we start sharing this border size across more panels, it might be worth relocating the constant to a common UI package, but that can be deferred.Based on learnings, centralizing cross-package layout constants in a shared place usually keeps dependency direction cleaner.
Also applies to: 89-99
src/internal/ui/zoxide/utils.go (1)
42-51: Minor: consider tying the comment explicitly toModalInputPaddingThe new
width - ModalInputPaddingkeeps behavior identical to the previouswidth - (2 - 1 - 2 - 1)decomposition, and the breakdown comment is still accurate today. To avoid future drift ifModalInputPaddingis ever adjusted, you might reference the constant directly in the comment (e.g., “ModalInputPadding (borders + padding + prompt + extra char)”) so maintainers change both together.src/pkg/file_preview/image_resize.go (1)
74-87: EXIF orientation annotations and ANSI height scaling look consistentThe added
//nolint:mndcomments document EXIF orientation cases nicely, and switchingmaxHeight*2tomaxHeight*HeightScaleFactorkeeps behavior while centralizing the magic number. You may just want to remember that theresizeForANSIcomment (“*2”) should stay aligned withHeightScaleFactorif that constant ever changes.Also applies to: 95-98
src/internal/type_utils.go (1)
30-37: Layout validation and sort options defaults are fine; minor comment driftUsing
common.BorderPaddinginvalidateLayoutkeeps the existing math but centralizes the border constant, and the//nolint:mndaround the sort options dimensions make the intent of20x4clear. The only nit is that the comment on Line 30 still says “+ 2 lines (main border)” even though the value now comes fromcommon.BorderPadding; consider rephrasing it to reference the constant instead of the number so it stays accurate if the padding changes later.Also applies to: 64-79
src/internal/ui/prompt/model.go (1)
207-216: Text input width now cleanly tied to PromptInputPaddingSwitching to
width - PromptInputPaddingkeeps the existing layout while centralizing the padding logic with the new constant; the explanatory comment still matches the underlying math. Just keep the constant and this comment in sync if prompt chrome changes later.src/internal/handle_panel_navigation.go (1)
50-71: Consistent use of shared UI constants for panel and search widthsAll the updated width/height calculations now rely on
common.InnerPadding,common.BorderPadding, andcommon.FilePanelWidthUnit, which keeps behavior aligned with the old literals while centralizing layout tuning knobs. The same patterns increateNewFilePanel,closeFilePanel, andtoggleFilePreviewPanelalso look algebraically consistent. If you ever revisit this area, you might consider a small helper to compute(filePanelWidth, previewWidth, maxFilePanel)fromfullWidthandnumPanelsso these formulas live in one place, but that’s purely optional for this MND-focused PR.Also applies to: 60-70, 80-105, 111-129
src/internal/ui/sidebar/constants.go (1)
3-13: Sidebar constants look good; consider keeping them unexported for now
SearchBarPadding,DirectoryCapacityExtra, andDefaultRenderHeightare nice, descriptive replacements for the old magic numbers, and the comments explain their composition well. Since they’re currently only used within thesidebarpackage, you could make them unexported (e.g.,searchBarPadding) to keep the public surface minimal, and only export if another package needs them later—but that’s a style preference, not a blocker.src/cmd/main.go (1)
278-280: CLI update-check timeout correctly tied to shared constantUsing
common.DefaultCLIContextTimeoutincheckAndNotifyUpdatepreserves the existing 5s timeout while making it configurable in one place alongside other CLI UI constants. No issues here.src/internal/ui/processbar/constants.go (1)
4-16: Consider unexported constants for internal-only usage.Based on the AI summary, only
BorderSizeappears to be referenced outside the processbar package (insrc/internal/ui/metadata/model.go). The other three constants (ProgressBarRightPadding,ProcessNameTruncatePadding,LinesPerProcess) appear to be used only within the processbar package itself.Per Go conventions, constants should only be exported (capitalized) if they need to be accessed from outside their package.
Consider making the internal-only constants unexported:
// UI dimension constants for process bar rendering const ( // BorderSize is the border width for the process bar panel BorderSize = 2 - // ProgressBarRightPadding is padding after progress bar - ProgressBarRightPadding = 3 + // progressBarRightPadding is padding after progress bar + progressBarRightPadding = 3 - // ProcessNameTruncatePadding is the space reserved for ellipsis and icon in process name - ProcessNameTruncatePadding = 7 + // processNameTruncatePadding is the space reserved for ellipsis and icon in process name + processNameTruncatePadding = 7 - // LinesPerProcess is the number of lines needed to render one process - LinesPerProcess = 3 + // linesPerProcess is the number of lines needed to render one process + linesPerProcess = 3 )If these constants are indeed used outside the package, please disregard this comment.
Based on learnings, constants should only be exported if accessed from outside their package.
src/internal/ui/rendering/constants.go (1)
1-19: Consider whether these constants need to be exported.All five constants appear to be used only within the
renderingpackage itself (specifically inborder.go). Per Go conventions, constants should only be exported if they need to be accessed from outside their package.If these constants are not used outside the rendering package, consider making them unexported:
// Border rendering constants const ( - // BorderCornerWidth is the width occupied by border corners - BorderCornerWidth = 2 + // borderCornerWidth is the width occupied by border corners + borderCornerWidth = 2 - // BorderPaddingWidth is padding around border title/content - BorderPaddingWidth = 2 + // borderPaddingWidth is padding around border title/content + borderPaddingWidth = 2 // ... and so on for remaining constants )If these are intentionally exported for future external use or are indeed accessed from other packages, please disregard this comment.
Based on learnings, constants should only be exported if accessed from outside their package.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
.golangci.yaml(1 hunks)plan.md(1 hunks)plan.md.bak(1 hunks)src/cmd/main.go(2 hunks)src/internal/common/string_function.go(7 hunks)src/internal/common/style_function.go(2 hunks)src/internal/common/ui_consts.go(1 hunks)src/internal/default_config.go(1 hunks)src/internal/file_operations_extract.go(2 hunks)src/internal/function.go(2 hunks)src/internal/handle_file_operations.go(1 hunks)src/internal/handle_modal.go(3 hunks)src/internal/handle_panel_movement.go(2 hunks)src/internal/handle_panel_navigation.go(4 hunks)src/internal/handle_panel_up_down.go(2 hunks)src/internal/model.go(7 hunks)src/internal/model_render.go(11 hunks)src/internal/type_utils.go(2 hunks)src/internal/ui/metadata/const.go(1 hunks)src/internal/ui/metadata/model.go(2 hunks)src/internal/ui/metadata/utils.go(1 hunks)src/internal/ui/preview/model.go(1 hunks)src/internal/ui/processbar/constants.go(1 hunks)src/internal/ui/processbar/model.go(2 hunks)src/internal/ui/processbar/model_navigation.go(1 hunks)src/internal/ui/processbar/model_utils.go(1 hunks)src/internal/ui/prompt/constants.go(1 hunks)src/internal/ui/prompt/model.go(1 hunks)src/internal/ui/prompt/utils.go(2 hunks)src/internal/ui/rendering/border.go(3 hunks)src/internal/ui/rendering/constants.go(1 hunks)src/internal/ui/sidebar/constants.go(1 hunks)src/internal/ui/sidebar/directory_utils.go(1 hunks)src/internal/ui/sidebar/render.go(1 hunks)src/internal/ui/sidebar/sidebar.go(1 hunks)src/internal/ui/sidebar/utils.go(1 hunks)src/internal/ui/zoxide/constants.go(1 hunks)src/internal/ui/zoxide/render.go(1 hunks)src/internal/ui/zoxide/test_helpers.go(3 hunks)src/internal/ui/zoxide/utils.go(1 hunks)src/internal/utils/ui_utils.go(1 hunks)src/pkg/file_preview/constants.go(1 hunks)src/pkg/file_preview/image_preview.go(3 hunks)src/pkg/file_preview/image_resize.go(2 hunks)src/pkg/file_preview/kitty.go(1 hunks)src/pkg/file_preview/utils.go(2 hunks)
🧰 Additional context used
🧠 Learnings (27)
📓 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: 781
File: src/internal/ui/sidebar/render.go:46-48
Timestamp: 2025-04-28T03:48:46.327Z
Learning: The user (lazysegtree) prefers to keep PRs focused and manageable in size, sometimes intentionally leaving TODO comments to track minor issues for later PRs rather than addressing everything at once.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-04-28T04:02:28.747Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/rendering/border.go:104-114
Timestamp: 2025-04-28T04:02:28.747Z
Learning: The BorderConfig.GetBorder method in src/internal/ui/rendering/border.go already protects against negative availWidth values through the condition `actualWidth >= cnt*4` which ensures that availWidth is at least 1.
Applied to files:
src/internal/ui/metadata/utils.gosrc/internal/handle_panel_navigation.gosrc/internal/ui/processbar/model_utils.gosrc/internal/common/style_function.gosrc/internal/ui/sidebar/render.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/ui/rendering/border.gosrc/internal/utils/ui_utils.gosrc/internal/ui/prompt/model.gosrc/internal/ui/zoxide/render.gosrc/internal/model_render.gosrc/internal/ui/rendering/constants.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/ui/metadata/utils.gosrc/internal/handle_file_operations.gosrc/internal/ui/metadata/model.gosrc/internal/ui/processbar/model_utils.gosrc/internal/ui/processbar/constants.gosrc/internal/common/style_function.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/ui/prompt/model.gosrc/internal/ui/zoxide/render.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/processbar/model.gosrc/internal/model_render.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/directory_utils.goplan.md.bak
📚 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/directory_utils.gosrc/internal/ui/sidebar/constants.gosrc/internal/common/style_function.gosrc/internal/ui/sidebar/render.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/directory_utils.gosrc/internal/ui/processbar/constants.gosrc/internal/ui/sidebar/constants.gosrc/internal/common/style_function.gosrc/internal/ui/sidebar/render.gosrc/internal/ui/sidebar/sidebar.gosrc/internal/utils/ui_utils.gosrc/pkg/file_preview/constants.gosrc/internal/common/ui_consts.gosrc/internal/ui/zoxide/constants.gosrc/internal/ui/rendering/constants.gosrc/internal/model.goplan.md.baksrc/internal/common/string_function.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/handle_file_operations.gosrc/internal/handle_panel_navigation.gosrc/pkg/file_preview/constants.gosrc/internal/ui/preview/model.gosrc/internal/model_render.gosrc/internal/model.gosrc/pkg/file_preview/image_preview.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/handle_file_operations.gosrc/internal/ui/rendering/border.gosrc/internal/ui/zoxide/render.gosrc/internal/ui/processbar/model.gosrc/internal/common/string_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/handle_file_operations.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/handle_file_operations.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/metadata/model.gosrc/internal/handle_modal.gosrc/internal/ui/rendering/border.gosrc/internal/ui/zoxide/render.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/sidebar/utils.gosrc/internal/model_render.gosrc/internal/common/string_function.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/metadata/model.gosrc/internal/ui/processbar/model_utils.gosrc/internal/ui/processbar/constants.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/processbar/model.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/prompt/constants.gosrc/internal/ui/sidebar/constants.gosrc/internal/common/ui_consts.gosrc/internal/common/string_function.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/handle_panel_up_down.gosrc/internal/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/default_config.gosrc/internal/ui/zoxide/utils.gosrc/internal/handle_modal.gosrc/internal/ui/zoxide/constants.gosrc/internal/model_render.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/handle_panel_navigation.gosrc/pkg/file_preview/image_preview.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/handle_panel_navigation.gosrc/internal/ui/sidebar/constants.gosrc/internal/handle_modal.gosrc/internal/ui/sidebar/render.gosrc/internal/ui/processbar/model_navigation.gosrc/internal/ui/sidebar/utils.gosrc/internal/ui/preview/model.gosrc/internal/model_render.go
📚 Learning: 2025-08-24T03:24:50.857Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:18-21
Timestamp: 2025-08-24T03:24:50.857Z
Learning: The superfile project uses github.com/charmbracelet/x/ansi as the standardized ANSI package and deprecates github.com/charmbracelet/x/exp/term/ansi to avoid binary bloat and potential incompatibilities.
Applied to files:
src/internal/file_operations_extract.gosrc/internal/handle_panel_movement.gosrc/internal/ui/preview/model.gosrc/internal/common/string_function.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/handle_modal.gosrc/internal/model_render.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/handle_modal.gosrc/internal/model_render.go
📚 Learning: 2025-09-06T12:03:51.042Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1039
File: src/internal/common/config_type.go:156-156
Timestamp: 2025-09-06T12:03:51.042Z
Learning: lazysegtree identified that the in-app help menu in getHelpMenuData function in src/internal/default_config.go is missing many hotkeys defined in HotkeysType, including the new OpenZoxide hotkey, causing inconsistent user experience and reduced feature discoverability.
Applied to files:
src/internal/handle_modal.gosrc/internal/model_render.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/sidebar.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.gosrc/internal/handle_panel_movement.go
📚 Learning: 2025-08-27T11:32:17.326Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR #1021 for yorukot/superfile, the image preview crashes and test failures were caused by duplicate ImagePreviewer initialization - one in defaultModelConfig() and another in preview.New(). This created two separate instances that conflicted in terminal state management, causing crashes during image preview operations and test execution.
Applied to files:
src/pkg/file_preview/image_preview.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/ui/zoxide/test_helpers.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:
plan.md.bak
📚 Learning: 2025-05-22T04:45:44.934Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 825
File: src/internal/model_render.go:634-644
Timestamp: 2025-05-22T04:45:44.934Z
Learning: In the superfile codebase, the `r.AddLines(res)` method handles truncation internally. When a string is passed to the `AddLines` method of a renderer, it internally calls `AddLineWithCustomTruncate` which truncates the line using `TruncateBasedOnStyle` with the configured maximum line width. Therefore, there's no need to truncate strings before passing them to `AddLines`.
Applied to files:
src/internal/common/string_function.go
🧬 Code graph analysis (29)
src/internal/ui/sidebar/directory_utils.go (1)
src/internal/ui/sidebar/constants.go (1)
DirectoryCapacityExtra(9-9)
src/internal/handle_file_operations.go (2)
src/internal/common/style_function.go (1)
GenerateRenameTextInput(277-293)src/internal/common/ui_consts.go (1)
InnerPadding(12-12)
src/internal/ui/metadata/model.go (1)
src/internal/ui/processbar/constants.go (1)
BorderSize(6-6)
src/internal/default_config.go (1)
src/internal/common/ui_consts.go (1)
DefaultFilePanelWidth(15-15)
src/internal/handle_panel_navigation.go (1)
src/internal/common/ui_consts.go (3)
InnerPadding(12-12)BorderPadding(11-11)FilePanelWidthUnit(27-27)
src/internal/ui/processbar/model_utils.go (2)
src/internal/ui/processbar/constants.go (1)
BorderSize(6-6)src/internal/ui/processbar/model.go (1)
Model(12-26)
src/internal/ui/zoxide/utils.go (1)
src/internal/ui/zoxide/constants.go (1)
ModalInputPadding(9-9)
src/cmd/main.go (2)
src/internal/common/ui_consts.go (2)
HelpKeyColumnWidth(7-7)DefaultCLIContextTimeout(8-8)src/config/fixed_variable.go (5)
ConfigFile(66-66)HotkeysFile(67-67)LogFile(51-51)SuperFileMainDir(34-34)SuperFileDataDir(36-36)
src/internal/function.go (1)
src/internal/common/ui_consts.go (1)
PanelPadding(10-10)
src/pkg/file_preview/kitty.go (1)
src/pkg/file_preview/constants.go (4)
KittyHashSeed(14-14)KittyHashPrime(15-15)KittyMaxID(16-16)KittyNonZeroOffset(17-17)
src/internal/file_operations_extract.go (1)
src/internal/common/ui_consts.go (2)
ExtractedFileMode(31-31)ExtractedDirMode(32-32)
src/internal/handle_modal.go (1)
src/internal/common/ui_consts.go (1)
InnerPadding(12-12)
src/internal/common/style_function.go (2)
src/internal/common/default_config.go (1)
Config(11-11)src/internal/common/ui_consts.go (1)
PanelPadding(10-10)
src/internal/ui/sidebar/render.go (1)
src/internal/ui/spf_renderers.go (1)
SidebarRenderer(12-32)
src/internal/ui/sidebar/sidebar.go (1)
src/internal/ui/sidebar/constants.go (1)
SearchBarPadding(6-6)
src/internal/ui/rendering/border.go (1)
src/internal/ui/rendering/constants.go (5)
BorderCornerWidth(6-6)BorderDividerWidth(12-12)MinTitleWidth(15-15)BorderPaddingWidth(9-9)MinInfoItemWidth(18-18)
src/internal/ui/prompt/model.go (1)
src/internal/ui/prompt/constants.go (1)
PromptInputPadding(6-6)
src/internal/handle_panel_movement.go (1)
src/internal/common/ui_consts.go (1)
InnerPadding(12-12)
src/internal/ui/zoxide/render.go (1)
src/internal/ui/zoxide/constants.go (1)
ScoreColumnWidth(6-6)
src/internal/ui/processbar/model_navigation.go (1)
src/internal/ui/processbar/constants.go (1)
LinesPerProcess(15-15)
src/internal/ui/prompt/utils.go (1)
src/internal/ui/prompt/constants.go (1)
ExpectedArgCount(9-9)
src/internal/type_utils.go (2)
src/internal/common/ui_consts.go (1)
BorderPadding(11-11)src/internal/utils/ui_utils.go (1)
FullFooterHeight(17-22)
src/internal/ui/sidebar/utils.go (1)
src/internal/ui/sidebar/constants.go (1)
DefaultRenderHeight(12-12)
src/internal/ui/preview/model.go (1)
src/internal/common/ui_consts.go (1)
DefaultPreviewTimeout(28-28)
src/internal/ui/processbar/model.go (2)
src/internal/ui/processbar/constants.go (2)
ProgressBarRightPadding(9-9)ProcessNameTruncatePadding(12-12)src/internal/common/string_function.go (1)
TruncateText(30-37)
src/internal/model_render.go (5)
src/internal/common/ui_consts.go (6)
InnerPadding(12-12)BorderPadding(11-11)FooterGroupCols(13-13)PanelPadding(10-10)FilePanelWidthUnit(27-27)CenterDivisor(35-35)src/internal/ui/spf_renderers.go (1)
ClipboardRenderer(147-151)src/internal/common/string_function.go (1)
TruncateTextBeginning(39-58)src/internal/utils/ui_utils.go (1)
FooterWidth(12-14)src/internal/common/predefined_variable.go (1)
ModalWidth(30-30)
src/pkg/file_preview/image_preview.go (1)
src/pkg/file_preview/constants.go (5)
DefaultThumbnailCacheSize(6-6)RGBShift16(10-10)RGBShift8(11-11)RGBMask(20-20)AlphaOpaque(21-21)
src/internal/ui/zoxide/test_helpers.go (1)
src/internal/ui/zoxide/model.go (1)
GenerateModel(20-32)
src/pkg/file_preview/image_resize.go (1)
src/pkg/file_preview/constants.go (1)
HeightScaleFactor(9-9)
🪛 LanguageTool
plan.md
[grammar] ~45-~45: Ensure spelling is correct
Context: ...erations.go, function.go` - Extracted lipgloss styles for cleaner code ### Phase 4: F...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~53-~53: Ensure spelling is correct
Context: ...y duplicating constants - Fixed missing processbar import in metadata/model.go - Fixed gol...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~64-~64: Ensure spelling is correct
Context: ... resolved ✅ ## REMAINING WORK ✅ ALL MND ISSUES RESOLVED! - All 54 MND issues...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
plan.md
58-58: Multiple headings with the same content
(MD024, no-duplicate-heading)
⏰ 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). (4)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Analyze (go)
- GitHub Check: Cloudflare Pages
- GitHub Check: Cloudflare Pages
🔇 Additional comments (49)
.golangci.yaml (1)
102-102: LGTM!The MND linter is correctly enabled, and the existing configuration at lines 356-375 already provides sensible exclusions for functions like
os.Chmod,os.Mkdir.*,os.OpenFile, andos.WriteFilewhere file permission modes are expected.src/internal/utils/ui_utils.go (1)
9-14: LGTM!The footer width calculation logic is correct, and the inline comments clearly explain the math:
(fullWidth - 6) / 3 = fullWidth/3 - 2.src/internal/common/string_function.go (4)
19-28: LGTM!Well-organized constants block with clear comments. The distinction between SI decimal (
KilobyteSize = 1000) and binary (KibibyteSize = 1024) units is correctly documented, and the hex constants for character handling are appropriately named.
65-66: LGTM!The
//nolint:mndannotation is appropriately used here with a clear justification for the standard halving operation in center truncation.
130-137: LGTM!The file size formatting correctly uses the named constants for SI decimal and binary unit calculations.
219-236: LGTM!The character constants (
NonBreakingSpace,EscapeChar,ASCIIMax) improve readability over the previous hex literals while maintaining identical behavior.src/internal/ui/metadata/utils.go (1)
26-29: Scoped MND suppression keeps intent clearThe
//nolint:mndis localized to the standardviewWidth/2halving logic and doesn’t change behavior ofcomputeMetadataWidths.src/internal/ui/sidebar/utils.go (1)
6-10: Divider height now centralized via DefaultRenderHeightUsing
DefaultRenderHeightfor divider directories is behaviorally equivalent to the old3literal and centralizes the value for future tweaks.src/internal/default_config.go (1)
30-35: File panel width sourced from shared UI constantInitializing
fileModel.widthwithcommon.DefaultFilePanelWidthremoves the magic10while preserving the same default width.src/internal/handle_panel_movement.go (1)
12-16: Search bar width aligned with InnerPadding constantImporting
commonand switching tom.fileModel.width - common.InnerPaddingkeeps the prior-4behavior while tying the search bar width to the shared padding constant used elsewhere.Also applies to: 178-180
src/internal/ui/preview/model.go (1)
293-297: Preview timeout now uses shared DefaultPreviewTimeoutReplacing the inline
500mswithcommon.DefaultPreviewTimeoutcentralizes the bat preview timeout without altering the actual duration or context-cancel behavior.src/internal/ui/sidebar/sidebar.go (1)
117-121: Sidebar search bar width uses SearchBarPadding
res.searchBar.Width = common.Config.SidebarWidth - SearchBarPaddingmatches the prior2+2+1subtraction and cleanly encapsulates the padding in a single constant.src/internal/file_operations_extract.go (1)
11-13: Extraction permissions centralized via ExtractedFileMode/DirModePassing
common.ExtractedFileModeandcommon.ExtractedDirModeintoxtractr.XFileremoves duplicated0644/0755literals while keeping the same file and directory permissions for extracted content.Also applies to: 21-26
src/internal/ui/sidebar/directory_utils.go (1)
72-83: DirectoryCapacityExtra makes slice preallocation intent clearUsing
DirectoryCapacityExtrato account for the two divider entries keeps the capacity math explicit without changing behavior. Looks good.src/internal/common/style_function.go (1)
295-309: Pinned rename input width now tied to PanelPaddingBasing
ti.WidthonConfig.SidebarWidth - PanelPaddingaligns this input with the shared sidebar padding constant and keeps the behavior essentially unchanged. Looks good.src/internal/function.go (2)
244-246: panelElementHeight now uses shared PanelPaddingSwapping the literal offset for
common.PanelPaddingkeeps the math equivalent while centralizing UI spacing. No concerns.
292-300: Nolint for suffixRegexp match length is clear and harmlessThe
//nolint:mndwith “3 = full match + 2 capture groups” documents whylen(match) == 3is expected; behavior ofrenameIfDuplicateis unchanged.src/internal/ui/sidebar/render.go (1)
22-25: SidebarRenderer now uses BorderPadding constantUsing
mainPanelHeight+common.BorderPaddingandSidebarWidth+common.BorderPaddingreplaces the previous magic border offset with a shared constant, keeping layout behavior the same. Looks good.src/internal/ui/zoxide/render.go (1)
48-55: ScoreColumnWidth constant clarifies path width calculationUsing
ScoreColumnWidthinstead of the raw13makes the path truncation math self-documenting while preserving existing behavior.src/internal/ui/processbar/model.go (1)
121-135: Centralized paddings for progress width and name truncationReplacing the inline offsets with
ProgressBarRightPaddingandProcessNameTruncatePaddingkeeps the rendered output the same while making the layout rules easier to tweak in one place.src/internal/handle_file_operations.go (1)
79-104: Rename input width correctly switched to sharedInnerPaddingUsing
m.fileModel.width - common.InnerPaddinghere keeps the previous effective width (4) while aligning rename input sizing with the centralized UI padding constant. This is a straight, behavior‑preserving cleanup that improves consistency.src/internal/ui/prompt/utils.go (1)
37-56: Argument count check nicely centralized withExpectedArgCountSwitching the
cd/openbranches to useExpectedArgCountkeeps behavior identical while removing duplicated magic numbers and tying these commands to a single, self‑documenting constant.src/internal/ui/metadata/const.go (1)
21-30: Display-order indices well documented and lint-suppressedThe added comment plus per-entry
//nolint:mndannotations make it clear these integers are intentional display order indices, avoiding MND noise without changing behavior.src/internal/handle_modal.go (1)
9-11: Help menu scroll math correctly aligned with sharedInnerPaddingSwitching the help menu height calculations from a hardcoded
4tocommon.InnerPadding(and adding the corresponding import) keeps the scroll behavior the same while reusing the centralized padding constant used across the UI. The existing max/bottom clamping still guards against negative indices.Also applies to: 164-199
src/internal/ui/zoxide/test_helpers.go (1)
13-36: Focusedmndsuppressions in tests look appropriateMarking the fixed test dimensions and synthetic scores with
//nolint:mndkeeps the test helpers simple and readable while silencing expected magic-number warnings. No functional behavior is changed.src/pkg/file_preview/utils.go (1)
12-16: Windows terminal cell defaults cleanly factored into named constantsIntroducing
WindowsPixelsPerColumn/WindowsPixelsPerRowand wiring them intogetWindowsDefaultCellSizeis a pure refactor: same values, now self‑documenting and easier to tweak per‑platform without hunting down literals.Also applies to: 136-140
src/internal/ui/processbar/model_utils.go (1)
13-24: Process bar view dimensions correctly switched toBorderSizeReplacing the hardcoded
2withBorderSizeinisValid,viewHeight, andviewWidthkeeps the existing layout semantics while centralizing the border adjustment. This makes the process bar sizing logic more maintainable and consistent with the new constants.src/internal/ui/prompt/constants.go (1)
3-10: Prompt constants are well-documented and scoped
PromptInputPaddingandExpectedArgCountnicely capture the previously inlined magic numbers, and the explanatory padding breakdown will help future adjustments. No issues from a correctness or readability standpoint.src/cmd/main.go (1)
52-71: Path-list output now consistently uses shared column widthSwitching the
printfformat to usecommon.HelpKeyColumnWidthinstead of a hard-coded55and factoring the styles intologStyle,configStyle, anddataStylekeeps the help output behavior while centralizing formatting knobs. This all looks straightforward and aligns with the rest of the MND cleanup.src/pkg/file_preview/kitty.go (1)
82-93: Placement ID hashing constants are well-defined; empty-path fallback is defensive but unreachableThe constants
KittyHashSeed,KittyHashPrime,KittyMaxID, andKittyNonZeroOffsetare clearly defined in constants.go and make the placement ID logic more readable. The empty-path check returnsKittyHashSeed(42) explicitly, but this branch is unreachable in practice: by the timegeneratePlacementID()is called at line 110, the file path has already been validated throughos.Stat()andos.ReadFile()at lines 269 and 275, so empty paths cannot arrive here. The defensive fallback is appropriate and intentional.src/pkg/file_preview/image_preview.go (4)
52-53: LGTM!The initialization properly uses
DefaultThumbnailCacheSizeconstant and appropriately applies//nolint:mndto the time duration with a clear comment explaining the context.
85-86: LGTM!The cleanup interval calculation appropriately uses
//nolint:mndsince dividing by 2 is intrinsic to the cleanup algorithm logic.
328-333: LGTM!The color conversion correctly uses RGB constants (
RGBShift16,RGBShift8,RGBMask,AlphaOpaque) making the bit manipulation operations more readable and maintainable.
338-338: LGTM!Consistent use of
RGBShift8constant for color component extraction.src/internal/common/ui_consts.go (1)
1-36: LGTM!Excellent centralization of UI/layout constants with clear documentation and logical grouping. The constant values are reasonable and follow standard conventions (e.g., Unix file permissions, centering divisor).
src/internal/ui/rendering/border.go (1)
56-121: LGTM!The border rendering logic correctly replaces all hard-coded dimension literals with the new constants (
BorderCornerWidth,BorderPaddingWidth,BorderDividerWidth,MinTitleWidth,MinInfoItemWidth). The functional behavior remains unchanged while improving code maintainability.src/internal/model.go (7)
237-244: LGTM!File panel and preview sizing correctly use
common.FilePanelMaxandcommon.BorderPaddingconstants.
251-264: LGTM!File panel width calculations correctly use
common.InnerPadding,common.BorderPadding, andcommon.FilePanelWidthUnitconstants throughout.
267-308: LGTM!Height breakpoints (
HeightBreakA/B/C/D) and responsive sizing logic correctly use the centralized constants. The functional behavior is preserved while improving maintainability.
310-337: LGTM!Modal sizing correctly uses
//nolint:mndfor inherent layout calculations (dividing by 2 or 3 for modal dimensions), and metadata/processbar models properly usecommon.BorderPadding.
570-570: LGTM!Rename width check correctly uses
common.MinWidthForRenameconstant.
621-667: LGTM!All overlay positioning consistently uses
common.CenterDivisorfor centering calculations, making the centering logic explicit and maintainable. This is a significant improvement over implicit division by 2.
735-735: LGTM!Re-render throttling correctly uses
common.ReRenderChunkDivisorconstant.src/pkg/file_preview/constants.go (1)
1-22: LGTM!Image preview constants are well-organized, properly documented, and use standard values. The grouping into cache configuration, image processing, Kitty protocol, and RGB color masks makes the purpose of each constant clear.
src/internal/model_render.go (5)
45-77: LGTM!File panel rendering correctly uses
common.InnerPaddingandcommon.BorderPaddingfor width calculations and path truncation.
213-241: LGTM!Clipboard rendering correctly uses
common.FooterGroupCols,common.BorderPadding, andcommon.PanelPaddingconstants.
268-299: LGTM!Terminal size warning and typing modal rendering correctly use
common.FilePanelWidthUnitandcommon.InnerPaddingconstants.
365-423: LGTM!Help menu rendering correctly uses
common.FooterGroupCols,common.BorderPadding,common.CenterDivisor, andcommon.InnerPaddingthroughout. The use ofCenterDivisormakes centering calculations explicit and maintainable.
459-459: LGTM!Sort options rendering correctly uses
common.BorderPaddingconstant.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pkg/file_preview/constants.go (1)
6-24: Consider making package-internal constants unexported.Since these constants are only used within the
filepreviewpackage, they could be unexported (lowercase initial letters) to follow Go convention of only exporting identifiers that need external access.Based on learnings, constants should only be exported if accessed outside their package.
Apply this diff if you'd like to unexport them:
// Image preview constants const ( // Cache configuration - DefaultThumbnailCacheSize = 100 // Default number of thumbnails to cache - DefaultCacheExpiration = 5 * time.Minute + defaultThumbnailCacheSize = 100 // Default number of thumbnails to cache + defaultCacheExpiration = 5 * time.Minute // Image processing - HeightScaleFactor = 2 // Factor for height scaling in terminal display - RGBShift16 = 16 // Bit shift for red channel in RGB operations - RGBShift8 = 8 // Bit shift for green channel in RGB operations + heightScaleFactor = 2 // Factor for height scaling in terminal display + rgbShift16 = 16 // Bit shift for red channel in RGB operations + rgbShift8 = 8 // Bit shift for green channel in RGB operations // Kitty protocol - KittyHashSeed = 42 // Seed for kitty image ID hashing - KittyHashPrime = 31 // Prime multiplier for hash calculation - KittyMaxID = 0xFFFF // Maximum ID value for kitty images - KittyNonZeroOffset = 1000 // Offset to ensure non-zero IDs + kittyHashSeed = 42 // Seed for kitty image ID hashing + kittyHashPrime = 31 // Prime multiplier for hash calculation + kittyMaxID = 0xFFFF // Maximum ID value for kitty images + kittyNonZeroOffset = 1000 // Offset to ensure non-zero IDs // RGB color masks - RGBMask = 0xFF // Mask for extracting 8-bit RGB channel values - AlphaOpaque = 255 // Fully opaque alpha channel value + rgbMask = 0xFF // Mask for extracting 8-bit RGB channel values + alphaOpaque = 255 // Fully opaque alpha channel value )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pkg/file_preview/constants.go(1 hunks)src/pkg/file_preview/image_preview.go(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 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: 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: 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`.
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-27T11:32:17.326Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR #1021 for yorukot/superfile, the preview panel refactoring created duplicate ImagePreviewer instances (one in defaultModelConfig and another in preview.New()), which caused image preview crashes. The refactoring also renamed SetContextWithRenderText to SetContentWithRenderText, and moved global batCmd to per-instance batCmd initialization, all contributing to crashes during image preview and test failures.
Applied to files:
src/pkg/file_preview/image_preview.gosrc/pkg/file_preview/constants.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/pkg/file_preview/constants.go
📚 Learning: 2025-08-27T11:32:17.326Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR #1021 for yorukot/superfile, the image preview crashes and test failures were caused by duplicate ImagePreviewer initialization - one in defaultModelConfig() and another in preview.New(). This created two separate instances that conflicted in terminal state management, causing crashes during image preview operations and test execution.
Applied to files:
src/pkg/file_preview/constants.go
🧬 Code graph analysis (1)
src/pkg/file_preview/image_preview.go (1)
src/pkg/file_preview/constants.go (6)
DefaultThumbnailCacheSize(8-8)DefaultCacheExpiration(9-9)RGBShift16(12-12)RGBShift8(13-13)RGBMask(22-22)AlphaOpaque(23-23)
⏰ 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 (4)
src/pkg/file_preview/image_preview.go (4)
52-52: LGTM! Proper use of named constants.Replacing hardcoded cache defaults with named constants improves maintainability and makes the default configuration explicit.
84-85: LGTM! Appropriate use of nolint directive.The
//nolint:mnddirective is correctly applied here since dividing by 2 establishes a clear mathematical relationship (cleanup interval is half the expiration period).
327-332: LGTM! Correct RGB extraction logic.The bit manipulation correctly extracts RGB channels from the hex color format (#RRGGBB), and using named constants makes the bit shift operations more readable and maintainable.
337-337: LGTM! Correct 16-bit to 8-bit color conversion.The bit shift correctly converts from Go's 16-bit color values (returned by
RGBA()) to 8-bit values needed for hex format. Using theRGBShift8constant makes this conversion explicit.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/internal/model.go (1)
256-264: File panel width and max-count now consistently use shared constantsReplacing inline literals with
common.InnerPadding,common.BorderPadding, andcommon.FilePanelWidthUnitkeeps horizontal spacing andmaxFilePanelcomputation centralized; assuming the existing invariant of at least one file panel, the divisions remain safe and behaviorally equivalent.If
utils.FooterWidthstill defines its ownFilePanelMax, consider deduping or renaming one of theFilePanelMaxconstants incommonvsutilsin a future PR to avoid conceptual collision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/internal/model.go(7 hunks)src/internal/ui/preview/model.go(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 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: 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: 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`.
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-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/preview/model.gosrc/internal/model.go
📚 Learning: 2025-08-24T03:24:50.857Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:18-21
Timestamp: 2025-08-24T03:24:50.857Z
Learning: The superfile project uses github.com/charmbracelet/x/ansi as the standardized ANSI package and deprecates github.com/charmbracelet/x/exp/term/ansi to avoid binary bloat and potential incompatibilities.
Applied to files:
src/internal/ui/preview/model.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/preview/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/model.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.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/model.go
🧬 Code graph analysis (2)
src/internal/ui/preview/model.go (1)
src/internal/common/ui_consts.go (1)
DefaultPreviewTimeout(28-28)
src/internal/model.go (3)
src/internal/utils/ui_utils.go (3)
FilePanelMax(4-4)FullFooterHeight(17-22)FooterWidth(12-14)src/internal/common/ui_consts.go (11)
FilePanelMax(16-16)BorderPadding(11-11)InnerPadding(12-12)FilePanelWidthUnit(27-27)HeightBreakA(20-20)HeightBreakB(21-21)HeightBreakC(22-22)HeightBreakD(23-23)ResponsiveWidthThreshold(18-18)MinWidthForRename(17-17)CenterDivisor(35-35)src/internal/common/predefined_variable.go (2)
ModalWidth(30-30)ModalHeight(31-31)
⏰ 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: Build and Test (windows-latest)
🔇 Additional comments (13)
src/internal/ui/preview/model.go (1)
325-325: LGTM! Clean refactoring to eliminate magic number.The replacement of the hard-coded 500ms timeout with
common.DefaultPreviewTimeoutimproves maintainability by centralizing the timeout configuration, while preserving the existing behavior.src/internal/model.go (12)
237-240: Max file panel cap now aligned with shared UI constantClamping
m.fileModel.maxFilePanelagainstcommon.FilePanelMaxcentralizes the limit and keeps behavior consistent with other layout helpers that use the same cap.
243-245: Preview height tied to main panel height + border padding looks correctUsing
m.mainPanelHeight + common.BorderPaddinginstead of a raw+2keeps the preview height aligned with the main panel plus border and removes the magic number without changing behavior (givenBorderPadding == 2).
249-252: Preview width formula cleanly replaces inline padding literalsThe updated width calculation uses
common.InnerPaddingandcommon.BorderPaddingin place of hard-coded4and2, preserving the prior arithmetic while making sidebar/panel spacing easier to adjust in one place.
267-287: Footer height thresholds correctly moved toHeightBreak*constantsThe
setHeightValuesladder now compares againstcommon.HeightBreakA-Dinstead of raw numbers while keeping the assigned footer heights (6–10) unchanged, which should be a no-op behaviorally and satisfies MND.
295-307: Help menu sizing and search bar width now parametrized via UI constantsUsing
common.BorderPadding,common.HeightBreakB,common.ResponsiveWidthThreshold, andcommon.InnerPaddingto drive help menu dimensions andsearchBar.Widthlines up with the comment’s breakdown (padding + border + icon space + extra char) and avoids scattered literals.
310-316: Prompt modal fractional sizing with nolint comments is reasonableThe explicit
3and2divisors for prompt modal height/width are now documented with//nolint:mndexplaining the layout choice, which keeps the intent clear without introducing awkwardly named constants.
318-324: Zoxide modal fractional sizing matches prompt modal approachUsing
/2with//nolint:mndfor zoxide height and width mirrors the prompt modal’s layout decisions and keeps modal sizing semantics obvious while satisfying the linter.
326-337: Metadata and process bar dimensions now use shared border paddingAdding
common.BorderPaddingon top ofutils.FooterWidth(m.fullWidth)andm.footerHeightmatches the previous+2behavior while tying the extra border space to the same padding constant used elsewhere, so footer subpanels should still line up visually.
566-572: Rename-width guard now referencesMinWidthForRenameChecking
m.fileModel.width < common.MinWidthForRenameinstead of a bare18centralizes the minimum rename width and makes it easier to keep any other rename-related checks in sync.
617-653: Overlay centering unified viaCenterDivisorfor main modalsAll main overlays (help, prompt, zoxide, sort options, first-use intro) now compute
overlayX/overlayYusingcommon.CenterDivisorinstead of hard-coded/2, which standardizes centering logic and makes future tweaks to the centering strategy a one-const change.
656-667: Typing and notify modals also centered viaCenterDivisorand shared modal sizeUsing
common.CenterDivisorwithcommon.ModalWidth/ModalHeightkeeps the overlay math for these modals consistent with the rest of the overlay system while still relying on the existing modal size constants.
735-736: Re-render throttle now parameterized withReRenderChunkDivisorMoving the
100divisor intocommon.ReRenderChunkDivisorpreserves the existinglen(elements)/Nthrottling behavior and makes it straightforward to tune re-render frequency globally without hunting down literals.
- Convert exported constants to unexported (lowercase) for package-internal use - Keep BorderSize in processbar exported as it's used by metadata package - Update all references to use the new lowercase constant names - This improves encapsulation and prevents unintended external dependencies Affected packages: - ui/processbar: 3 constants made unexported - ui/prompt: 2 constants made unexported - ui/rendering: 5 constants made unexported - ui/sidebar: 3 constants made unexported - ui/zoxide: 2 constants made unexported - file_preview: 12 constants made unexported
Merged constants.go into const.go/consts.go for processbar, prompt, sidebar and zoxide packages
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/internal/ui/sidebar/consts.go (1)
18-28: Constants are well-defined and achieve the PR objective.The new constants properly eliminate magic numbers and are correctly unexported per package conventions. The detailed breakdown for
searchBarPadding(2 + 2 + 1 = 5) is particularly helpful for understanding the calculation.Optional: Consider the relationship between
defaultRenderHeightandsideBarInitialHeight.Both constants have the value 3, which may be coincidental or may indicate a shared underlying constraint:
sideBarInitialHeight(line 16): represents the initial static content heightdefaultRenderHeight(line 27): serves as a fallback when height is unavailableIf these represent the same fundamental minimum height for the sidebar, consider using a single constant or documenting their relationship. However, this is architectural polish and not critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/internal/ui/processbar/const.go(1 hunks)src/internal/ui/prompt/consts.go(1 hunks)src/internal/ui/sidebar/consts.go(1 hunks)src/internal/ui/zoxide/consts.go(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 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: 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: 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: 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`.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:32:06.922Z
Learning: When reviewing large refactoring PRs that change async patterns (like moving from goroutines to tea.Cmd), always check for incomplete refactoring where some call sites still use the old pattern while others use the new pattern, as this often leads to compilation errors and architectural inconsistencies.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 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-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/ui/zoxide/consts.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/zoxide/consts.gosrc/internal/ui/processbar/const.gosrc/internal/ui/sidebar/consts.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/ui/processbar/const.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/ui/processbar/const.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/const.go
📚 Learning: 2025-09-09T14:23:14.164Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/ui/rendering/truncate.go:6-6
Timestamp: 2025-09-09T14:23:14.164Z
Learning: The truncate test failure in src/internal/ui/rendering/truncate_test.go during the ANSI package migration from experimental to stable was caused by a truncation bug in the experimental package. The experimental package incorrectly truncated strings even when input length equaled maxWidth (e.g., "1234" with maxWidth=4 became "1..."), while the stable package correctly only truncates when input exceeds maxWidth. The test expectation was based on the buggy behavior and needs to be corrected.
Applied to files:
src/internal/ui/processbar/const.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.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.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/consts.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (windows-latest)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
src/internal/ui/zoxide/consts.go (1)
11-16: Good encapsulation of UI magic numbers into well‑named, unexported constants
scoreColumnWidthandmodalInputPaddingare clearly named, well-documented, and kept unexported, which matches the package‑scoped-constants preference and keeps UI layout math readable in call sites. No issues from my side.src/internal/ui/processbar/const.go (1)
12-23: New process bar layout constants look appropriate and well-scopedThe added constants (borderSize, progressBarRightPadding, processNameTruncatePadding, linesPerProcess) are clearly named, documented, and kept unexported, which fits their package-local layout role and aligns with the goal of removing magic numbers without widening API surface. No further changes needed here.
Based on learnings, ...
src/internal/ui/prompt/consts.go (1)
40-45: Prompt UI constants look good and keep MND changes scoped
promptInputPaddingandexpectedArgCountare well‑named, documented, and remove the remaining prompt magic numbers without adding complexity. Keeping these unexported and local to the prompt package fits the linting-focused scope of this PR. Based on learnings, this localized approach aligns with the preference for focused, non-overengineered refactors.
Description
This PR enables the MND (Magic Number Detector) linter and resolves all 54 magic number issues throughout the codebase.
Changes
Phase 1: String/Size Constants
KilobyteSize,KibibyteSize) and formatting (TabWidth)FormatFileSize()andCheckAndTruncateLineLengths()Phase 2: UI Constants
Phase 3: Image Preview Constants
Phase 4: Final Cleanup
//nolint:mndannotations for test dimensions and metadata display orderingRGBMask,AlphaOpaque)Statistics
Testing
go build -o bin/spf ./src/cmdgolangci-lint run.golangci.yamlNotes
Used
//nolint:mndjudiciously for:This improves code maintainability by replacing magic numbers with meaningful constants throughout the codebase.
Summary by CodeRabbit
Release Notes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.