-
-
Notifications
You must be signed in to change notification settings - Fork 417
chore: enable MND linter and fix all magic number issues #1180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
1b05a03
task details
lazysegtree eafc1ca
chore(mnd): add shared UI constants; apply to CLI, core model, render…
lazysegtree 93f4908
fix: Remove useless comments
lazysegtree 7cac44b
fix: golines
lazysegtree 694fce7
fix goline part 2
lazysegtree 283bd20
chore(mnd): add size/string constants for Phase 1
lazysegtree 517549d
chore(mnd): add Phase 2 internal UI constants
lazysegtree 8bffadf
fix: Fix minor lint issues
lazysegtree 6dcbe47
chore(mnd): add Phase 3 image preview constants
lazysegtree 845252b
chore(mnd): fix Priority 1 - metadata indices and processbar dimensions
lazysegtree 99d442e
chore(mnd): fix Priority 2 - UI component constants
lazysegtree 5a3dd8c
chore(mnd): complete MND linter remediation
lazysegtree 030297b
docs: update plan.md with complete MND remediation summary
lazysegtree 2226d14
fix: Fix minor lint issues
lazysegtree 428a6f8
Merge branch 'main' into add-mnd
lazysegtree e1cd016
refactor: make package-internal constants unexported
lazysegtree 6f3a090
fix: Update constants
lazysegtree 5486bb7
fix: consolidate duplicate constant files into existing const.go files
lazysegtree File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| # MND LINTER REMEDIATION PLAN - HANDOFF DOCUMENT | ||
|
|
||
| ## EXECUTIVE SUMMARY | ||
| **Progress: 100% Complete (54/54 issues resolved)** ✅ | ||
| - Started with 54 MND issues + 4 golines issues | ||
| - All 54 MND issues resolved ✅ | ||
| - All golines issues resolved ✅ | ||
| - MND linter enabled in golangci.yaml ✅ | ||
| - 4 commits total | ||
|
|
||
| ## COMPLETED WORK ✅ | ||
|
|
||
| ### Phase 1: String/Size Constants (Commit: 283bd20) | ||
| - Created constants in `src/internal/common/string_function.go`: | ||
| - `KilobyteSize = 1000` (SI decimal) | ||
| - `KibibyteSize = 1024` (binary) | ||
| - `TabWidth = 4` (tab expansion) | ||
| - Applied in `FormatFileSize()` and `CheckAndTruncateLineLengths()` | ||
| - Resolved 5 MND issues | ||
|
|
||
| ### Phase 2: Internal UI Constants (Commit: 517549d) | ||
| - Extended `src/internal/common/ui_consts.go` with: | ||
| - `DefaultFilePanelWidth = 10` | ||
| - `ExtractedFileMode = 0644`, `ExtractedDirMode = 0755` | ||
| - `CenterDivisor = 2` (for UI centering math) | ||
| - Applied across 7 files: | ||
| - Fixed 14 centering operations in `model.go` | ||
| - Updated panel calculations in `handle_panel_navigation.go` | ||
| - Fixed width calculations in `model_render.go` | ||
| - Resolved ~10 MND issues | ||
|
|
||
| ### Phase 3: Image Preview Constants (Commit: 6dcbe47) | ||
| - Created `src/pkg/file_preview/constants.go` with: | ||
| - Cache: `DefaultThumbnailCacheSize = 100` | ||
| - RGB operations: `RGBShift16 = 16`, `RGBShift8 = 8`, `HeightScaleFactor = 2` | ||
| - Kitty protocol: `KittyHashSeed = 42`, `KittyHashPrime = 31`, `KittyMaxID = 0xFFFF` | ||
| - Added Windows-specific pixel constants in `utils.go`: | ||
| - `WindowsPixelsPerColumn = 8`, `WindowsPixelsPerRow = 16` | ||
| - Preserved original defaults (10/20) to avoid regression | ||
| - Applied `//nolint:mnd` for EXIF orientation values (industry standard 1-8) | ||
| - Resolved 14 MND issues | ||
|
|
||
| ### Golines Formatting (Throughout) | ||
| - Fixed 4 long-line issues in `cmd/main.go`, `model.go`, `handle_file_operations.go`, `function.go` | ||
| - Extracted lipgloss styles for cleaner code | ||
|
|
||
| ### Phase 4: Final Cleanup (Commit 4: 5a3dd8c) | ||
| - Fixed remaining 7 MND issues: | ||
| - Added `//nolint:mnd` to metadata sort indices (display order) | ||
| - Added `//nolint:mnd` to test dimensions in zoxide test_helpers | ||
| - Added RGB constants: `RGBMask = 0xFF`, `AlphaOpaque = 255` | ||
| - Fixed import cycle in utils/ui_utils.go by duplicating constants | ||
| - Fixed missing processbar import in metadata/model.go | ||
| - Fixed golines formatting in sidebar/render.go | ||
| - Enabled MND linter in .golangci.yaml | ||
| - Resolved final 7 MND issues | ||
|
|
||
| ### Golines Formatting (Throughout) | ||
| - Fixed 5 long-line issues including sidebar/render.go | ||
| - All golines issues resolved ✅ | ||
|
|
||
| ## REMAINING WORK | ||
|
|
||
| ✅ **ALL MND ISSUES RESOLVED!** | ||
|
|
||
| - All 54 MND issues fixed | ||
| - MND linter enabled in `.golangci.yaml` | ||
| - Lint passes cleanly with 0 issues | ||
|
|
||
| ## FINAL STATISTICS | ||
|
|
||
| - **Total Issues Resolved**: 54 MND + 5 golines = 59 total | ||
| - **Commits**: 4 focused commits | ||
| - **Files Modified**: 18 files | ||
| - **Constants Created**: ~30 new constants across multiple packages | ||
| - **Lint Status**: Clean (0 issues) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| MND LINTER REMEDIATION PLAN (HIGHLY DETAILED) | ||
|
|
||
| Objective | ||
| - Enable and satisfy the mnd (magic number) linter across the codebase. | ||
| - Replace repeated numeric literals with named constants. | ||
| - Add targeted //nolint:mnd with concise justification where constants reduce clarity. | ||
| - Preserve behavior 100%. No feature changes. | ||
|
|
||
| Prerequisites | ||
| - Ensure golangci-lint is available (already present: v2.5.0). | ||
| - Work on a feature branch (e.g., mnd-remediation). | ||
| - Build baseline: CGO_ENABLED=0 go build -o bin/spf ./src/cmd | ||
| - Lint baseline: golangci-lint run --enable=mnd | ||
|
|
||
| Current Progress (executed) | ||
| - Added shared constants: src/internal/common/ui_consts.go (HelpKeyColumnWidth, DefaultCLIContextTimeout, PanelPadding, BorderPadding, InnerPadding, FooterGroupCols, FilePanelMax, MinWidthForRename, ResponsiveWidthThreshold, HeightBreakA–D, ReRenderChunkDivisor, FilePanelWidthUnit, DefaultPreviewTimeout). | ||
| - CLI updates: src/cmd/main.go uses HelpKeyColumnWidth and DefaultCLIContextTimeout; long fmt.Printf lines reformatted. | ||
| - Core model/layout: src/internal/model.go replaced hardcoded 10/18/95/height breaks; applied BorderPadding/InnerPadding; SetDimensions + preview height updated; searchBar widths use InnerPadding. | ||
| - Panel nav: src/internal/handle_panel_navigation.go width math updated to constants; preview height uses BorderPadding; searchBar width uses InnerPadding; max panels uses FilePanelWidthUnit. | ||
| - Rendering: src/internal/model_render.go applied paddings, truncation widths, footer group cols, and window padding constants. | ||
| - Sorting utils: src/internal/function.go panelElementHeight uses PanelPadding; regex submatch check annotated with //nolint:mnd. | ||
| - Modal/help: src/internal/handle_modal.go switched to InnerPadding for calculations; removed redundant inline comments. | ||
| - Panel movement: src/internal/handle_panel_movement.go searchBar width uses InnerPadding; imported common. | ||
| - Preview: src/internal/ui/preview/model.go timeout uses DefaultPreviewTimeout. | ||
| - Build validated after each batch. | ||
|
|
||
| Outstanding Work (next up) | ||
| - Fix remaining golines long-line warnings (a few lines in cmd/main.go, function.go, handle_file_operations.go, model.go). | ||
| - Replace or justify remaining mnd sites in internal: | ||
| - src/internal/default_config.go: width: 10 → constant or //nolint:mnd. | ||
| - Overlay math (/2 occurrences) in src/internal/model.go → consider //nolint:mnd with rationale or new constants. | ||
| - src/internal/type_utils.go: layout check uses +2 → replace with BorderPadding. | ||
| - src/internal/utils/ui_utils.go: replace /3 and +2 with constants or add //nolint with rationale. | ||
| - Preview/image and kitty packages: | ||
| - src/pkg/file_preview/image_preview.go: 100, 5*time.Minute, /2, bit shifts/masks → constants with brief doc; selectively //nolint where clearer. | ||
| - src/pkg/file_preview/image_resize.go: switch cases 2..8 → add concise //nolint:mnd per case (quality mapping) or local constants. | ||
| - src/pkg/file_preview/kitty.go: 42, 31, 0xFFFF, +1000 → constants. | ||
| - src/pkg/file_preview/utils.go: PixelsPerColumn/Row → constants. | ||
|
|
||
| Open Lint Items (from latest run) | ||
| - golines (formatting): | ||
| - src/cmd/main.go (fmt.Printf block), src/internal/function.go (regex nolint line), src/internal/handle_file_operations.go (rename line), src/internal/model.go (SetDimensions line). | ||
| - mnd (magic numbers) remain in: | ||
| - src/internal/default_config.go, src/internal/handle_panel_navigation.go (some math remnants), src/internal/model.go (overlay /2), src/internal/model_render.go (counts and mins), src/internal/type_utils.go (+2), src/internal/utils/ui_utils.go (/3, +2), and several pkg/file_preview/* files including kitty.go and image_resize.go. | ||
|
|
||
| Handoff Notes | ||
| - Behavior unchanged; only constants and formatting so far. | ||
| - Keep diffs focused; avoid unrelated reformatting. | ||
| - After finishing internal mnd items, handle preview/kitty in a separate commit for easier review. | ||
|
|
||
| Step 1 — Enable mnd in existing linter config (uncomment only) | ||
| 1. File to edit: .golangci.yaml (do NOT add .golangci.yml). | ||
| 2. In the `linters.enable` list, locate the commented mnd entry: | ||
| `# - mnd # detects magic numbers` and UNCOMMENT it to: `- mnd # detects magic numbers`. | ||
| - Location: around lines 100–115 in .golangci.yaml, under `linters: enable:`. | ||
| 3. Do not change the existing `mnd:` settings block (already present at ~line 356). Keep it as-is. | ||
| 4. Exclusions: follow the repo’s current convention only if needed. | ||
| - If tests begin flagging noisy mnd hits, append `mnd` to the existing per-path exclusion rule for tests: | ||
| `.golangci.yaml -> exclusions.rules -> - path: '_test\.go' -> linters: [ … add mnd here … ]`. | ||
| - Keep order/format consistent with current style. Do not add new exclusion sections. | ||
|
|
||
| Step 2 — Add shared UI/layout constants | ||
| 1. Create file: src/internal/common/ui_consts.go | ||
| 2. Add the following constants with comments: | ||
| - HelpKeyColumnWidth = 55 // width of help key column in CLI help | ||
| - DefaultCLIContextTimeout = 5 * time.Second // default CLI context timeout | ||
| - PanelPadding = 3 // rows reserved around file list for borders/header/footer | ||
| - BorderPadding = 2 // rows/cols for outer border frame | ||
| - InnerPadding = 4 // cols for inner content padding (truncate widths) | ||
| - FooterGroupCols = 3 // columns per group in footer layout math | ||
| - FilePanelMax = 10 // max number of file panels supported | ||
| - MinWidthForRename = 18 // minimal width for rename input to render | ||
| - ResponsiveWidthThreshold = 95 // width breakpoint for layout behavior | ||
| - HeightBreakA = 30; HeightBreakB = 35; HeightBreakC = 40; HeightBreakD = 45 // stacked height breakpoints | ||
| - ReRenderChunkDivisor = 100 // divisor for re-render throttling | ||
| 3. Import time at file top. Ensure package is common. | ||
|
|
||
| Step 3 — CLI fixes (src/cmd/main.go) | ||
| 1. Replace 55 in fmt.Printf("%-*s %s\n", 55, ...) with common.HelpKeyColumnWidth in lines printing colored help entries (approx lines 52, 54, 56, 58, 60). | ||
| - Add above the first print: // use shared help column width (mnd) | ||
| 2. Replace 5*time.Second in context.WithTimeout(..., 5*time.Second) with common.DefaultCLIContextTimeout (approx line 270). | ||
| - Add comment: // shared CLI timeout (mnd) | ||
|
|
||
| Step 4 — Core model/layout (src/internal/model.go) | ||
| 1. Replace literal 10 with common.FilePanelMax for file panel max check (approx line 237). | ||
| 2. Replace height threshold literals: | ||
| - if height < 30 → if height < common.HeightBreakA | ||
| - else if height < 35 → else if height < common.HeightBreakB | ||
| - else if height < 40 → else if height < common.HeightBreakC | ||
| - else if height < 45 → else if height < common.HeightBreakD | ||
| - if m.fullHeight > 35 → > common.HeightBreakB | ||
| - if m.fullWidth > 95 → > common.ResponsiveWidthThreshold | ||
| - Add comment near block: // responsive layout breakpoints (mnd) | ||
| 3. Replace +2 with +common.BorderPadding for: | ||
| - m.fileModel.filePreview.SetHeight(m.mainPanelHeight + 2) | ||
| - Bottom/footer widths where +2 is used (search for SetHeight/SetDimensions with +2). | ||
| 4. Replace /3, /2 usages for modal sizing: | ||
| - m.promptModal.SetMaxHeight(m.fullHeight / 3) | ||
| - m.promptModal.SetWidth(m.fullWidth / 2) | ||
| - m.zoxideModal.SetMaxHeight(m.fullHeight / 2) | ||
| - m.zoxideModal.SetWidth(m.fullWidth / 2) | ||
| Options: | ||
| - Prefer constants ModalThird=3, ModalHalf=2 in common; OR | ||
| - Keep divisions and add //nolint:mnd with comment “modal uses thirds/halves”. | ||
| 5. Replace -4 and -3 style paddings using constants when adjusting widths/heights in render/layout helpers: | ||
| - Use common.InnerPadding for -4 | ||
| - Use common.PanelPadding for -3 | ||
| 6. Replace m.fileModel.width < 18 with < common.MinWidthForRename (approx line 566). Add comment. | ||
| 7. Replace reRenderTime := int(float64(len(...))/100) with /common.ReRenderChunkDivisor (approx line 731). Add comment. | ||
|
|
||
| Step 5 — Panel navigation & operations | ||
| - src/internal/handle_panel_navigation.go: replace +2 with +common.BorderPadding when setting preview height (approx line 111). Add comment. | ||
| - src/internal/handle_file_operations.go: replace width-4 with width-common.InnerPadding when creating rename input (approx line 100). Add comment. | ||
|
|
||
| Step 6 — Rendering (src/internal/model_render.go) | ||
| 1. Replace +2 with +common.BorderPadding at: | ||
| - FilePanelRenderer(mainPanelHeight+2, filePanelWidth+2, …) (approx line 65) | ||
| - ClipboardRenderer(m.footerHeight+2, bottomWidth+2) (approx line 217) | ||
| 2. Replace filePanelWidth-4 with filePanelWidth-common.InnerPadding (approx line 77) | ||
| 3. Replace bottom width calc utils.FooterWidth(m.fullWidth + m.fullWidth%3 + 2): | ||
| - Use %common.FooterGroupCols | ||
| - Replace +2 with +common.BorderPadding (approx line 213) | ||
| 4. Replace -3 when truncating display width within metadata/preview draws with -common.PanelPadding (approx line 236). Add comment. | ||
| 5. Replace ModalWidth-4 with ModalWidth-common.InnerPadding (approx line 297). | ||
| 6. Replace panel.sortOptions.width-2 with -common.BorderPadding (approx line 457). | ||
|
|
||
| Step 7 — Directory listing & sorting (src/internal/function.go) | ||
| 1. func panelElementHeight(mainPanelHeight int) int { return mainPanelHeight - 3 } | ||
| - Replace 3 with common.PanelPadding (approx line 244). Add comment. | ||
| 2. In suffixRegexp.FindStringSubmatch(name); len(match) == 3 (approx line 294): | ||
| - Keep 3 and add inline: //nolint:mnd — 3 = full match + 2 capture groups (regex) | ||
|
|
||
| Step 8 — Preview subsystem | ||
| - src/internal/ui/preview/model.go: replace 500*time.Millisecond with a named constant. | ||
| Options: | ||
| - Add in common: DefaultPreviewTimeout = 500*time.Millisecond | ||
| - Or local const in preview package with comment. | ||
| Add comment: // preview operation timeout (mnd) | ||
|
|
||
| Step 9 — Image preview utils (src/pkg/file_preview/image_preview.go) | ||
| 1. Replace 100 with DefaultThumbnailWidth (const in this file). Comment: // default thumb width (mnd) | ||
| 2. Replace 5*time.Minute with DefaultCacheExpiration. Comment. | ||
| 3. Replace /2 on ticker with either const DividerTwo or //nolint:mnd “half expiration interval”. | ||
| 4. Replace 16, 8, 0xFF, 255 with named consts: Shift16, Shift8, MaskFF, OpaqueAlpha; add comment block explaining RGB math. | ||
| 5. Replace 8 in fmt.Sprintf("#%02x%02x%02x", uint8(r>>8), …) with Shift8. | ||
|
|
||
| Step 10 — Image resize (src/pkg/file_preview/image_resize.go) | ||
| 1. Switch cases 2..8: | ||
| - Prefer //nolint:mnd on each case with comment: // 2=low … 8=ultra quality levels | ||
| - Or introduce Quality2..Quality8 if reused elsewhere. | ||
| 2. imaging.Fit(img, maxWidth, maxHeight*2, …): replace 2 with HeightScaleFactor const or //nolint:mnd with comment “fit scales height x2”. | ||
|
|
||
| Step 11 — Kitty utils (src/pkg/file_preview/kitty.go) | ||
| 1. Replace 42 with KittyDefaultSeed | ||
| 2. Replace 31 with HashPrime | ||
| 3. Replace 0xFFFF with MaskFFFF | ||
| 4. Replace +1000 with NonZeroOffset | ||
| 5. Add one-line comments next to const block. | ||
|
|
||
| Step 12 — Preview terminal metrics (src/pkg/file_preview/utils.go) | ||
| 1. Replace PixelsPerColumn: 8, PixelsPerRow: 16 with named consts (PixelsPerColumnDefault, PixelsPerRowDefault) and comment on typical terminal cell sizes. | ||
|
|
||
| Step 13 — External path detection (optional cleanup) | ||
| - src/internal/function.go:isExternalDiskPath | ||
| - Create consts: TimeMachinePrefix, VolumesPrefix, MediaPrefixes (slice) | ||
| - Use them in HasPrefix checks. Rationale: clarity; not necessarily for mnd. | ||
|
|
||
| Step 14 — Commenting & //nolint practices | ||
| - Only add //nolint:mnd where constants reduce clarity or are inherently part of API/math: | ||
| - Regex submatch count (len(match)==3): add concise reason | ||
| - Switch cases for fixed, human-defined quality levels (2..8): add mapping comment | ||
| - Simple halves/thirds if not centralized: add “half/third sizing” comments | ||
| - For every //nolint:mnd, add a short, explicit justification on the same line. | ||
|
|
||
| Validation Checklist | ||
| 1. golangci-lint run --enable=mnd — should show a decreasing count; iterate until 0 or only justified //nolint sites remain. | ||
| 2. Build: CGO_ENABLED=0 go build -o bin/spf ./src/cmd | ||
| 3. Tests: run focused suites to ensure no behavior change | ||
| - go test ./src/internal -run '^TestInitialFilePathPositionsCursor|TestInitialFilePathPositionsCursorWindow$' | ||
| - go test ./src/internal -run '^TestReturnDirElement$' | ||
| 4. Manual smoke: | ||
| - Launch spf in a directory with many files; verify layout unchanged. | ||
| - Open with a file path; ensure cursor targets file and remains visible. | ||
|
|
||
| Commit Strategy | ||
| - Commit 1: Add ui_consts.go and .golangci.yml mnd enablement. | ||
| - Commit 2: Apply constants to src/cmd and internal model/layout. | ||
| - Commit 3: Rendering replacements. | ||
| - Commit 4: Preview and image utils (with //nolint where needed). | ||
| - Commit 5: Kitty utils and optional path-prefix constants. | ||
| - Keep each commit small and scoped; include brief messages referencing mnd. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.