Skip to content

Conversation

@lazysegtree
Copy link
Collaborator

@lazysegtree lazysegtree commented Apr 7, 2025

Fixes

Test

➜  ~/Workspace/kuknitin/superfile git:(multiple_panel_at_startup) [7:07:36] spft "~" "." "/"
➜  ~/Workspace/kuknitin/superfile git:(multiple_panel_at_startup) [7:11:18]
image

Too many panels

➜  ~/Workspace/kuknitin/superfile git:(multiple_panel_at_startup) [7:12:41] spft --print-last-dir "~" "." "/" "." "." "."
/Users/kuknitin
➜  ~/Workspace/kuknitin/superfile git:(multiple_panel_at_startup) [7:13:13]
image

Summary by CodeRabbit

  • New Features

    • Now supports specifying multiple starting directories for file panels, offering a broader configuration experience.
  • Refactor

    • Streamlined initialization and configuration logic to handle a list of directory inputs along with appropriate defaults.
  • Tests

    • Updated validation to ensure the new multi-directory input is processed correctly.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 7, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The pull request refactors the application to support multiple directory inputs instead of a single directory. Command-line arguments are now parsed into a slice of strings, and core functions—such as model initialization, configuration, and file panel creation—have been updated to work with this slice. Additionally, new helper functions have been introduced to construct file panel instances based on the provided directories, aligning the code with the multi-pane directory loading requirement.

Changes

Files Change Summary
src/cmd/main.go, src/internal/config_function.go, src/internal/default_config.go, src/internal/model.go,
src/internal/model_prompt_test.go
Changed function signatures to accept a slice of strings (firstFilePanelDirs) instead of a single string. Internal logic in these functions was updated to loop through the directories and apply default values/path resolution accordingly.
src/internal/type_utils.go Added new helper functions filePanelSlice and defaultFilePanel to construct and initialize multiple filePanel instances based on the provided directory names.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant M as Main (Run)
    participant IM as InitialModel
    participant IC as initialConfig
    participant DC as defaultModelConfig
    participant TU as filePanelSlice

    U->>M: Provide command-line directories
    M->>IM: Pass firstFilePanelDirs slice
    IM->>IC: Call initialConfig(firstFilePanelDirs)
    IC-->>IM: Return config (toggleDotFile, toggleFooter)
    IM->>DC: Call defaultModelConfig(toggleDotFile, toggleFooter, firstFilePanelDirs)
    DC->>TU: Call filePanelSlice(firstFilePanelDirs)
    TU-->>DC: Return slice of filePanel instances
    DC-->>IM: Return configured model
    IM-->>M: Return complete model
Loading

Assessment against linked issues

Objective Addressed Explanation
Load multiple directories in separate panes (#286)

Poem

I'm a rabbit with nimble code,
Hopping through directories in a joyful mode.
Multiple panes now spring to life,
Each one free from previous strife.
Code carrots crunch in every byte!
🐰✨


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@lazysegtree lazysegtree force-pushed the golangci_lint_fixes_2 branch 3 times, most recently from 92f3eb5 to 8a71560 Compare April 8, 2025 13:04
@lazysegtree lazysegtree changed the base branch from golangci_lint_fixes_2 to lazysegtree_dev April 8, 2025 13:05
@lazysegtree lazysegtree force-pushed the multiple_panel_at_startup branch 4 times, most recently from 0a0da79 to 5a5291a Compare April 8, 2025 13:37
@lazysegtree
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 8, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

1 similar comment
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 8, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/internal/type_utils.go (1)

57-78: New helper function to create a default file panel.

This function nicely encapsulates the creation of a default file panel with all necessary properties initialized. The code is well-structured and uses configuration values appropriately.

One potential improvement would be to add directory path validation to handle non-existent directories gracefully.

Consider adding directory validation to handle cases where a specified directory doesn't exist:

func defaultFilePanel(dir string) filePanel {
+   // Use empty string or current directory if dir is invalid
+   if dir == "" {
+       var err error
+       dir, err = os.Getwd()
+       if err != nil {
+           dir = "/"
+       }
+   } else if _, err := os.Stat(dir); os.IsNotExist(err) {
+       // Log the error or handle invalid directory
+       slog.Warn("Directory does not exist, using fallback", "dir", dir)
+       dir = "/"
+   }
+
    return filePanel{
        render:   0,
        cursor:   0,
        location: dir,
        // Rest of the code remains the same
    }
}
src/internal/config_function.go (2)

18-20: Update function comment to reflect new behavior

The function comment still describes returning the absolute path of a directory, but the function no longer returns firstFilePanelDir.

- // initialConfig load and handle all configuration files (spf config,Hotkeys
- // themes) setted up. Returns absolute path of dir pointing to the file Panel
+ // initialConfig loads and handles all configuration files (spf config, hotkeys,
+ // themes). Processes input directories and returns toggle states.

57-57: Consider handling empty slice case

The function doesn't explicitly handle the case where firstFilePanelDirs is an empty slice. Consider adding a check to provide a default directory in this case.

+	if len(firstFilePanelDirs) == 0 {
+		firstFilePanelDirs = append(firstFilePanelDirs, common.Config.DefaultDirectory)
+	}
+
 	for i := range firstFilePanelDirs {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9653d8a and 5a5291a.

📒 Files selected for processing (6)
  • src/cmd/main.go (2 hunks)
  • src/internal/config_function.go (2 hunks)
  • src/internal/default_config.go (2 hunks)
  • src/internal/model.go (1 hunks)
  • src/internal/model_prompt_test.go (1 hunks)
  • src/internal/type_utils.go (1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
src/internal/model_prompt_test.go (3)
src/internal/function.go (1)
  • TeaUpdate (590-599)
src/internal/common/utils/tea_utils.go (1)
  • TeaRuneKeyMsg (5-10)
src/internal/common/default_config.go (1)
  • Hotkeys (12-12)
src/cmd/main.go (1)
src/internal/model.go (1)
  • InitialModel (40-46)
src/internal/type_utils.go (2)
src/internal/common/default_config.go (1)
  • Config (11-11)
src/internal/common/style_function.go (1)
  • GenerateSearchBar (241-253)
🔇 Additional comments (12)
src/internal/model_prompt_test.go (2)

73-73: Updated test to match new function signature.

The test has been correctly updated to use a slice of strings []string{"/"} instead of a single string "/" to match the new function signature of defaultModelConfig.


81-81: Updated test to match new function signature.

Similar to the previous change, this test case was also correctly updated to use a slice of strings for the directory parameter.

src/cmd/main.go (3)

87-87: Variable type changed to support multiple directories.

Changed from a single string to a slice of strings, which aligns with the multi-panel feature implementation.


89-90: Updated to capture all command line arguments.

Now using c.Args().Slice() to get all provided paths instead of just the first one, allowing multiple directories to be specified at startup.


103-103: Updated model initialization to use multiple directories.

The call to InitialModel now correctly passes the slice of directories instead of a single directory string.

src/internal/type_utils.go (1)

49-55: New utility function to create panels from multiple directories.

This utility function creates a slice of file panels from a slice of directory paths, which is essential for the multi-panel feature. The implementation looks clean and efficient.

src/internal/model.go (2)

40-42: Updated function signature for multi-panel support.

Changed the InitialModel function to accept a slice of directories instead of a single directory, and correctly passes this to initialConfig. This is a key change for the multi-panel feature.


45-45: Updated model configuration to support multiple panels.

The call to defaultModelConfig now passes the slice of directories, enabling the creation of multiple panels at startup.

src/internal/default_config.go (1)

11-11: LGTM: Function signature and implementation correctly updated to support multiple directories

The function now accepts a slice of strings for directory paths instead of a single string, and uses the filePanelSlice helper function to create the appropriate file panels.

Also applies to: 26-26

src/internal/config_function.go (3)

21-21: LGTM: Function signature updated to handle multiple directories

The function now accepts a slice of directory paths and no longer returns a single directory, which aligns with the multiple panels feature.

Also applies to: 81-81


57-73: LGTM: Loop implementation correctly processes each directory

The loop appropriately handles each directory in the slice, maintaining the same logic that was previously applied to a single directory - handling empty directories, tilde prefixes, and calculating absolute paths.


76-76: LGTM: Debug message updated to reflect multiple directories

The log message has been correctly updated to indicate multiple directories.

@lazysegtree lazysegtree force-pushed the multiple_panel_at_startup branch from 5a5291a to 6f90021 Compare April 8, 2025 13:46
@lazysegtree lazysegtree merged commit d69cc95 into lazysegtree_dev Apr 8, 2025
@coderabbitai coderabbitai bot mentioned this pull request May 11, 2025
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request May 23, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [yorukot/superfile](https://github.com/yorukot/superfile) | minor | `v1.2.1` -> `v1.3.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.3.0`](https://github.com/yorukot/superfile/releases/tag/v1.3.0)

[Compare Source](yorukot/superfile@v1.2.1...v1.3.0)

We are excited to release v1.3.0 for superfile, with new features, key bug fixes, and lots of polish under the hood.

#### Install:

[**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation)

#### Highlights

-   We have revamped the command prompt and added built-in commands for SuperFile-specific actions. Check out yorukot/superfile#745

-   Multi-panel startup support. You can now open multiple panels right from startup, making your workflow even more efficient.

-   Added new configurations :  --chooser-file option, show_panel_footer_info config flag and many command prompt specific flags.

#### Improvements & Fixes

-   The sidebar code was refactored and separated for better maintainability and various linter fixes and CI/CD improvements were made to keep the codebase clean and robust.

-   A new Rendering package is implemented, centralising border, content, and section rendering logic into reusable renderer components, fixing many layout bugs.

-   Model behaviour, file operations and rendering-related unit tests were added to improve test coverage.

#### Detailed Change Summary

<details><summary>Details</summary>
<p>

#### New Features

-   Added a Command-Prompt for SuperFile specific actions [`#752`](yorukot/superfile#752) by [@&#8203;Rocco-Gossmann](https://github.com/Rocco-Gossmann), [@&#8203;yorukot](https://github.com/yorukot)  and [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Allow specifying multiple panels at startup [`#759`](yorukot/superfile#759) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Initial draft of rendering package [`#775`](yorukot/superfile#775) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Render unit tests for prompt model [`#809`](yorukot/superfile#809) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Chooser file option, --lastdir-file option, and improvements in quit, and bug fixes [`#812`](yorukot/superfile#812) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Prompt feature leftover items [`#804`](yorukot/superfile#804) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   SPF Prompt tutorial and fixes [`#814`](yorukot/superfile#814) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Write prompt tutorial, rename prompt mode to spf mode, add develop branch in GitHub workflow, show_panel_footer_info flag [`#815`](yorukot/superfile#815) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Theme: Add gruvbox-dark-hard [`#828`](yorukot/superfile#828) by [@&#8203;Frost-Phoenix](https://github.com/Frost-Phoenix)

#### Updates & Improvements

-   Sidebar separation [`#767`](yorukot/superfile#767) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Sidebar code separation [`#770`](yorukot/superfile#770) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Rendering package and rendering bug fixes [`#781`](yorukot/superfile#781) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Refactor CheckForUpdates [`#797`](yorukot/superfile#797) by [@&#8203;JassonCordones](https://github.com/JassonCordones)
-   Rename metadata strings [`#731`](yorukot/superfile#731) by [@&#8203;booth-w](https://github.com/booth-w)

#### Bug Fixes

-   Fix crash with opening file with editor on an empty panel [`#730`](yorukot/superfile#730) by [@&#8203;booth-w](https://github.com/booth-w)
-   Fix: Add some of the remaining linter and fix errors [`#756`](yorukot/superfile#756) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Golangci lint fixes [`#757`](yorukot/superfile#757) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Fix: Remove redundant function containsKey [`#765`](yorukot/superfile#765) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Fix: Correctly resolve path in open and cd prompt actions [`#802`](yorukot/superfile#802) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Prompt dynamic dimensions and unit tests fix [`#805`](yorukot/superfile#805) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Fix: Convert unicode space to normal space, use rendered in file preview to fix layout bugs, Release 1.3.0 [`#825`](yorukot/superfile#825) by [@&#8203;lazysegtree](https://github.com/lazysegtree)

#### Optimization & Code Quality

-   Adding linter to CI/CD and fix some lint issues [`#739`](yorukot/superfile#739) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Linter fixes, new feature of allowing multiple directories at startup, other code improvements [`#764`](yorukot/superfile#764) by [@&#8203;lazysegtree](https://github.com/lazysegtree)
-   Model unit tests [`#803`](yorukot/superfile#803) by [@&#8203;lazysegtree](https://github.com/lazysegtree)

#### Dependency Updates

-   fix(deps): update dependency astro to v5.7.7 [`#726`](yorukot/superfile#726) by [@&#8203;renovate](https://github.com/renovate)
-   fix(deps): update module github.com/shirou/gopsutil/v4 to v4.25.3 [`#749`](yorukot/superfile#749) by [@&#8203;renovate](https://github.com/renovate)
-   fix(deps): update module github.com/pelletier/go-toml/v2 to v2.2.4 [`#760`](yorukot/superfile#760) by [@&#8203;renovate](https://github.com/renovate)
-   fix(deps): update module github.com/alecthomas/chroma/v2 to v2.16.0 [`#751`](yorukot/superfile#751) by [@&#8203;renovate](https://github.com/renovate)
-   fix(deps): update dependency sharp to ^0.34.0 [`#755`](yorukot/superfile#755) by [@&#8203;renovate](https://github.com/renovate)
-   fix(deps): update dependency [@&#8203;astrojs/starlight](https://github.com/astrojs/starlight) to ^0.34.0 [`#761`](yorukot/superfile#761) by [@&#8203;renovate](https://github.com/renovate)

</p>
</details> 

#### New Contributors
* @&#8203;Rocco-Gossmann made their first contribution in yorukot/superfile#736
* @&#8203;Frost-Phoenix made their first contribution in yorukot/superfile#828

**Full Changelog**: yorukot/superfile@v1.2.1...v1.3.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:eyJjcmVhdGVkSW5WZXIiOiI0MC4yMi4wIiwidXBkYXRlZEluVmVyIjoiNDAuMjMuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
},
Action: func(c *cli.Context) error {
// If no args are called along with "spf" use current dir
path := ""
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai
Please generate a task descriptions for this PR. I will give someone this task. He doesn't know anything about this product. Context - SPF is a file manager, it can be opened with directory's path. But not more than one. To support more than 1.

takes args more than one, use them to init initial filemodel strcut with more panels.

Add anything else that I missed, which is done in this PR, and should be in task description.

Keep it in 6-7 points or less. Format should be markdown. Make sure to highlight what to be done, not how or where.

Give basic context about superfile and what is the requirement.

Also generate a TL;DR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lazysegtree

Task Description: Multi-Panel Startup Support for Superfile

Context

Superfile is a modern terminal-based file manager that provides an intuitive interface for navigating and managing files and directories. Currently, users can only specify a single directory path when launching the application, limiting them to viewing one directory at startup.

Requirement

Extend Superfile to support multiple directory paths at startup, allowing users to open several file panels simultaneously by providing multiple directory arguments via the command line.

Tasks

  1. Modify command-line argument processing to accept and handle multiple directory paths instead of a single path argument

  2. Update the configuration initialization system to process multiple directory paths and apply appropriate defaults (like home directory or current directory) to empty/invalid paths

  3. Create utility functions for generating multiple file panel instances from a list of directory paths, ensuring each panel is properly initialized with default settings

  4. Refactor the model initialization to accept multiple directories and create corresponding file panels in the initial application state

  5. Update the default model configuration to support multiple file panels instead of a single panel, maintaining backward compatibility for single-directory usage

  6. Ensure proper error handling for invalid directory paths while allowing valid paths to still create functional panels

TL;DR

Enable Superfile to accept multiple directory arguments at startup (e.g., spf /home /tmp /var) and initialize the application with multiple file panels, one for each specified directory, instead of the current single-panel limitation.


🧠 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#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#970
File: src/internal/ui/processbar/model.go:11-26
Timestamp: 2025-08-02T17:00:37.262Z
Learning: lazysegtree prefers to track memory leak issues and feature enhancements via dedicated GitHub issues when identified during PR reviews but are beyond the current PR scope, particularly for processbar-related improvements like cleanup mechanisms and keyboard shortcuts.

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.

Learnt from: lazysegtree
PR: yorukot/superfile#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.

Comment on lines +49 to +78
func filePanelSlice(dir []string) []filePanel {
res := make([]filePanel, len(dir))
for i := range dir {
res[i] = defaultFilePanel(dir[i])
}
return res
}

func defaultFilePanel(dir string) filePanel {
return filePanel{
render: 0,
cursor: 0,
location: dir,
sortOptions: sortOptionsModel{
width: 20,
height: 4,
open: false,
cursor: common.Config.DefaultSortType,
data: sortOptionsModelData{
options: []string{"Name", "Size", "Date Modified"},
selected: common.Config.DefaultSortType,
reversed: common.Config.SortOrderReversed,
},
},
panelMode: browserMode,
focusType: focus,
directoryRecords: make(map[string]directoryRecord),
searchBar: common.GenerateSearchBar(),
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't allow passing the focusType argument in the defaultFilePanel method, leading to having focusType set to focus for all the file panels. This is bad as it results in an inconsistent state of the initial file panels, where all panels are set to focus.

@coderabbitai

This is a bug, create and issue to track this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@lazysegtree You're absolutely right! This is indeed a bug where all file panels are incorrectly initialized with focusType: focus, which violates the single-focus principle in the UI.


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue to track the focusType bug
gh issue create \
  --title "Bug: All file panels initialized with focus type causing inconsistent state" \
  --body "## Problem
The new \`defaultFilePanel\` function hardcodes \`focusType: focus\` for all file panels, leading to an inconsistent state where multiple panels appear focused simultaneously.

## Root Cause
In the \`defaultFilePanel\` function added in PR #759, the \`focusType\` field is hardcoded to \`focus\`:

\`\`\`go
func defaultFilePanel(dir string) filePanel {
    return filePanel{
        // ... other fields ...
        focusType: focus,  // ← This should not be hardcoded
        // ... other fields ...
    }
}
\`\`\`

## Expected Behavior
Only one file panel should have \`focusType: focus\` at initialization, while others should have \`focusType: noneFocus\` or \`focusType: secondFocus\`.

## Suggested Fix
The \`defaultFilePanel\` function should either:
1. Accept a \`focusType\` parameter to allow caller to specify the focus state
2. Have the caller (\`filePanelSlice\`) handle focus assignment logic

## References
- Original PR: https://github.com/yorukot/superfile/pull/759
- Comment: https://github.com/yorukot/superfile/pull/759#discussion_r2311878216
- Reported by: @lazysegtree" \
  --assignee lazysegtree

Length of output: 1257


Perfect! I've created GitHub issue #1030 to track this bug. The issue documents the problem where all file panels are incorrectly initialized with focusType: focus, creating an inconsistent UI state.

The issue includes:

  • Clear problem description
  • Root cause analysis with code reference
  • Expected behavior explanation
  • Suggested fix approaches
  • References to this PR and your comment

This will help ensure the focus type bug gets proper attention and resolution in a future PR.

🐰💭 (thoughtful rabbit considering the focus state issue)


🧠 Learnings used
Learnt from: lazysegtree
PR: yorukot/superfile#906
File: src/internal/file_panel.go:11-14
Timestamp: 2025-07-12T05:32:53.131Z
Learning: lazysegtree is concerned about the efficiency impact of adding bounds checking to frequently called methods like getSelectedItem() in file panels, preferring to defer such changes to consider performance implications.

Learnt from: lazysegtree
PR: yorukot/superfile#1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.296Z
Learning: In PR #1021 for yorukot/superfile, lazysegtree reported image preview crashes and rename test failures after refactoring FilePreviewPanel to a separate preview package. The crashes were likely caused by: 1) imagePreviewer initialization differences between old global variable and new per-instance creation, 2) method name changes from SetContextWithRenderText to SetContentWithRenderText, and 3) batCmd initialization moving from global to per-instance without preserving the original configuration.

Learnt from: lazysegtree
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.333Z
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.

Learnt from: lazysegtree
PR: yorukot/superfile#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.

faisal-990 added a commit to faisal-990/superfile that referenced this pull request Aug 31, 2025
The defaultFilePanel function previously hardcoded focusType to 'focus',
causing multiple panels to appear focused simultaneously.

Now, defaultFilePanel accepts a focusType parameter, allowing callers
to initialize panels with the correct focus state. This ensures only
one panel is focused at initialization, and others are set to 'noneFocus'
or 'secondFocus' as needed.

Fixes the inconsistent panel focus issue introduced in PR yorukot#759.
lazysegtree pushed a commit that referenced this pull request Aug 31, 2025
The defaultFilePanel function previously hardcoded focusType to 'focus',
causing multiple panels to appear focused simultaneously.

Now, defaultFilePanel accepts a focusType parameter, allowing callers to
initialize panels with the correct focus state. This ensures only one
panel is focused at initialization, and others are set to 'noneFocus' or
'secondFocus' as needed.

Fixes the inconsistent panel focus issue introduced in PR #759.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants