Linter fixes, new feature of allowing multiple directories at startup, other code improvements #764
Conversation
Golangci lint fixes
WalkthroughThis update streamlines the GolangCI-Lint configuration by enabling linters and explicitly adding exclusions for specific files. Extensive refactoring has been applied across the codebase, including renaming functions and struct fields for consistency, replacing legacy logging with new error utility functions, updating error wrapping with Changes
Sequence Diagram(s)sequenceDiagram
participant F as Function
participant U as Utils
participant OS as Operating System
F->>U: Detects error condition
U->>OS: Logs error (using PrintfAndExit/PrintlnAndExit)
OS-->>F: Terminates process with exit status 1
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/internal/handle_file_operations.go (1)
456-463: Simplify conditional logic with early returns.The current error handling approach with nested if-else could be further improved by using early returns to reduce nesting.
- // Todo : These error cases are hard to test. We have to somehow make the paste operations fail, - // which is time consuming and manual. We should test these with automated testcases - err = pasteDir(filePath, filepath.Join(panel.location, filepath.Base(filePath)), id, m) - if err != nil { - errMessage = "paste item error" - } else if m.copyItems.cut { - os.RemoveAll(filePath) - } + // Todo : These error cases are hard to test. We have to somehow make the paste operations fail, + // which is time consuming and manual. We should test these with automated testcases + err = pasteDir(filePath, filepath.Join(panel.location, filepath.Base(filePath)), id, m) + if err != nil { + errMessage = "paste item error" + break + } + + if m.copyItems.cut { + os.RemoveAll(filePath) + }.golangci.yaml (1)
421-422: Fix trailing whitespace.There are trailing whitespace characters at the end of lines 421-422 that should be removed to maintain clean YAML formatting.
- - gochecknoglobals - + - gochecknoglobals🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 421-421: trailing spaces
(trailing-spaces)
[error] 422-422: trailing spaces
(trailing-spaces)
src/internal/model.go (1)
25-33: Improved documentation for global variables.Added comments clarifying that these variables represent model state and should be moved to the model struct. This highlights technical debt that should be addressed in the future.
Consider refactoring these global variables into the model struct in a future PR to improve encapsulation and testability of the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
.golangci.yaml(5 hunks)src/cmd/main.go(9 hunks)src/config/fixed_variable.go(1 hunks)src/config/icon/function.go(1 hunks)src/config/icon/icon.go(2 hunks)src/internal/common/load_config.go(3 hunks)src/internal/common/predefined_variable.go(1 hunks)src/internal/common/style.go(0 hunks)src/internal/common/style_function.go(1 hunks)src/internal/common/utils/bool_file_store.go(1 hunks)src/internal/common/utils/bool_file_store_test.go(4 hunks)src/internal/common/utils/consts.go(1 hunks)src/internal/common/utils/file_utils.go(4 hunks)src/internal/common/utils/log_utils.go(1 hunks)src/internal/common/utils/shell_utils.go(2 hunks)src/internal/config_function.go(2 hunks)src/internal/file_operations.go(10 hunks)src/internal/file_operations_compress.go(1 hunks)src/internal/file_operations_extract.go(1 hunks)src/internal/function.go(18 hunks)src/internal/get_data.go(2 hunks)src/internal/handle_file_operations.go(17 hunks)src/internal/handle_modal.go(0 hunks)src/internal/handle_panel_movement.go(2 hunks)src/internal/handle_panel_navigation.go(0 hunks)src/internal/handle_panel_up_down_test.go(1 hunks)src/internal/internal_consts.go(1 hunks)src/internal/key_function.go(2 hunks)src/internal/model.go(6 hunks)src/internal/model_prompt_test.go(0 hunks)src/internal/model_render.go(10 hunks)src/internal/sidebar.go(0 hunks)src/internal/sidebar_test.go(5 hunks)src/internal/string_function.go(6 hunks)src/internal/string_function_test.go(0 hunks)src/internal/type.go(1 hunks)src/internal/type_utils.go(1 hunks)src/internal/ui/prompt/model.go(2 hunks)src/internal/ui/prompt/model_test.go(0 hunks)src/internal/ui/prompt/tokenize.go(3 hunks)src/internal/ui/prompt/tokenize_test.go(1 hunks)src/internal/ui/prompt/utils.go(0 hunks)src/internal/ui/prompt/utils_test.go(1 hunks)src/internal/wheel_function.go(1 hunks)src/pkg/file_preview/image_preview.go(3 hunks)src/pkg/string_function/overplace.go(1 hunks)
💤 Files with no reviewable changes (8)
- src/internal/model_prompt_test.go
- src/internal/common/style.go
- src/internal/sidebar.go
- src/internal/ui/prompt/utils.go
- src/internal/string_function_test.go
- src/internal/handle_modal.go
- src/internal/handle_panel_navigation.go
- src/internal/ui/prompt/model_test.go
🧰 Additional context used
🧠 Learnings (1)
src/internal/handle_panel_up_down_test.go (1)
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/handle_panel_up_down_test.go:13-13
Timestamp: 2025-04-08T12:59:09.350Z
Learning: Go 1.22+ supports the integer range loop feature that allows iterating over integers directly with the range keyword. The syntax `for i := range n` iterates from 0 to n-1, which is equivalent to the traditional `for i := 0; i < n; i++` loop. This feature was implemented via GitHub issue #61405.
🧬 Code Definitions (13)
src/config/icon/function.go (1)
src/config/icon/icon.go (2)
Folders(397-417)Style(4-7)
src/internal/handle_panel_movement.go (1)
src/internal/common/utils/consts.go (2)
OsDarwin(10-10)OsWindows(8-8)
src/internal/common/utils/bool_file_store.go (1)
src/internal/common/utils/consts.go (2)
TrueString(4-4)FalseString(5-5)
src/internal/common/utils/shell_utils.go (1)
src/internal/common/utils/consts.go (1)
OsWindows(8-8)
src/internal/handle_file_operations.go (1)
src/internal/common/utils/consts.go (2)
OsWindows(8-8)OsDarwin(10-10)
src/internal/common/load_config.go (2)
src/internal/common/utils/log_utils.go (2)
PrintlnAndExit(12-15)PrintfAndExit(20-23)src/internal/common/style_function.go (1)
LoadHotkeysError(236-238)
src/internal/config_function.go (3)
src/config/fixed_variable.go (4)
LogFile(45-45)HomeDir(27-27)ToggleDotFile(41-41)ToggleFooter(42-42)src/internal/common/utils/log_utils.go (1)
PrintfAndExit(20-23)src/internal/common/utils/bool_file_store.go (1)
ReadBoolFile(12-31)
src/internal/file_operations.go (2)
src/internal/common/utils/consts.go (2)
OsWindows(8-8)OsDarwin(10-10)src/config/fixed_variable.go (1)
DarwinTrashDirectory(49-49)
src/cmd/main.go (4)
src/internal/common/predefined_variable.go (1)
LoadInitialPrerenderedVariables(34-37)src/config/fixed_variable.go (3)
UpdateVarFromCliArgs(73-97)ConfigFile(57-57)HotkeysFile(58-58)src/internal/common/utils/log_utils.go (2)
PrintfAndExit(20-23)PrintlnAndExit(12-15)src/internal/common/utils/consts.go (1)
OsDarwin(10-10)
src/internal/model.go (3)
src/internal/ui/prompt/type.go (1)
Model(6-24)src/internal/common/default_config.go (1)
Hotkeys(12-12)src/config/fixed_variable.go (1)
SetLastDir(69-71)
src/internal/common/utils/bool_file_store_test.go (1)
src/internal/common/utils/consts.go (3)
TrueString(4-4)FalseString(5-5)OsWindows(8-8)
src/internal/model_render.go (3)
src/internal/common/predefined_variable.go (2)
SideBarPinnedDivider(27-27)SideBarDisksDivider(28-28)src/internal/common/default_config.go (1)
Config(11-11)src/config/icon/icon.go (1)
Error(34-34)
src/internal/string_function.go (3)
src/internal/common/style_function.go (1)
StringColorRender(210-214)src/internal/common/style.go (2)
FilePanelStyle(17-17)FooterBGColor(78-78)src/internal/common/default_config.go (1)
Config(11-11)
🪛 YAMLlint (1.35.1)
.golangci.yaml
[error] 421-421: trailing spaces
(trailing-spaces)
[error] 422-422: trailing spaces
(trailing-spaces)
🔇 Additional comments (114)
src/internal/common/predefined_variable.go (1)
34-34: Improved function naming convention.The function name has been updated from
LoadInitial_PrerenderedVariablestoLoadInitialPrerenderedVariables, which aligns better with Go's naming conventions that favor camelCase over snake_case for function names.src/internal/type.go (1)
251-251: Correct identifier naming convention.The field name has been updated from
messageIdtomessageID, following Go's convention of usingID(all caps) instead ofIdfor abbreviations in identifiers.src/internal/common/utils/bool_file_store.go (1)
24-27: Improved constant naming references.The code now references the camelCase constants
TrueStringandFalseStringfrom the consts.go file instead of the previous snake_case versions. This change aligns with Go's naming conventions and improves consistency across the codebase.src/internal/file_operations_extract.go (1)
29-29: Consistent identifier naming.The field name has been updated from
messageIdtomessageID, maintaining consistency with the same change in the type definition and following Go's convention for abbreviations in identifiers.src/internal/handle_panel_up_down_test.go (1)
13-13: Relying on Go 1.22+ “integer range” feature.This line uses
for i := range count, which is only valid in Go 1.22+ (according to your retrieved learnings). If your project or CI environment isn’t pinned to Go 1.22+ yet, this will fail to compile.Consider reverting to a traditional for loop for broader compatibility:
-for i := range count { +for i := 0; i < count; i++ {src/internal/common/style_function.go (1)
19-19: Likewise depends on Go 1.22+ “integer range” feature.Similar to the previous file,
for i := range heightwill only work on Go 1.22+ compilers. If you need broader support, revert to a standard for loop.-for i := range height { +for i := 0; i < height; i++ {src/config/icon/function.go (1)
41-44: Switch toStyletype looks correct.Using
Folders["folder"] = Style{...}aligns with the newStylestruct fromicon.go. It maintains consistency with the updated type across the codebase. No further changes are needed.src/internal/type_utils.go (1)
28-28: Consistent error message formatting.The error message now begins with lowercase "invalid" instead of uppercase "Invalid", following Go's error message style conventions where error messages typically start with a lowercase letter.
src/internal/ui/prompt/utils_test.go (1)
112-112: Appropriate linter directive usage with explanation.Good addition of a comment explaining why
errorlintis disabled for this line. The comment clearly explains that type assertion is intentional here since wrapped errors aren't expected in this test context.src/internal/internal_consts.go (2)
5-6: Clear explanation for the use ofvarinstead ofconst.The added comment properly explains why
varis used (Go doesn't allow const structs) and includes the appropriate linter directive to suppress the global variable check, along with a justification.
10-11: Consistent linter comment for global variable.The same correct pattern is applied to
diskDividerDir, maintaining consistency across similar variables.src/internal/handle_panel_movement.go (3)
77-78: Improved variable naming convention.Variable renamed from snake_case to camelCase (
symlink_err→symlinkErr), adhering to Go's standard naming conventions.
82-84: Consistent variable naming convention.Variable renamed from snake_case to camelCase (
lstat_err→lstatErr), maintaining consistency with Go's naming conventions.
96-98: Better use of centralized constants.The OS check now uses constants from the utils package (
utils.OsDarwinandutils.OsWindows) instead of direct string literals or variables, promoting consistency and reducing the risk of typos.src/internal/file_operations_compress.go (1)
36-36: Field renamed to follow Go naming conventions.The field has been renamed from
messageIdtomessageIDto align with Go's convention of capitalizing acronyms in camelCase identifiers.src/internal/ui/prompt/tokenize_test.go (1)
19-19: Added comment explains variable usage and suppresses linter warning.The added comment clarifies why
testEnvValuesis declared as a variable instead of a constant (Go doesn't allow constant maps) and includes a linter directive to suppress the warning about global variables.src/internal/common/utils/consts.go (2)
4-5: Updated constant names to follow Go naming conventions.Constants have been renamed from
TRUE_STRINGandFALSE_STRINGtoTrueStringandFalseStringto adhere to Go's recommended naming style (PascalCase for exported constants).
6-10: Added OS identifier constants with documentation.New constants
OsWindowsandOsDarwinhave been introduced with appropriate documentation to centralize OS-specific string identifiers. This is consistent with the linter improvement effort and will help standardize OS checks throughout the codebase.src/internal/common/utils/shell_utils.go (2)
19-19: Replaced external OS constant with local package constant.The condition now uses the newly defined
OsWindowsconstant instead of importing it from thevariablepackage, reducing dependencies and centralizing OS-related constants.
44-44: Added linter directive to explain type assertion usage.The comment with
//nolint:errorlintclarifies why type assertion is used instead oferrors.As(), appropriately suppressing the linter warning with a clear explanation.src/pkg/string_function/overplace.go (1)
163-165: Function signature improved for clarity.The refactoring removes named return parameters in favor of unnamed return types, and explicitly initializes variables using short variable declarations. This is a good change that maintains the same functionality while making the code more idiomatic.
src/internal/wheel_function.go (4)
14-23: Good restructuring using switch statement.Replacing if-else chains with a switch statement improves readability and maintainability, especially for this type of case-based logic.
26-35: Consistent structure applied here too.The switch statement structure is consistently applied for both wheel directions, enhancing code readability.
36-38: Added helpful error handling for unexpected messages.Adding a default case with proper error logging is a good practice that improves robustness.
41-41: Simplified loop construction.Changing to a
for rangeloop is more idiomatic when only iteration count matters.src/internal/key_function.go (2)
29-42: Improved control flow with switch statement.Replacing if-else chains with a switch statement makes the code more readable and easier to maintain, especially for this kind of panel-focused logic.
46-59: Consistent structure applied for list down handling.The switch statement structure is consistently applied here too, maintaining a uniform coding style.
src/internal/common/utils/log_utils.go (2)
9-15: New utility function for error reporting and exit.This function provides a cleaner alternative to
log.Fatal()by ensuring error messages go to stderr and the process exits with the correct status code.
17-21: Added formatted printing capability.This companion function provides the same exit behavior with formatted output capability, completing the error reporting utility set.
src/config/icon/icon.go (4)
4-7: Structure rename improves namespace clarity.The renaming of
IconStyletoStylemakes the code more concise while maintaining readability, since the package nameiconprovides appropriate context when referenced asicon.Style.
10-42: Variable declaration simplification is clean and idiomatic.Removing explicit type declarations from these string constants is a good practice in Go where type inference provides cleaner code without losing type safety.
50-165: Type update to match renamed structure.The map declaration has been correctly updated to use the new
Styletype in place of the previousIconStyle, maintaining consistency with the type renaming.
397-417: Type update to match renamed structure.The
Foldersmap declaration has been correctly updated to useStyletype, maintaining consistency with the renamed structure.src/internal/handle_file_operations.go (11)
14-15: Import update for better organizational structure.Replacing the variable package import with utils helps maintain better package organization and dependency structure. This is part of the broader refactoring to use utility functions from a centralized location.
59-60: Field naming follows Go conventions.Changing
messageIdtomessageIDfollows Go's standard naming convention for acronyms and initialisms, where only the first letter is capitalized in camelCase fields.Also applies to: 100-101, 144-147, 191-194, 259-262, 310-313, 433-436
114-120: Code structure simplification improves readability.The conditional flow has been simplified by removing an unnecessary else block, which reduces nesting and makes the code more readable. This is a good practice that aligns with the overall goal of the PR to improve code quality.
167-169: Simplified conditional logic.Replacing nested if-else with a direct else-if improves readability without changing functionality.
217-223: Code flattening improves readability.Removing nested if blocks in favor of a flatter structure improves readability and maintainability.
338-344: Consistent code structure improvement.Similar to the previous changes, this code has been refactored to remove nesting, which aligns with the pattern applied throughout the PR for more readable code.
456-458: Good explanatory comment about testing challenges.Adding this comment about the difficulty of testing error cases provides valuable context for future developers. It's a good practice to document such challenges and considerations.
572-576: Using constants from utils package improves maintainability.Replacing string literals with constants from the utils package (
utils.OsWindows) makes the code more maintainable and less prone to spelling errors or inconsistencies.
583-584: Clear linter directive with explanation.The linter directive includes a comment explaining why it's being used, which is good practice for code clarity and maintainability.
598-605: Improved conditional structure with switch statement.Converting the if-else chain to a switch statement improves readability and is more idiomatic Go for multiple condition handling based on a single variable.
611-612: Consistent linter directive with explanation.The same linter directive is applied consistently in similar contexts, which is good for code maintainability.
src/internal/common/load_config.go (4)
30-31: Using specialized utility functions improves error handling.Replacing generic error handling with the specialized
PrintlnAndExitfunction from the utils package improves code organization and provides consistent error reporting across the application.
68-69: Consistent use of utility functions.Using
PrintlnAndExitconsistently for error handling creates a standardized approach across the codebase.Also applies to: 73-74
84-90: Simplified error handling removes unnecessary nesting.The error handling has been simplified to remove an unnecessary conditional block, making the code more readable and maintainable.
96-97: Using format-specific error utility.Using
PrintfAndExitfor formatted error messages is a good practice that provides a consistent approach to error handling while leveraging the appropriate formatting capabilities.src/pkg/file_preview/image_preview.go (2)
102-103: Improved error wrapping for better context.Using the
%wverb infmt.Errorfproperly wraps the original error, preserving the error chain for better debugging and error handling upstream.
166-169: Function simplification improves readability.Removing the named return variable and directly returning the formatted string makes the function more concise and easier to understand.
src/internal/common/utils/file_utils.go (6)
17-17: Improved error wrapping using %w format verb.The use of
%wfor error wrapping is a good practice as it preserves the original error's context for better error handling downstream.
21-21: Consistent use of lowercase in error messages.This change follows Go's style guidelines where error messages should start with lowercase letters.
28-28: Function signature simplification.Simplifying the return type from a named return
(hasError bool)to justboolmakes the API cleaner.
34-34: Updated error handling approach.Changing from
LogAndExittoPrintfAndExitsuggests a standardization of error reporting methods across the codebase.
37-37: Explicit variable initialization.Adding an explicit initialization for
hasErrorimproves code clarity.
69-69: More idiomatic loop iteration.Using
range targetType.NumField()instead of an index-based loop is more idiomatic in Go and improves code readability..golangci.yaml (2)
278-278: Good: Enabled previously commented linters.Enabling additional linters like
errorlint,gochecknoglobals,gocritic,intrange,reassign,revive, andstylecheckwill improve code quality by catching more potential issues.Also applies to: 287-287, 293-293, 304-304, 327-327, 330-330, 336-336, 345-346
400-421: Good: Added explicit linter exclusions for specific files.The added exclusions for files with globals (icon.go, style.go, fixed_variable.go, etc.) provide clear documentation about which files are intentionally exempt from the
gochecknoglobalslinter checks.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 421-421: trailing spaces
(trailing-spaces)
src/internal/common/utils/bool_file_store_test.go (1)
25-25: Improved constant naming convention.The changes from uppercase snake case (
TRUE_STRING,FALSE_STRING) to Go's standard exported constant naming (TrueString,FalseString) follow Go's style guidelines. Similarly, the transition fromvariable.OS_WINDOWStoOsWindowsimproves code consistency.These constant names now match those defined in the
consts.gofile, ensuring consistency across the codebase.Also applies to: 32-32, 131-134, 139-139, 158-158, 166-166, 184-184
src/internal/ui/prompt/tokenize.go (3)
30-84: Improved code structure with switch statement.Refactoring the nested if-else structure to a switch-case makes the code more readable and maintainable. The switch statement provides a clearer view of the different cases being handled when a dollar sign is encountered.
45-52: Simplified error handling flow.The environment variable lookup error handling has been simplified by removing the unnecessary else clause, making the code more straightforward while maintaining the same functionality.
72-78: Cleaner conditional logging.The handling of non-zero return codes has been improved by eliminating the nested if-else structure, resulting in cleaner and more direct code flow while preserving the debug logging functionality.
src/internal/file_operations.go (4)
12-13: Good package organization.Clean separation with an empty line after this import makes the code more readable.
25-25: Good error wrapping practice.Properly wrapping errors with
%wverb infmt.Errorfpreserves the original error context and allows error unwrapping downstream. This is a recommended Go practice.Also applies to: 30-30, 56-56, 70-70, 75-75, 98-98, 103-103, 112-112, 131-131, 137-137, 142-142, 254-254
33-34: Good refactoring of OS checks.Replacing the OS constants with centralized constants from the utils package (
utils.OsWindowsandutils.OsDarwin) and changing the if-else structure to a switch statement improves code readability and maintainability.Also applies to: 150-157
206-207: Consistent naming convention.Renaming
messageIdtomessageIDfollows Go's naming conventions for acronyms (ID instead of Id). This makes the codebase more consistent.src/internal/config_function.go (4)
21-21: Good exception documentation.Using a nolint comment that explains why named returns are being used here provides clear context for future maintainers.
29-29: Improved error handling.Replacing
utils.LogAndExitwithutils.PrintfAndExitmaintains consistent error handling patterns across the codebase.
57-67: Simplified directory path handling.The refactoring improves clarity by:
- Directly assigning dir to firstFilePanelDir
- Only replacing the first tilde occurrence as a prefix
- Adding clear comments about the logic
This makes the code more straightforward and easier to understand.
78-80: Better code organization.Moving these boolean file reads to the end of the function creates a more logical flow, ensuring the directory logic is fully processed before setting these values.
src/internal/get_data.go (2)
22-27: Performance optimization.Preallocating the slice capacity based on the total number of directories is a good optimization that reduces memory allocations during append operations.
89-97: Simplified function signature and error handling.Removing the named return value and returning
nilon error makes the function behavior clearer. Declaring thedisksvariable before populating it ensures consistent initialization.src/internal/string_function.go (5)
5-5: Added error handling capability.Adding the errors package import supports the improved error checking with errors.Is.
71-75: Code simplification.Removing unnecessary else branches in both functions makes the code more concise and readable while maintaining the same functionality.
Also applies to: 93-97
118-127: Documenting technical debt.Adding a TODO comment about removing duplication is good practice for identifying future refactoring needs. The refactored calculation logic for binary units is cleaner.
186-186: Robust error checking.Using
errors.Isinstead of direct error comparison is more robust as it handles wrapped errors correctly, following Go's error handling best practices.
201-201: Modern loop syntax.Changing to a range-based loop over the length of the string is a minor syntactical improvement for clarity.
src/internal/ui/prompt/model.go (2)
75-75: Proper linter directive added to explain type assertion.The addition of the
//nolint: errorlintdirective with an explanatory comment is a good practice. It explicitly documents why the type assertion is being used instead oferrors.As()and suppresses the linter warning appropriately.
89-94: Improved readability with switch statement.Replacing the if-else structure with a switch statement for handling different input conditions is a good refactoring. The switch with case conditions is more readable and maintainable, especially when there are multiple conditions to check.
src/internal/sidebar_test.go (1)
160-160: Good variable renaming for better Go naming convention compliance.Renaming variables from snake_case to camelCase (e.g.,
sidebar_atosidebarA) follows Go's naming conventions and improves code consistency. This makes the code more idiomatic and easier to read for Go developers.Also applies to: 165-170, 239-242, 247-251, 252-256, 259-261
src/cmd/main.go (6)
31-31: Function name improved to follow Go conventions.Renaming
LoadInitial_PrerenderedVariablestoLoadInitialPrerenderedVariablesremoves the underscore, which is consistent with Go's naming conventions that prefer camelCase over snake_case.
44-44: Unused parameter properly marked with underscore.Replacing
c *cli.Contextwith_ *cli.Contextexplicitly indicates that the parameter is not used in the function body, which improves code clarity and helps prevent accidental usage of an unused variable.
92-92: Consolidated CLI argument handling.Using
variable.UpdateVarFromCliArgs(c)centralizes the logic for updating variables from CLI arguments, making the code more maintainable by encapsulating related functionality in a single function.
105-105: Improved error handling with utility functions.Replacing direct log calls with utility functions like
utils.PrintfAndExitandutils.PrintlnAndExitstandardizes error handling across the codebase. This encapsulates the pattern of logging an error and exiting, which makes the code more consistent and maintainable.Also applies to: 124-124, 138-138, 148-148, 153-153, 157-157, 161-161, 217-217
160-160: Correctly capitalized acronym in function name.Renaming
initJsonFiletoinitJSONFilefollows Go's convention of capitalizing acronyms in identifiers. This improves consistency and adheres to the standard naming guidelines.Also applies to: 233-233
168-168: Improved maintainability with constant usage.Using the constant
utils.OsDarwininstead of a string literal"darwin"is a good practice. If the value needs to change in the future, it only needs to be updated in one place, reducing the risk of inconsistencies.src/config/fixed_variable.go (3)
14-23: Simplified variable declarations with type inference.Removing explicit type declarations for constants and variables allows Go's type inference to determine the types automatically. This results in cleaner code while maintaining type safety, as the types are obvious from the assigned values.
Also applies to: 27-52
55-65: Well-documented variable section for dynamic values.The addition of a separate section specifically for variables that can be updated dynamically, with a clear comment explaining their purpose, improves code organization and makes the intent clear to other developers.
69-97: Good encapsulation with functions for dynamic variables.The addition of functions to manage dynamic variables (
SetLastDirandUpdateVarFromCliArgs) provides proper encapsulation and controlled access to state variables. TheUpdateVarFromCliArgsfunction also includes robust error handling for file existence checks.src/internal/model_render.go (10)
6-6: Good addition of the errors package.Adding the errors package is necessary for using
errors.Is()later in the file, which is a more robust way to check error types.
72-77: Improved control flow with switch-case.Replacing nested if-else conditions with a switch-case statement enhances readability and maintainability, making the code structure clearer.
112-117: Better variable declaration and helpful TODOs.Extracting variable declarations improves readability, and the added TODOs provide guidance for future improvements. Consider implementing these TODOs in a future PR.
127-128: Improved variable declarations.Declaring variables before assigning values to them follows better Go coding practices and makes the code more maintainable.
Also applies to: 141-142
287-289: Better variable declarations for clarity.Properly declared variables
symbolandcursorbefore use, enhancing code readability and maintainability.
315-317: Improved conditional logic.The condition has been inverted to first check if the process list has items before trying to access elements, which is a safer approach that avoids potential issues when the list is empty.
415-416: Better variable declaration for bottomWidth.Properly declared the variable before use, following consistent style with other similar changes in the file.
663-667: Improved variable naming and error handling.Renamed
info_errtoinfoErrfor consistency with Go naming conventions (camelCase). This style is more idiomatic in Go codebases.
722-724: Better error type checking.Using
errors.Is()for checking specific error types provides more robust error handling than direct error comparisons, as it works correctly with wrapped errors.
799-800: Minor formatting improvement.Small readability enhancement without functional impact.
src/internal/function.go (10)
19-20: Good reorganization of imports.Adding the utils package import supports the OS constant refactoring throughout the file.
40-40: Consistent OS check refactoring.Replaced direct variable references with utility function calls, improving maintainability by centralizing OS-specific constants.
Also applies to: 57-57, 94-94, 107-107
120-136: Improved error handling and return values.Now returning
nilinstead of empty slices when errors occur or no data is available, which provides a clearer indication of the absence of valid results.
153-154: Added case-insensitive sorting.The lowercase comparison ensures consistent sorting regardless of letter case, which improves user experience.
180-184: Better code comments and simplified variable names.The added comment clarifies why error checking is not needed here, and the variable names have been shortened for better readability.
195-197: Memory optimization with slice preallocation.Preallocating the slice capacity based on the known size improves performance by avoiding multiple reallocations during append operations.
241-244: Better code organization and memory efficiency.The code is now more readable and efficient with preallocation of the slice.
454-455: Improved error wrapping.Using the
%wverb for error wrapping follows Go 1.13+ best practices and preserves the original error for better error handling up the call stack.Also applies to: 460-461
470-485: Avoided variable shadowing.Renamed variables to prevent shadowing issues, which can lead to subtle bugs. This change makes the code more robust.
532-585: Refactored icon handling with better formatting.The getElementIcon function has been reformatted with more consistent indentation and clearer logic flow, making it easier to understand and maintain.
src/internal/model.go (5)
36-40: Better function documentation.Updated comments clarify the return type and purpose of the InitialModel function, making it more discoverable and easier to use correctly.
122-126: Consistent field naming with camelCase.Renamed
messageIdtomessageIDfollowing Go conventions where abbreviations in camelCase should be capitalized (ID rather than Id).
230-277: Improved control flow with switch-case.Refactored multiple if-else statements into a more readable switch-case structure with condition expressions. This makes the code more maintainable by clearly separating each case.
284-301: Consistent control flow improvements.Applied the same switch-case pattern for better readability, making the code structure consistent with the previous refactoring.
588-588: Better encapsulation with setter method.Using
variable.SetLastDir(currentDir)instead of direct assignment improves encapsulation by allowing the package to control how the variable is updated.
| for i := range val.NumField() { | ||
| field := val.Type().Field(i) | ||
| value := val.Field(i) | ||
|
|
||
| // Although this is redundant as Hotkey is always a slice | ||
| // This adds a layer against accidental struct modifications | ||
| // Makes sure its always be a string slice. It's somewhat like a unit test | ||
| if value.Kind() != reflect.Slice || value.Type().Elem().Kind() != reflect.String { | ||
| utils.LogAndExit(LoadHotkeysError(field.Name)) | ||
| utils.PrintlnAndExit(LoadHotkeysError(field.Name)) | ||
| } | ||
|
|
||
| hotkeysList, ok := value.Interface().([]string) | ||
| if !ok || len(hotkeysList) == 0 || hotkeysList[0] == "" { | ||
| utils.LogAndExit(LoadHotkeysError(field.Name)) | ||
| utils.PrintlnAndExit(LoadHotkeysError(field.Name)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Idiomatic Go loop pattern improves readability.
Replacing the traditional index-based for loop with a range-based loop is more idiomatic Go and easier to understand. The loop structure is now cleaner while maintaining the same functionality.
However, there's an issue with this loop:
- for i := range val.NumField() {
+ for i := 0; i < val.NumField(); i++ {When using range with a single value, that value is an index, not a range limit. This will cause a runtime error as you can't use range with an integer value like val.NumField().
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for i := range val.NumField() { | |
| field := val.Type().Field(i) | |
| value := val.Field(i) | |
| // Although this is redundant as Hotkey is always a slice | |
| // This adds a layer against accidental struct modifications | |
| // Makes sure its always be a string slice. It's somewhat like a unit test | |
| if value.Kind() != reflect.Slice || value.Type().Elem().Kind() != reflect.String { | |
| utils.LogAndExit(LoadHotkeysError(field.Name)) | |
| utils.PrintlnAndExit(LoadHotkeysError(field.Name)) | |
| } | |
| hotkeysList, ok := value.Interface().([]string) | |
| if !ok || len(hotkeysList) == 0 || hotkeysList[0] == "" { | |
| utils.LogAndExit(LoadHotkeysError(field.Name)) | |
| utils.PrintlnAndExit(LoadHotkeysError(field.Name)) | |
| } | |
| } | |
| for i := 0; i < val.NumField(); i++ { | |
| field := val.Type().Field(i) | |
| value := val.Field(i) | |
| // Although this is redundant as Hotkey is always a slice | |
| // This adds a layer against accidental struct modifications | |
| // Makes sure its always be a string slice. It's somewhat like a unit test | |
| if value.Kind() != reflect.Slice || value.Type().Elem().Kind() != reflect.String { | |
| utils.PrintlnAndExit(LoadHotkeysError(field.Name)) | |
| } | |
| hotkeysList, ok := value.Interface().([]string) | |
| if !ok || len(hotkeysList) == 0 || hotkeysList[0] == "" { | |
| utils.PrintlnAndExit(LoadHotkeysError(field.Name)) | |
| } | |
| } |
There was a problem hiding this comment.
No, this is the int range for loop introduced by go 1.22 . See - golang/go#61405
There was a problem hiding this comment.
@lazysegtree You're absolutely right! Thank you for the correction. The integer range loop syntax (for i := range val.NumField()) is indeed a valid feature introduced in Go 1.22 via GitHub issue #61405. This syntax iterates from 0 to n-1 and is equivalent to the traditional for i := 0; i < n; i++ loop.
I see from my knowledge base that this feature has been documented in previous discussions. I'll make sure to recognize this syntax as valid Go 1.22+ code in future reviews.
(_____)
(• ◡•)
/ >🐰 Learning something new!
🧠 Learnings used
Learnt from: lazysegtree
PR: yorukot/superfile#752
File: src/internal/ui/prompt/model.go:14-17
Timestamp: 2025-04-05T18:59:22.440Z
Learning: In the superfile codebase, hotkey configuration validation happens in the LoadHotkeysFile() function, which ensures that all hotkey slices are non-empty and contain valid values. If validation fails, the application exits early with an error message, eliminating the need for runtime checks when accessing slice elements.
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/sidebar_test.go:12-12
Timestamp: 2025-04-08T13:00:48.543Z
Learning: Integer range loops in Go use the syntax `for i := range n` to iterate from 0 to n-1, which is equivalent to `for i := 0; i < n; i++`. This feature was introduced in Go 1.22 via GitHub issue #61405 and is valid Go code.
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/common/style_function.go:19-19
Timestamp: 2025-04-08T12:59:02.404Z
Learning: Integer range loops (`for i := range n`) were introduced in Go 1.22. This syntax iterates from 0 to n-1 and is equivalent to `for i := 0; i < n; i++`. This is valid Go code and should not be flagged as an error.
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/handle_panel_up_down_test.go:13-13
Timestamp: 2025-04-08T12:59:09.350Z
Learning: Go 1.22+ supports the integer range loop feature that allows iterating over integers directly with the range keyword. The syntax `for i := range n` iterates from 0 to n-1, which is equivalent to the traditional `for i := 0; i < n; i++` loop. This feature was implemented via GitHub issue #61405.
| for x := range width { | ||
| upperColor := cache.getTermenvColor(img.At(x, y), defaultBGHex) |
There was a problem hiding this comment.
Incorrect range usage in for loop.
The change from a counter-based loop to a range-based loop introduces a bug. When using range with a single value like width, the loop iterates over indices from 0 to width-1, but doesn't provide the value at each position as you'd expect with slices or maps.
Correct this by reverting to the counter-based loop:
- for x := range width {
+ for x := 0; x < width; x++ {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for x := range width { | |
| upperColor := cache.getTermenvColor(img.At(x, y), defaultBGHex) | |
| for x := 0; x < width; x++ { | |
| upperColor := cache.getTermenvColor(img.At(x, y), defaultBGHex) |
There was a problem hiding this comment.
No, this is the int range for loop introduced by go 1.22 . See - golang/go#61405
There was a problem hiding this comment.
@lazysegtree You're absolutely right - thank you for the correction. I see from my knowledge base that I've already learned about this integer range loop feature introduced in Go 1.22 in previous PRs. This syntax for x := range width is indeed valid and equivalent to the traditional for x := 0; x < width; x++ loop. I apologize for incorrectly flagging this as an issue.
🧠 Learnings used
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/common/style_function.go:19-19
Timestamp: 2025-04-08T12:59:02.404Z
Learning: Integer range loops (`for i := range n`) were introduced in Go 1.22. This syntax iterates from 0 to n-1 and is equivalent to `for i := 0; i < n; i++`. This is valid Go code and should not be flagged as an error.
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/sidebar_test.go:12-12
Timestamp: 2025-04-08T13:00:48.543Z
Learning: Integer range loops in Go use the syntax `for i := range n` to iterate from 0 to n-1, which is equivalent to `for i := 0; i < n; i++`. This feature was introduced in Go 1.22 via GitHub issue #61405 and is valid Go code.
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/handle_panel_up_down_test.go:13-13
Timestamp: 2025-04-08T12:59:09.350Z
Learning: Go 1.22+ supports the integer range loop feature that allows iterating over integers directly with the range keyword. The syntax `for i := range n` iterates from 0 to n-1, which is equivalent to the traditional `for i := 0; i < n; i++` loop. This feature was implemented via GitHub issue #61405.
| @@ -9,7 +9,7 @@ import ( | |||
|
|
|||
| func dirSlice(count int) []directory { | |||
| res := make([]directory, count) | |||
| for i := 0; i < count; i++ { | |||
| for i := range count { | |||
There was a problem hiding this comment.
Critical error in loop syntax.
The loop has been incorrectly changed from a traditional for-loop to a range-based loop. Since count is an integer and not a slice or collection, this will cause a compilation error.
-for i := range count {
+for i := 0; i < count; i++ {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for i := range count { | |
| for i := 0; i < count; i++ { |
There was a problem hiding this comment.
No, this is the int range for loop introduced by go 1.22 . See - golang/go#61405
There was a problem hiding this comment.
@lazysegtree I understand now. You're absolutely right - this is the integer range loop feature introduced in Go 1.22 via the GitHub issue you referenced (#61405). The syntax for i := range count iterates from 0 to count-1, which is equivalent to the traditional for i := 0; i < count; i++ loop. Thank you for the correction.
🧠 Learnings used
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/common/style_function.go:19-19
Timestamp: 2025-04-08T12:59:02.404Z
Learning: Integer range loops (`for i := range n`) were introduced in Go 1.22. This syntax iterates from 0 to n-1 and is equivalent to `for i := 0; i < n; i++`. This is valid Go code and should not be flagged as an error.
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/sidebar_test.go:12-12
Timestamp: 2025-04-08T13:00:48.543Z
Learning: Integer range loops in Go use the syntax `for i := range n` to iterate from 0 to n-1, which is equivalent to `for i := 0; i < n; i++`. This feature was introduced in Go 1.22 via GitHub issue #61405 and is valid Go code.
Learnt from: lazysegtree
PR: yorukot/superfile#757
File: src/internal/handle_panel_up_down_test.go:13-13
Timestamp: 2025-04-08T12:59:09.350Z
Learning: Go 1.22+ supports the integer range loop feature that allows iterating over integers directly with the range keyword. The syntax `for i := range n` iterates from 0 to n-1, which is equivalent to the traditional `for i := 0; i < n; i++` loop. This feature was implemented via GitHub issue #61405.
lazysegtree
changed the title
feat: Allow specifying multiple panels at startup
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/internal/default_config.go (1)
26-26: Dynamically generating file panels from a slice.
UsingfilePanelSlicehere seamlessly accommodates multiple directories. Just ensure no unexpected edge cases arise iffirstFilePanelDirsis empty or contains invalid paths.src/cmd/main.go (1)
44-44: Ignoring the CLI context parameter.
Renaming the parameter to_is fine if we truly don’t need it, but consider whether we might use CLI context for future expansions or logging.src/internal/config_function.go (1)
75-75: Debug logging for runtime info.
Including the OS name and the starting directories is helpful for troubleshooting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/cmd/main.go(9 hunks)src/internal/config_function.go(2 hunks)src/internal/default_config.go(2 hunks)src/internal/model.go(6 hunks)src/internal/model_prompt_test.go(1 hunks)src/internal/type_utils.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/internal/model_prompt_test.go
- src/internal/type_utils.go
🧰 Additional context used
🧬 Code Definitions (3)
src/internal/model.go (3)
src/internal/ui/prompt/type.go (1)
Model(6-24)src/internal/common/default_config.go (1)
Hotkeys(12-12)src/config/fixed_variable.go (1)
SetLastDir(69-71)
src/internal/config_function.go (3)
src/config/fixed_variable.go (4)
LogFile(45-45)HomeDir(27-27)ToggleDotFile(41-41)ToggleFooter(42-42)src/internal/common/utils/log_utils.go (1)
PrintfAndExit(20-23)src/internal/common/utils/bool_file_store.go (1)
ReadBoolFile(12-31)
src/cmd/main.go (5)
src/internal/common/predefined_variable.go (1)
LoadInitialPrerenderedVariables(34-37)src/config/fixed_variable.go (3)
UpdateVarFromCliArgs(73-97)ConfigFile(57-57)HotkeysFile(58-58)src/internal/model.go (1)
InitialModel(40-46)src/internal/common/utils/log_utils.go (2)
PrintfAndExit(20-23)PrintlnAndExit(12-15)src/internal/common/utils/consts.go (1)
OsDarwin(10-10)
🔇 Additional comments (27)
src/internal/default_config.go (1)
11-11: Switching to a slice for multiple directories is great.
This change significantly improves flexibility by allowing multiple directory inputs at startup.src/cmd/main.go (13)
31-31: Prerendered variables loading.
Callingcommon.LoadInitialPrerenderedVariables()here is consistent with the refactored naming. Good job integrating it early in the startup sequence.
87-87: Default to empty directory string array.
DeclaringfirstFilePanelDirs := []string{""}is a straightforward way to ensure an initial entry. Just confirm that the rest of the code properly handles this as a fallback rather than an actual directory path if the user provides no arguments.
89-89: Populating directories from arguments.
Assigningc.Args().Slice()here properly captures user-supplied paths. This completes the multi-directory approach introduced by the slice parameter.
92-92: Centralized CLI argument handling.
Callingvariable.UpdateVarFromCliArgs(c)keeps the code clean and modular. Nice use of the dedicated function for configuration overlay.
103-103: Initializing the TUI with multiple directories.
Callinginternal.InitialModel(firstFilePanelDirs, firstUse, hasTrash)properly passes the list of directories. This is a solid approach to integrate the new feature in the main flow.
105-105: Using PrintfAndExit for errors.
Replacing direct logging and exiting withutils.PrintfAndExitcentralizes error handling and maintains consistency. Good improvement.
124-124: Consistent error exit strategy.
Usingutils.PrintlnAndExitfor a final catch-all error path aligns the app’s error-handling pattern.
138-138: Clean directory creation failure messages.
utils.PrintlnAndExit("Error creating directories:", err)ensures a clear user-facing message.
148-148: File creation error handling.
This call is analogous to the directory creation approach and uses a similar user-facing error message pattern.
153-153: Explicit config file write errors.
Maintaining the same exit approach ensures consistent error messages across different file operations.
157-157: Hotkey config file write failure.
Likewise, consistently reporting errors withutils.PrintlnAndExitis a clear improvement.
160-161: Ensuring pinned JSON file initialization.
This guarded call toinitJSONFilecovers file creation in a uniform manner. Good job unifying error handling.
168-168: Platform-specific Darwin check.
This runtime check is a straightforward way to skip trash directory creation on macOS.src/internal/config_function.go (5)
19-20: Improved function signature for multiple directories.
SwitchinginitialConfigto accept a string slice clarifies that multiple paths can be processed at once. Good job aligning with the multi-pane feature.
28-28: Exiting on log file open error.
UsingPrintfAndExitquickly surfaces the problem and prevents partial initialization. This is consistent with the global error strategy.
56-72: Robust directory path resolution loop.
This block handles multiple directories gracefully by:
- Defaulting empty entries to the config’s default directory.
- Converting
~to the user’s home directory.- Falling back to the home directory if
filepath.Absfails.
Very thorough and user-friendly approach.
77-78: Reading toggle states from files.
UsingReadBoolFilefortoggleDotFileandtoggleFooteris a clean approach to persist user preferences.
80-80: Return final toggle states.
The refactor leads to a simpler, more readable function with a clear outcome.src/internal/model.go (8)
25-33: Good improvement with clarifying commentsAdding comments to clarify that these variables represent model state and not global properties improves code readability. The
//nolint: gochecknoglobalsdirectives are also helpful to suppress linter warnings while acknowledging technical debt with "Todo" comments about moving these to a model struct.
36-39: Good documentation of implementation detailsThese comments clarify the design intent behind returning
tea.Modelinstead of the concretemodeltype, which improves maintainability and helps future developers understand why this approach was chosen.
40-41: Successfully implemented multiple directories featureThe function signature change from accepting a single directory string to a slice of directories (
firstFilePanelDirs []string) properly implements the PR objective of allowing multiple directories at startup.Also applies to: 45-45
122-123: Fixed field name inconsistencyChanged
messageIdtomessageIDto follow Go naming conventions for abbreviations (ID should be all caps). This type of consistency improvement is part of the linter fixes mentioned in the PR description.Also applies to: 125-125
171-171: Appropriate use of linter directiveThe
//nolint: gocriticdirective is correctly used with a comment explaining why the code is exempted from the linter rule, which aligns with good practices for linter suppressions.
230-275: Improved control flow with switch statementRefactoring nested if-else statements to a switch-case structure with logical conditions improves readability and maintainability. This is a good example of the linter fixes mentioned in the PR objectives.
284-301: Improved control flow with switch statementSimilar to the previous refactoring, this change replaces multiple if-else statements with a cleaner switch-case structure, making the code more maintainable and easier to follow.
588-588: Better encapsulation with setter methodChanged direct assignment to
variable.LastDirto use theSetLastDirfunction from the imported variable package. This improves encapsulation and is consistent with the linter fix objectives of the PR.
lazysegtree
changed the title
fix: remove redundant function containsKey
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/internal/key_function.go (3)
214-216: Enhance the TODO comment with actionable itemsThe TODO comment about duplication in modal handlers is valid and important. Consider enhancing it with specific refactoring suggestions to make it more actionable.
-// Todo : There is a lot of duplication for these models, each one of them has to handle -// ConfirmTyping and CancleTyping in a similar way. There is a scope of some good refactoring here. +// TODO: Refactor modal key handlers to reduce duplication. Consider: +// 1. Creating a common modal interface with confirm/cancel methods +// 2. Implementing a generic key handler that delegates to specific handlers +// 3. Using a modal controller pattern to centralize modal state management
219-219: Fix typo in common.Hotkeys referenceThere appears to be a typo in the comment at line 216 referring to "CancleTyping" when the code correctly uses "CancelTyping".
217-254: Consider further refactoring of the warn modal key handlerThis function has become cleaner with the switch statement, but it's still complex. Consider extracting the warn type handling logic into separate functions for better maintainability.
You could refactor this by extracting the confirm action logic for each warn type into separate methods, making the code more modular and easier to maintain:
func (m *model) warnModalOpenKey(msg string) { switch { case slices.Contains(common.Hotkeys.CancelTyping, msg) || slices.Contains(common.Hotkeys.Quit, msg): m.cancelWarnModal() if m.warnModal.warnType == confirmRenameItem { m.cancelRename() } case slices.Contains(common.Hotkeys.Confirm, msg): m.warnModal.open = false - switch m.warnModal.warnType { - case confirmDeleteItem: - panel := m.fileModel.filePanels[m.filePanelFocusIndex] - if m.fileModel.filePanels[m.filePanelFocusIndex].panelMode == selectMode { - if !hasTrash || isExternalDiskPath(panel.location) { - go func() { - m.completelyDeleteMultipleItems() - m.fileModel.filePanels[m.filePanelFocusIndex].selected = m.fileModel.filePanels[m.filePanelFocusIndex].selected[:0] - }() - } else { - go func() { - m.deleteMultipleItems() - m.fileModel.filePanels[m.filePanelFocusIndex].selected = m.fileModel.filePanels[m.filePanelFocusIndex].selected[:0] - }() - } - } else { - if !hasTrash || isExternalDiskPath(panel.location) { - go func() { - m.completelyDeleteSingleItem() - }() - } else { - go func() { - m.deleteSingleItem() - }() - } - } - case confirmRenameItem: - m.confirmRename() - } + m.handleWarnModalConfirm() } } +// handleWarnModalConfirm handles the confirmation action for different warn modal types +func (m *model) handleWarnModalConfirm() { + switch m.warnModal.warnType { + case confirmDeleteItem: + m.handleDeleteItemConfirm() + case confirmRenameItem: + m.confirmRename() + } +} + +// handleDeleteItemConfirm handles the confirmation action for delete item warn modal +func (m *model) handleDeleteItemConfirm() { + panel := m.fileModel.filePanels[m.filePanelFocusIndex] + isSelectMode := m.fileModel.filePanels[m.filePanelFocusIndex].panelMode == selectMode + noTrashAvailable := !hasTrash || isExternalDiskPath(panel.location) + + if isSelectMode { + m.handleMultipleItemsDelete(noTrashAvailable) + } else { + m.handleSingleItemDelete(noTrashAvailable) + } +} + +// handleMultipleItemsDelete handles the deletion of multiple selected items +func (m *model) handleMultipleItemsDelete(noTrashAvailable bool) { + go func() { + if noTrashAvailable { + m.completelyDeleteMultipleItems() + } else { + m.deleteMultipleItems() + } + m.fileModel.filePanels[m.filePanelFocusIndex].selected = m.fileModel.filePanels[m.filePanelFocusIndex].selected[:0] + }() +} + +// handleSingleItemDelete handles the deletion of a single item +func (m *model) handleSingleItemDelete(noTrashAvailable bool) { + go func() { + if noTrashAvailable { + m.completelyDeleteSingleItem() + } else { + m.deleteSingleItem() + } + }() +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/internal/key_function.go(6 hunks)src/internal/model.go(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/internal/model.go
🧰 Additional context used
🧬 Code Definitions (1)
src/internal/key_function.go (1)
src/internal/common/default_config.go (1)
Hotkeys(12-12)
🔇 Additional comments (6)
src/internal/key_function.go (6)
17-138: Improved control flow with switch statement and slices.ContainsThe refactoring from if-else chains to a switch statement with
slices.Containsmakes the code more readable and maintainable. Using the standard library'sslices.Containsfunction instead of a customcontainsKeyfunction reduces code duplication and follows Go best practices.
159-177: Consistent improvement with switch-case structureThe refactoring in the select mode handling follows the same pattern, improving code consistency and readability.
180-201: Well-structured key handler for normal modeThe normal mode key handling continues the consistent pattern of using
switchandslices.Contains. This creates a unified approach to key handling throughout the codebase.
204-212: Clean implementation of typing modal keysThe simplified switch structure here handles the two possible actions (cancel or confirm) clearly and concisely.
257-269: Improved readability in confirmToQuitSuperfileThe switch statement makes the confirmation logic clearer. Nice improvement in this function.
272-341: Consistent use of switch statements in remaining key handlersAll the remaining key handler functions (
sortOptionsKey,renamingKey, etc.) follow the same pattern of using switch statements withslices.Contains. This consistent approach improves code readability and maintainability throughout the file.
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 [@​Rocco-Gossmann](https://github.com/Rocco-Gossmann), [@​yorukot](https://github.com/yorukot) and [@​lazysegtree](https://github.com/lazysegtree) - Allow specifying multiple panels at startup [`#759`](yorukot/superfile#759) by [@​lazysegtree](https://github.com/lazysegtree) - Initial draft of rendering package [`#775`](yorukot/superfile#775) by [@​lazysegtree](https://github.com/lazysegtree) - Render unit tests for prompt model [`#809`](yorukot/superfile#809) by [@​lazysegtree](https://github.com/lazysegtree) - Chooser file option, --lastdir-file option, and improvements in quit, and bug fixes [`#812`](yorukot/superfile#812) by [@​lazysegtree](https://github.com/lazysegtree) - Prompt feature leftover items [`#804`](yorukot/superfile#804) by [@​lazysegtree](https://github.com/lazysegtree) - SPF Prompt tutorial and fixes [`#814`](yorukot/superfile#814) by [@​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 [@​lazysegtree](https://github.com/lazysegtree) - Theme: Add gruvbox-dark-hard [`#828`](yorukot/superfile#828) by [@​Frost-Phoenix](https://github.com/Frost-Phoenix) #### Updates & Improvements - Sidebar separation [`#767`](yorukot/superfile#767) by [@​lazysegtree](https://github.com/lazysegtree) - Sidebar code separation [`#770`](yorukot/superfile#770) by [@​lazysegtree](https://github.com/lazysegtree) - Rendering package and rendering bug fixes [`#781`](yorukot/superfile#781) by [@​lazysegtree](https://github.com/lazysegtree) - Refactor CheckForUpdates [`#797`](yorukot/superfile#797) by [@​JassonCordones](https://github.com/JassonCordones) - Rename metadata strings [`#731`](yorukot/superfile#731) by [@​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 [@​booth-w](https://github.com/booth-w) - Fix: Add some of the remaining linter and fix errors [`#756`](yorukot/superfile#756) by [@​lazysegtree](https://github.com/lazysegtree) - Golangci lint fixes [`#757`](yorukot/superfile#757) by [@​lazysegtree](https://github.com/lazysegtree) - Fix: Remove redundant function containsKey [`#765`](yorukot/superfile#765) by [@​lazysegtree](https://github.com/lazysegtree) - Fix: Correctly resolve path in open and cd prompt actions [`#802`](yorukot/superfile#802) by [@​lazysegtree](https://github.com/lazysegtree) - Prompt dynamic dimensions and unit tests fix [`#805`](yorukot/superfile#805) by [@​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 [@​lazysegtree](https://github.com/lazysegtree) #### Optimization & Code Quality - Adding linter to CI/CD and fix some lint issues [`#739`](yorukot/superfile#739) by [@​lazysegtree](https://github.com/lazysegtree) - Linter fixes, new feature of allowing multiple directories at startup, other code improvements [`#764`](yorukot/superfile#764) by [@​lazysegtree](https://github.com/lazysegtree) - Model unit tests [`#803`](yorukot/superfile#803) by [@​lazysegtree](https://github.com/lazysegtree) #### Dependency Updates - fix(deps): update dependency astro to v5.7.7 [`#726`](yorukot/superfile#726) by [@​renovate](https://github.com/renovate) - fix(deps): update module github.com/shirou/gopsutil/v4 to v4.25.3 [`#749`](yorukot/superfile#749) by [@​renovate](https://github.com/renovate) - fix(deps): update module github.com/pelletier/go-toml/v2 to v2.2.4 [`#760`](yorukot/superfile#760) by [@​renovate](https://github.com/renovate) - fix(deps): update module github.com/alecthomas/chroma/v2 to v2.16.0 [`#751`](yorukot/superfile#751) by [@​renovate](https://github.com/renovate) - fix(deps): update dependency sharp to ^0.34.0 [`#755`](yorukot/superfile#755) by [@​renovate](https://github.com/renovate) - fix(deps): update dependency [@​astrojs/starlight](https://github.com/astrojs/starlight) to ^0.34.0 [`#761`](yorukot/superfile#761) by [@​renovate](https://github.com/renovate) </p> </details> #### New Contributors * @​Rocco-Gossmann made their first contribution in yorukot/superfile#736 * @​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-->
Individual PRs
Related issues
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Bug Fixes
These internal improvements contribute to a more reliable and responsive application experience during everyday file operations.