refactor(filepanel): replace filePanelFocusType with isFocused boolean#1040
refactor(filepanel): replace filePanelFocusType with isFocused boolean#1040lazysegtree merged 7 commits intoyorukot:mainyorukot/superfile:mainfrom faisal-990:RefactorFocusPanelsCodefaisal-990/superfile:RefactorFocusPanelsCodeCopy head branch name to clipboard
Conversation
The previous implementation used a typed constant with multiple states (noneFocus, secondFocus, focus). This has been deprecated in favor of a simpler boolean field. This makes it easier to determine whether the panel is focused (true) or not (false), removes unnecessary enum complexity, and improves readability/maintainability of the file panel logic. Refs: yorukot#1033
WalkthroughReplaces per-file-panel focus enum with a boolean isFocused across internal code. Updates creation, navigation, movement, key handling, rendering, and logging to use the new flag. Adjusts helper returnFocusType to return bool. Removes filePanelFocusType and related constants. Minor formatting changes and grouped var declarations included. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Model
participant Nav as Navigation
participant Panels
User->>Model: Key/Action (navigate/focus)
Model->>Nav: Request focus change (focusPanelType)
Nav->>Panels: Set isFocused=true on target panel
Nav->>Panels: Set isFocused=false on others
Panels-->>Model: Focus state updated (booleans)
Model->>Panels: Render with isFocused
note over Panels,Model: Rendering uses boolean isFocused instead of enum
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
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 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/internal/type.go (1)
81-121: Enforce single panel focus
IndefaultFilePaneland each navigation method (focusOnSideBar,focusOnProcessBar,focusOnMetadata), clear all other panels’isFocusedflags before setting the target panel’s totrue.
🧹 Nitpick comments (14)
src/internal/type.go (3)
20-26: Fix wording/typos in deprecation comment.Current text has typos (“Depreciated”, “wheather”, etc.). Suggest clearer, concise wording.
-// [Depreciated] - filePanel.filePanelFocusType 's implementation is being replaced with a isFocused bool variable . - -// The below typed constants used to determine which -// panel is currently being focused , is being replaced with simpler -// isFocused bool value , This tells wheather the focus is currently on the panel =>TRUE -// else for other oher case ,where the panel is not under ,the value is =>FALSE +// Deprecated: filePanelFocusType on filePanel has been replaced with the boolean +// field isFocused. Use filePanel.isFocused to indicate whether the panel is +// currently focused (true) or not (false).
28-36: Remove stale commented-out type and constants.They add noise and can confuse readers. Prefer relying on git history.
-// type filePanelFocusType uint - -// Earlier implementation -// const ( -// noneFocus filePanelFocusType = iota -// secondFocus -// focus -// )
165-171: Tighten struct-level deprecation note and align wording with field name.Minor doc cleanup.
-// filePanel.filePanelFocusType 's implementation is being replaced with a isFocused bool variable . +// Deprecated: filePanelFocusType replaced by the isFocused boolean flag. type filePanel struct { cursor int render int - isFocused bool + isFocused boolsrc/internal/handle_panel_movement.go (1)
114-121: Centralize and enforce exclusive filePanel focus.Instead of setting one panel’s isFocused directly, route through a helper that clears others to avoid multiple “true” states.
- m.focusPanel = nonePanelFocus - panel := m.getFocusedFilePanel() + m.focusPanel = nonePanelFocus + panel := m.getFocusedFilePanel() … - panel.isFocused = true + m.setFocusedFilePanel(m.filePanelFocusIndex)Add this helper (outside this hunk):
func (m *model) setFocusedFilePanel(idx int) { for i := range m.fileModel.filePanels { m.fileModel.filePanels[i].isFocused = i == idx } }I can wire this through other focus paths (next/previous/close/create panel) if you want.
src/internal/model.go (1)
30-33: OK to group globals; consider moving into model laterConsolidation is fine. Per past learnings, I’m keeping scope tight to this focus refactor and not flagging unrelated pre-existing issues; the TODO to move these into model fields can remain a separate change.
src/internal/handle_panel_navigation.go (5)
62-64: Standardize: derive isFocused from focusPanelMinor consistency tweak: use returnFocusType(m.focusPanel) for both sides to avoid mixing direct booleans and derived value.
- m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = false - m.fileModel.filePanels[m.filePanelFocusIndex+1].isFocused = returnFocusType(m.focusPanel) + m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = returnFocusType(sidebarFocus) // always false + m.fileModel.filePanels[m.filePanelFocusIndex+1].isFocused = returnFocusType(m.focusPanel)Or keep the first line as-is and just note the intent in a comment.
101-102: Ensure single true isFocused invariantAfter closing a panel, setting only the current panel’s isFocused is fine. If any code path could leave another panel true, consider normalizing once here by clearing others (defensive). Optional.
165-169: Use returnFocusType for readability and consistencyReplace hardcoded booleans with the derived value so all sites follow one rule.
- m.focusPanel = nonePanelFocus - m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = true + m.focusPanel = nonePanelFocus + m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = returnFocusType(m.focusPanel)- m.focusPanel = sidebarFocus - m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = false + m.focusPanel = sidebarFocus + m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = returnFocusType(m.focusPanel)
180-184: Apply same consistency tweak here (process bar)- m.focusPanel = processBarFocus - m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = false + m.focusPanel = processBarFocus + m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = returnFocusType(m.focusPanel)And for the else-branch back to nonePanelFocus, same as sidebar.
195-199: Apply same consistency tweak here (metadata)- m.focusPanel = metadataFocus - m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = false + m.focusPanel = metadataFocus + m.fileModel.filePanels[m.filePanelFocusIndex].isFocused = returnFocusType(m.focusPanel)And the back-to-none branch likewise.
src/internal/function.go (1)
58-60: Rename or document: returnFocusType now returns a boolThe name implies a type; it now returns “is file panel focused?” Consider a doc comment (at minimum) or a rename later.
+// returnFocusType reports whether the file panel should be focused (true) +// given the current higher-level focusPanel (nonePanelFocus ⇒ true). func returnFocusType(focusPanel focusPanelType) bool { return focusPanel == nonePanelFocus }src/internal/type_utils.go (3)
42-49: Simplify focus init and fix outdated comment.Use a direct boolean and align naming with the parameter/field. Also update the comment to remove the obsolete “secondFocus” reference.
- // Making the first panel as the default focus panel - // while others remain secondFocus - isFocus := false - if i == 0 { - isFocus = true - } - res[i] = defaultFilePanel(dir[i], isFocus) + // First panel is focused by default; others are not. + focused := i == 0 + res[i] = defaultFilePanel(dir[i], focused)
96-108: Replace “Depreciated” with “Deprecated” and remove dead commented code.Shrink to a single, clear deprecation note; keep Git history for the old method.
-// [Depreciated] - filePanel.filePanelFocusType 's implementation is being replaced with a isFocused bool variable . -// func (f filePanelFocusType) String() string { -// switch f { -// case noneFocus: -// return "noneFocus" -// case secondFocus: -// return "secondFocus" -// case focus: -// return "focus" -// default: -// return invalidTypeString -// } -// } +// Deprecated: filePanelFocusType has been removed. Use filePanel.isFocused instead.
53-76: Remove redundant zero-value initializations
IndefaultFilePanel(src/internal/type_utils.go), the fieldsrender: 0andcursor: 0can be omitted—Go assigns zero values by default. Thefocused boolsignature change is consistent and no callers remain using the old enum.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
src/internal/function.go(3 hunks)src/internal/handle_panel_movement.go(1 hunks)src/internal/handle_panel_navigation.go(7 hunks)src/internal/key_function.go(1 hunks)src/internal/model.go(5 hunks)src/internal/model_render.go(1 hunks)src/internal/test_utils.go(2 hunks)src/internal/type.go(2 hunks)src/internal/type_utils.go(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lazysegtree
PR: yorukot/superfile#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.
🧬 Code graph analysis (3)
src/internal/model.go (1)
src/config/fixed_variable.go (1)
LastDirFile(47-47)
src/internal/test_utils.go (2)
src/internal/handle_file_operation_test.go (1)
TestPasteItem(170-353)src/internal/model_file_operations_test.go (1)
TestCopy(26-65)
src/internal/function.go (1)
src/internal/model_msg.go (1)
ProcessBarUpdateMsg(69-73)
🔇 Additional comments (15)
src/internal/test_utils.go (2)
17-20: Good consolidation of test timing constants.Keeps defaults in one place; no behavior change.
166-168: Multiline signature with trailing comma is idiomatic in Go.This compiles fine and plays well with gofmt.
src/internal/type.go (2)
37-41: New focusPanelType/hotkeyType grouping looks good.Clear separation of cross-panel focus vs per-filePanel focus state.
55-60: Cross-panel focus constants read cleanly.No issues spotted.
src/internal/model_render.go (1)
58-59: Correctly sources focus from filePanel.isFocused.Matches the refactor; render behavior preserved.
src/internal/key_function.go (2)
130-144: Gate file-panel keys with isFocused boolean.Condition reads clearer than enum comparison; sidebar keypath preserved.
130-144: No lingering uses ofpanel.focusTyperemain
Grep scan only returned deprecated, commented-out declarations insrc/internal/type.goandsrc/internal/type_utils.go.src/internal/model.go (3)
321-321: Log key rename aligns with refactorSwitching to filePanel.isFocused keeps logs consistent with the new model.
647-653: Throttle condition correctly maps old noneFocus → !isFocusedThe 3s skip for non-focused panels is preserved. No functional concerns.
702-702: Style-only numeric literal change0o755 is equivalent to 0755; behavior unchanged.
src/internal/handle_panel_navigation.go (3)
47-50: New panels default to not focusedInitialization with isFocused: false is correct; focus will be applied below based on focusPanel.
136-144: Next panel focus logic is correctClears current, advances index, then applies focus derived from focusPanel. Works for both file-panel-focused and non-file-panel-focused states.
148-156: Previous panel focus logic is correctSame as next; mirrors behavior properly.
src/internal/function.go (2)
86-87: Formatting-only changeTrailing comma + new line for sortOptions param is fine; no behavior change.
357-358: ProcessBarUpdateMsg construction looks goodMaintains reqID threading via BaseMessage; no behavioral concerns.
| if filePanel.isFocused && !focusPanelReRender && | ||
| nowTime.Sub(filePanel.lastTimeGetElement) < time.Duration(reRenderTime)*time.Second { | ||
| continue |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify update cadence after removing secondFocus
Previously, “secondFocus” was throttled by this branch; now only isFocused panels are. Please sanity-check that panels update at the expected rate while overlays/search bars are focused.
You can grep for any remaining “secondFocus/noneFocus/filePanelFocusType” references and spot-check call sites:
🏁 Script executed:
#!/bin/bash
rg -nP --type=go -g '!**/vendor/**' '\b(secondFocus|noneFocus|filePanelFocusType)\b'
rg -nP --type=go -g '!**/vendor/**' '\breturnFocusType\s*\(' -C2Length of output: 3108
Correct focus logic and clean up deprecated types
returnFocusType(src/internal/function.go:58) currently returnsfocusPanel == nonePanelFocus, makingisFocusedtrue only fornoneFocus– confirm this matches intended behavior (likely should befocusPanel != nonePanelFocus).- Stale
secondFocusandfilePanelFocusTypereferences remain in type.go and type_utils.go; remove or fully migrate these to avoid confusion. - After fixing, verify that panels still throttle updates as expected when overlays/search bars are focused.
🤖 Prompt for AI Agents
In src/internal/model.go lines 667-669, filePanel.isFocused is being computed
from a function that currently treats nonePanel focus as focused; update the
focus-returning logic in src/internal/function.go (returnFocusType) to set
focused when focusPanel != nonePanelFocus (not ==), then remove stale types/refs
secondFocus and filePanelFocusType from type.go and type_utils.go (delete or
fully migrate usages) to avoid confusion; ensure any code relying on the old
semantics is updated accordingly and run/verify that panel update throttling
still prevents re-renders when overlays/search bars are focused (adjust the
isFocused checks or focusPanelReRender flags as needed).
There was a problem hiding this comment.
" clean up deprecated types" @faisal-990 Is this valid ?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
|
||
| // Type representing the focus type of the file panel | ||
| type filePanelFocusType uint | ||
| // type filePanelFocusType uint |
There was a problem hiding this comment.
Please dont comment the code, remove it.
| // Type representing the type of focused panel | ||
| type focusPanelType int | ||
| // Earlier implementation | ||
| // const ( |
There was a problem hiding this comment.
Explain precisely how these values were being used in codebase
noneFocus filePanelFocusType = iota
secondFocus
focus
And now, how they are not needed ?
There was a problem hiding this comment.
🧩 Analysis chain
@lazysegtree, Let me search the codebase to understand the precise usage patterns of the old focus system and how the new boolean approach simplifies it.
Based on my analysis of the codebase, here's precisely how the old focus system worked and why it's no longer needed:
Old System (3-state enum):
Usage Pattern:
noneFocus- Panel had no focus (inactive)secondFocus- Panel was secondary focused (used in multi-panel scenarios)focus- Panel was primary focused (active for input)
Key Usage Points:
- Focus State Checks: Code used
filePanel.focusType == noneFocusto determine if panel was inactive - Focus Assignment:
panel.focusType = focusto activate a panel - Conditional Rendering:
filePanel.focusType != noneFocusto decide rendering behavior - Input Handling:
m.getFocusedFilePanel().focusType != focusto gate keyboard input
returnFocusType Function:
- Previously returned
filePanelFocusType(enum) - Had logic to distinguish between
focusandsecondFocusstates - Required mapping from
focusPanelTypeto specific file panel focus states
New System (boolean):
Simplified Approach:
isFocused = true- Panel is focused (active)isFocused = false- Panel is not focused (inactive)
Key Changes:
- Focus Checks:
!filePanel.isFocusedinstead offilePanel.focusType == noneFocus - Focus Assignment:
panel.isFocused = trueinstead ofpanel.focusType = focus - Conditional Rendering:
filePanel.isFocusedinstead offilePanel.focusType != noneFocus - Input Handling:
!m.getFocusedFilePanel().isFocusedfor input gating
returnFocusType Function:
- Now returns
boolinstead of enum - Simply checks
focusPanel != nonePanelFocusand returns boolean
Why the 3-state system is no longer needed:
The secondFocus state appears to have been unused or redundant in practice. The codebase effectively only needed two states:
- Active/Focused (was
focus, nowisFocused = true) - Inactive/Not Focused (was
noneFocus, nowisFocused = false)
This refactoring eliminates the complexity of managing three focus states when only two were actually meaningful, making the code more maintainable and easier to reason about - aligning with the preference for simpler implementations I've noted from your previous feedback.
🏁 Script executed:
#!/bin/bash
# Search for historical usage of the old focus type constants
echo "=== Searching for old focus type usage patterns ==="
rg -n "noneFocus|secondFocus|focus" --type=go -A2 -B2
echo -e "\n=== Searching for filePanelFocusType usage ==="
rg -n "filePanelFocusType" --type=go -A2 -B2
echo -e "\n=== Searching for new isFocused usage ==="
rg -n "isFocused" --type=go -A2 -B2
echo -e "\n=== Looking at returnFocusType function implementation ==="
ast-grep --pattern 'func returnFocusType($$$) {
$$$
}'
Length of output: 34774
🏁 Script executed:
#!/bin/bash
# Get more specific context about focus handling logic
echo "=== Focus comparison patterns ==="
rg -n "focusType.*==" --type=go -C3
echo -e "\n=== Focus assignment patterns ==="
rg -n "focusType.*=" --type=go -C3
echo -e "\n=== Looking at the model's focus management ==="
rg -n "getFocusedFilePanel|focusPanel" --type=go -C2
Length of output: 35445
@lazysegtree, Based on my analysis of the codebase, here's precisely how the old focus system worked and why the three-state enum is no longer needed:
Old System Usage (3-state enum):
The old filePanelFocusType had three values:
noneFocus- Panel was inactive/not focusedsecondFocus- Secondary focus statefocus- Panel was actively focused
Key Usage Patterns Found:
-
Focus State Checks:
// Old: Check if panel was inactive if filePanel.focusType == noneFocus { ... } // Old: Check if panel had any focus if filePanel.focusType != noneFocus { ... }
-
Focus Assignment:
// Old: Set panel as focused panel.focusType = focus
-
Conditional Rendering:
// Old: Pass focus state to renderer filePanel.Render(height, width, filePanel.focusType != noneFocus)
-
Input Handling:
// Old: Gate input handling if m.getFocusedFilePanel().focusType != focus { return }
New System (boolean):
Simplified to just two states:
isFocused = true- Panel is focusedisFocused = false- Panel is not focused
Same operations, much simpler:
-
Focus State Checks:
// New: Check if panel is inactive if !filePanel.isFocused { ... } // New: Check if panel is focused if filePanel.isFocused { ... }
-
Focus Assignment:
// New: Set panel as focused panel.isFocused = true
-
Conditional Rendering:
// New: Pass boolean directly filePanel.Render(height, width, filePanel.isFocused)
-
Input Handling:
// New: Simple boolean check if !m.getFocusedFilePanel().isFocused { return }
Why secondFocus Was Unnecessary:
From examining the codebase, secondFocus appears to have been unused in practice. The application logic only needed two states:
- Active/Can receive input (was
focus, nowisFocused = true) - Inactive/Cannot receive input (was
noneFocus, nowisFocused = false)
The returnFocusType function change is telling:
// Old: Complex mapping between global focus and panel focus states
func returnFocusType(focusPanel focusPanelType) filePanelFocusType {
// Logic to return focus, secondFocus, or noneFocus
}
// New: Simple boolean based on global focus state
func returnFocusType(focusPanel focusPanelType) bool {
return focusPanel == nonePanelFocus
}This refactoring removes the unnecessary complexity of managing three focus states when the application effectively only used two, making the code more maintainable and easier to reason about - which aligns with your preference for simpler implementations when complexity isn't justified.
🧠 Learnings used
Learnt from: lazysegtree
PR: yorukot/superfile#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
PR: yorukot/superfile#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
PR: yorukot/superfile#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
PR: yorukot/superfile#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
PR: yorukot/superfile#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
PR: yorukot/superfile#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
PR: yorukot/superfile#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
PR: yorukot/superfile#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
PR: yorukot/superfile#963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
Learnt from: lazysegtree
PR: yorukot/superfile#924
File: src/internal/model_test.go:147-168
Timestamp: 2025-08-09T02:52:22.516Z
Learning: In superfile tests, lazysegtree prefers tests that accurately mimic the actual production behavior, including file operations like creating and deleting the LastDir file, rather than isolating such operations to temporary locations when the real behavior involves modifying/deleting those specific files.
Learnt from: lazysegtree
PR: yorukot/superfile#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.
| sortOptions sortOptionsModelData) []element { | ||
| sortOptions sortOptionsModelData, | ||
| ) []element { |
There was a problem hiding this comment.
Please dont add unnecessary diff. It adds noise in the PR.
There was a problem hiding this comment.
@lazysegtree
Some of the changes you noticed are the result of running go fmt ./... and golangci-lint run.
They were not intentional logic changes, just formatting and lint fixes that came up during the checks.
| var LastTimeCursorMove = [2]int{int(time.Now().UnixMicro()), 0} //nolint: gochecknoglobals // TODO: Move to model struct | ||
| var et *exiftool.Exiftool //nolint: gochecknoglobals // TODO: Move to model struct | ||
| var ( | ||
| LastTimeCursorMove = [2]int{int(time.Now().UnixMicro()), 0} //nolint: gochecknoglobals // TODO: Move to model struct | ||
| et *exiftool.Exiftool //nolint: gochecknoglobals // TODO: Move to model struct |
There was a problem hiding this comment.
Why this diff ? Please dont add unnecessary changes in PR. A formatting change like this, if needed can be taken in a different PR.
|
@lazysegtree Should i remove all the unused code ( the depreciated comments , as well as unused functions and variables that are now commented post refactoring) . |
Yes @faisal-990 |
…on that was commented out
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
A bit busy this weekend. Will review this on Monday. If you want, you can have yorukot take a look. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Thanks for PR @faisal-990 |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [yorukot/superfile](https://github.com/yorukot/superfile) | minor | `v1.3.3` -> `v1.4.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>yorukot/superfile (yorukot/superfile)</summary> ### [`v1.4.0`](https://github.com/yorukot/superfile/releases/tag/v1.4.0) [Compare Source](yorukot/superfile@v1.3.3...v1.4.0) Hey folks. Releasing v1.4.0 with many new features, improvements, and bug fixes. We have an async file preview now, a zoxide panel, and various new features improving UX. #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - We have the Zoxide Panel now. Ensure zoxide is installed on your system, set `zoxide_support` to `true` in the config, and press `z` to use zoxide. <img width="645" height="295" alt="Image" src="https://github.com/user-attachments/assets/238f6549-5318-49d1-a3a0-14cf8a686955" /> - File previewing is now async, meaning reduced lag while scrolling through images, or on slow systems. - Many bug fixes. See 'Detailed Change Summary' ##### Internal Updates - Most file operations are now truly async with the usage of the recommended `tea.Cmd` pattern. - Enabled many new linters to improve code quality. - Moved golangci-lint to v2. Now developers don't need to keep the old v1 in their systems. - Refactored file preview in its own package for better maintainability and readability. - Fixed flaky unit tests. #### Detailed Change Summary <details><summary>Details</summary> <p> ##### Update - feat: File operation via tea cmd [#​963](yorukot/superfile#963) by @​lazysegtree - feat: processbar improvements, package separation, better channel management [#​973](yorukot/superfile#973) by @​lazysegtree - feat: enable lll and recvcheck linter, fix tests, more refactors [#​977](yorukot/superfile#977) by @​lazysegtree - feat: Remove channel for notification models [#​979](yorukot/superfile#979) by @​lazysegtree - feat: enable cyclop, funlen, gocognit, gocyclo linters, and refactor large functions [#​984](yorukot/superfile#984) by @​lazysegtree - feat: Add a new hotkey to handle cd-on-quit whenever needed [#​924](yorukot/superfile#924) by @​ahmed-habbachi - feat: added option to permanently delete files [#​987](yorukot/superfile#987) by @​hupender - feat: Preview panel separation [#​1021](yorukot/superfile#1021) by @​lazysegtree - feat: Add search functionality to help menu [#​1011](yorukot/superfile#1011) by @​iZarrios - feat: Use zoxide lib [#​1036](yorukot/superfile#1036) by @​lazysegtree - feat: Add zoxide directory tracking on navigation [#​1041](yorukot/superfile#1041) by @​lazysegtree - feat: Zoxide integration [#​1039](yorukot/superfile#1039) by @​lazysegtree - feat: Select mode with better feedback [#​1074](yorukot/superfile#1074) by @​lazysegtree - feat: owner/group in the metadata [#​1093](yorukot/superfile#1093) by @​xelavopelk - feat: Async zoxide [#​1104](yorukot/superfile#1104) by @​lazysegtree ##### Bug Fix - fix: sorting in searchbar [#​985](yorukot/superfile#985) by @​hupender - fix: Async rendering, Include clipboard check in paste items, and update linter configs [#​997](yorukot/superfile#997) by @​lazysegtree - fix: Move utility functions to utils package [#​1012](yorukot/superfile#1012) by @​lazysegtree - fix: Refactoring and separation of preview panel and searchbar in help menu [#​1013](yorukot/superfile#1013) by @​lazysegtree - fix(filePanel): allow focusType to be set correctly [#​1033](yorukot/superfile#1033) by @​faisal-990 - fix(ci): Update gomod2nix.toml, allow pre release in version output, release 1.4.0-rc1, bug fixes, and improvements [#​1054](yorukot/superfile#1054) by @​lazysegtree - fix(nix): resolve build failures in the nix flake [#​1068](yorukot/superfile#1068) by @​Frost-Phoenix - fix: Retry the file deletion to prevent flakies (#​938) [#​1076](yorukot/superfile#1076) by @​lazysegtree - fix(issue-1066): Fixed issue where enter was not searchable [#​1078](yorukot/superfile#1078) by @​Simpaqt - fix(#​1073): Tech debt fix [#​1077](yorukot/superfile#1077) by @​Simpaqt - fix: fix deleted directory not able to remove from pins (#​1067) [#​1081](yorukot/superfile#1081) by @​yorukot - fix: fix child process spawning attached [#​1084](yorukot/superfile#1084) by @​guemidiborhane - fix: always clear images when showing a FullScreenStyle [#​1094](yorukot/superfile#1094) by @​snikoletopoulos - fix: Allow j and k keys in zoxide [#​1102](yorukot/superfile#1102) by @​lazysegtree - fix: Zoxide improvements and 1.4.0-rc2 [#​1105](yorukot/superfile#1105) by @​lazysegtree - fix: rename cursor beginning on wrong character because of multiple dots in name (#​813) [#​1112](yorukot/superfile#1112) by @​SyedAsadK - fix: check and fix file panel scroll position on height changes [#​1095](yorukot/superfile#1095) by @​snikoletopoulos ##### Optimization - perf(website): optimize font loading and asset organization [#​1089](yorukot/superfile#1089) by @​yorukot ##### Documentation - docs: fix incorrect zoxide plugin config name [#​1049](yorukot/superfile#1049) by @​shree-xvi - docs(hotkeys): Fix typo in vimHotkeys.toml comments [#​1080](yorukot/superfile#1080) by @​wleoncio - docs: add section for core maintainers in README.md [#​1088](yorukot/superfile#1088) by @​yorukot - chore: add winget install instruction to readme and website [#​943](yorukot/superfile#943) by @​claykom ##### Dependencies - chore(deps): update dependency go to v1.25.0, golangci-lint to v2, golangci-lint actions to v8 [#​750](yorukot/superfile#750) by @​renovate[bot] - chore(deps): update amannn/action-semantic-pull-request action to v6 [#​1006](yorukot/superfile#1006) by @​renovate[bot] - chore(deps): update actions/first-interaction action to v3 [#​1005](yorukot/superfile#1005) by @​renovate[bot] - chore(deps): update actions/checkout action to v5 [#​1004](yorukot/superfile#1004) by @​renovate[bot] - chore(deps): bump astro from 5.10.1 to 5.12.8 [#​982](yorukot/superfile#982) by @​dependabot[bot] - fix(deps): update module golang.org/x/mod to v0.27.0 [#​989](yorukot/superfile#989) by @​renovate[bot] - fix(deps): update dependency @​expressive-code/plugin-collapsible-sections to v0.41.3 [#​990](yorukot/superfile#990) by @​renovate[bot] - fix(deps): update dependency sharp to v0.34.3 [#​992](yorukot/superfile#992) by @​renovate[bot] - fix(deps): update dependency @​expressive-code/plugin-line-numbers to v0.41.3 [#​991](yorukot/superfile#991) by @​renovate[bot] - chore(deps): update dependency go to v1.25.0 [#​994](yorukot/superfile#994) by @​renovate[bot] - fix(deps): update astro monorepo [#​995](yorukot/superfile#995) by @​renovate[bot] - fix(deps): update dependency @​astrojs/starlight to ^0.35.0 [#​1000](yorukot/superfile#1000) by @​renovate[bot] - fix(deps): update module github.com/urfave/cli/v3 to v3.4.1 [#​1001](yorukot/superfile#1001) by @​renovate[bot] - fix(deps): update module golang.org/x/text to v0.28.0 [#​1003](yorukot/superfile#1003) by @​renovate[bot] ##### Misc - chore: migrate from superfile.netlify.app to superfile.dev [#​1087](yorukot/superfile#1087) by @​yorukot - refactor(filepanel): replace filePanelFocusType with isFocused boolean [#​1040](yorukot/superfile#1040) by @​faisal-990 - refactor(ansi): Migrate from github.com/charmbracelet/x/exp/term/ansi to github.com/charmbracelet/x/ansi [#​1044](yorukot/superfile#1044) by @​faisal-990 - refactor: common operation on pinned directory file using PinnedManager [#​1085](yorukot/superfile#1085) by @​Manaswa-S - test: unit tests for pinned manager [#​1090](yorukot/superfile#1090) by @​Manaswa-S </p> </details> #### New Contributors * @​hupender made their first contribution in yorukot/superfile#985 * @​ahmed-habbachi made their first contribution in yorukot/superfile#924 * @​iZarrios made their first contribution in yorukot/superfile#1011 * @​faisal-990 made their first contribution in yorukot/superfile#1033 * @​shree-xvi made their first contribution in yorukot/superfile#1049 * @​Simpaqt made their first contribution in yorukot/superfile#1078 * @​wleoncio made their first contribution in yorukot/superfile#1080 * @​guemidiborhane made their first contribution in yorukot/superfile#1084 * @​Manaswa-S made their first contribution in yorukot/superfile#1085 * @​xelavopelk made their first contribution in yorukot/superfile#1093 * @​snikoletopoulos made their first contribution in yorukot/superfile#1094 * @​SyedAsadK made their first contribution in yorukot/superfile#1112 **Full Changelog**: <yorukot/superfile@v1.3.3...v1.4.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xNDYuMCIsInVwZGF0ZWRJblZlciI6IjQxLjE0Ni4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
The filePanelFocusType typed constant with multiple states (noneFocus, secondFocus, focus) added unnecessary complexity for managing panel focus. It also made the code harder to read and maintain.
Solution
Replaced filePanelFocusType with a simple isFocused boolean field in filePanel.
isFocused = true → panel is focused
isFocused = false → panel is not focused
This change removes unused constants and simplifies focus handling.
Refs #1033
Refs #1030
Summary by CodeRabbit
No user-facing features changed; interactions should feel more consistent and stable, especially when switching panels and using the sidebar or process bar.