-
-
Notifications
You must be signed in to change notification settings - Fork 414
Compress all files selected #821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
""" WalkthroughThe changes replace the single-source zip compression function with a new one that compresses multiple source paths into a single archive, including progress reporting and error handling. The file operations handler's compression method is refactored to use this new function directly, handling both single and multiple selections. Comprehensive tests for multi-source compression and error cases are added. Minor formatting fixes are applied to the documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FileOps as File Operations
participant ZipUtil as zipSources
Caller->>FileOps: compressFile()
alt Multiple files selected
FileOps->>ZipUtil: zipSources(multiple sources, targetZip)
else Single file/directory selected
FileOps->>ZipUtil: zipSources(single source, targetZip)
end
ZipUtil->>ZipUtil: Verify sources exist
ZipUtil->>ZipUtil: Count total files
ZipUtil->>ZipUtil: Walk each source, add files/dirs to zip archive
ZipUtil-->>FileOps: Return error or success
FileOps-->>Caller: Return error or success
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/internal/file_operations_compress.go (1)
126-187: Consider adding progress tracking for large file operations.Unlike the
zipSourcefunction, this newzipSourcesfunction doesn't track progress or update the user through the channel messaging system. For large file operations, this could lead to a poor user experience where the application seems unresponsive.Consider adding progress tracking similar to what's implemented in the
zipSourcefunction to provide better feedback to users during compression operations.src/internal/handle_file_operations.go (1)
536-569: Suggestion: Add a comment explaining the zip naming convention.For clarity and future maintenance, consider adding a comment explaining why the zip file is named after the first selected file.
Add comments like:
} else { + // For multiple selected files, create a zip named after the first selected file first := panel.selected[0] base := filepath.Base(first)🧰 Tools
🪛 golangci-lint (1.64.8)
552-552: unnecessary trailing newline
(whitespace)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/internal/file_operations_compress.go(1 hunks)src/internal/handle_file_operations.go(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
src/internal/handle_file_operations.go
552-552: unnecessary trailing newline
(whitespace)
🔇 Additional comments (6)
src/internal/file_operations_compress.go (3)
126-134: LGTM! Good file creation and resource management.The function correctly creates a zip file and sets up deferred closing of both the file and the zip writer, ensuring resources are properly released.
140-162: Properly handles relative paths for zip entries.The code correctly calculates relative paths from the parent directory of each source, ensuring files are archived with intuitive directory structures.
173-180: File handling is correct with proper resource management.The implementation correctly opens source files, defers their closing, and copies content to the zip archive with appropriate error handling.
src/internal/handle_file_operations.go (3)
536-551: LGTM! Good implementation of single file compression.The code correctly handles single file compression, creating an appropriate zip name derived from the original file, handling duplicates, and using the new
zipSourcesfunction.
552-569:✅ Verification successful
Great implementation of multi-file compression.
The implementation correctly compresses multiple selected files into a single zip named after the first file in the selection. Error handling is appropriate with early returns.
Run the following script to check for unnecessary trailing newline at line 552 (reported by static analysis):
🏁 Script executed:
#!/bin/bash # Description: Check if there's an unnecessary trailing newline cat -A src/internal/handle_file_operations.go | grep -A 3 -B 3 "else {"Length of output: 1495
No unnecessary trailing newline detected – approval confirmed
The
grep -A3 -B3output shows only a single blank line separating the previous block from the} else {, which is idiomatic for readability. No removal is needed.Great implementation of multi-file compression with proper error handling.
🧰 Tools
🪛 golangci-lint (1.64.8)
552-552: unnecessary trailing newline
(whitespace)
536-569:✅ Verification successful
Verify behavior when folders are selected.
The implementation doesn't explicitly handle the case when a folder is among the selected items. The
zipSourcesfunction should handle this correctly throughfilepath.Walk, but it's worth verifying.Run this script to check if the code handles directory walking correctly:
🏁 Script executed:
#!/bin/bash # Description: Check implementation of directory walking in zipSources # Check if zipSources uses filepath.Walk which is necessary for directory traversal rg -A 5 "filepath.Walk" src/internal/file_operations_compress.go # Verify the loop structure that processes each source rg -A 20 "for _, src := range sources" src/internal/file_operations_compress.goLength of output: 953
zipSources correctly handles directories
I’ve verified thatzipSourcesiterates over each entry insourcesand invokesfilepath.Walk(src, …), so selected folders and their contents are recursively included. No additional handling is needed.🧰 Tools
🪛 golangci-lint (1.64.8)
552-552: unnecessary trailing newline
(whitespace)
|
Please write a unit test for compressing. Refer to a copy test in Set up a test directory, as its done in copy test. Add some files. Then do a compression, compressed file name validation and then extraction to validate that correct files were compressed. |
lazysegtree
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs work
|
@larsn03 would you be actively working on this ? In case this PR could get delayed, I would move it to 1.3.1 milestone. |
|
@lazysegtree Yes I'm gonna work on it |
|
@lazysegtree I'll be sending you a new solution by thursday |
There was a problem hiding this 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 (2)
src/internal/file_operations_compress.go (2)
15-235: Consider removing commented-out code instead of keeping itThe entire original
zipSourcefunction is commented out but still present in the codebase. While it can be useful to keep old implementations for reference during development, it's generally best practice to remove commented-out code before merging to keep the codebase clean.I recommend removing the commented-out code since it's tracked in version control and can be referenced from the commit history if needed in the future.
316-317: Improve error handlingThe error from
io.Copyis returned directly without any logging or context that was present in the original implementation. This might make debugging more difficult.Consider adding context to the error before returning:
- return err + if err != nil { + return fmt.Errorf("failed to copy file %s: %w", path, err) + } + return nil
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/internal/file_operations_compress.go(1 hunks)src/internal/handle_file_operations.go(1 hunks)testsuite/ReadMe.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- testsuite/ReadMe.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/internal/handle_file_operations.go
🧰 Additional context used
🪛 golangci-lint (1.64.8)
src/internal/file_operations_compress.go
237-237: unnecessary leading newline
(whitespace)
331-331: unnecessary trailing newline
(whitespace)
241-241: unnecessary leading newline
(whitespace)
245-245: unnecessary trailing newline
(whitespace)
253-253: unnecessary leading newline
(whitespace)
327-327: unnecessary trailing newline
(whitespace)
257-257: unnecessary leading newline
(whitespace)
319-319: unnecessary trailing newline
(whitespace)
259-259: unnecessary leading newline
(whitespace)
263-263: unnecessary trailing newline
(whitespace)
🔇 Additional comments (3)
src/internal/file_operations_compress.go (3)
237-331: Missing progress tracking in new zipSources implementationThe new
zipSourcesfunction doesn't include the progress tracking and channel messaging that was present in the original implementation. This might affect the user experience if progress information was being displayed in the UI.Was the progress tracking intentionally removed? If progress updates are still needed, consider adapting the progress tracking logic from the original implementation to work with multiple sources.
🧰 Tools
🪛 golangci-lint (1.64.8)
237-237: unnecessary leading newline
(whitespace)
331-331: unnecessary trailing newline
(whitespace)
241-241: unnecessary leading newline
(whitespace)
245-245: unnecessary trailing newline
(whitespace)
253-253: unnecessary leading newline
(whitespace)
327-327: unnecessary trailing newline
(whitespace)
257-257: unnecessary leading newline
(whitespace)
319-319: unnecessary trailing newline
(whitespace)
259-259: unnecessary leading newline
(whitespace)
263-263: unnecessary trailing newline
(whitespace)
333-341: Good implementation of getZipArchiveName functionThe helper function cleanly handles creating a zip archive name from a base filename and ensures it's unique by using the
renameIfDuplicatefunction.
305-314: 🛠️ Refactor suggestionPotential resource leak with deferred file closes
The
defer file.Close()statement is inside thefilepath.Walkcallback, which means files will remain open until the entire walk completes. For large directories with many files, this could lead to too many open files.Consider closing each file immediately after copying its contents instead of using defer:
file, err := os.Open(path) if err != nil { return err } - defer file.Close() _, err = io.Copy(hw, file) + file.Close() return errLikely an incorrect or invalid review comment.
|
@lazysegtree So we've changed everything you mentioned, except the process management so far. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/internal/file_operatio_compress_test.go (1)
275-289: Error case testing for non-existent source.While the test correctly verifies that an error is returned for non-existent sources, it could be enhanced by checking the specific error message or type to ensure the error is occurring for the expected reason.
Consider adding more specific error checking:
err := zipSources([]string{"/non/existent/path"}, targetZip) if err == nil { t.Error("zipSources should return error for non-existent source") +} else if !os.IsNotExist(err) && !strings.Contains(err.Error(), "no such file or directory") { + t.Errorf("Expected 'file not found' error, got: %v", err) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/internal/file_operatio_compress_test.go(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
src/internal/file_operatio_compress_test.go
54-54: shadow: declaration of "err" shadows declaration at line 27
(govet)
🔇 Additional comments (5)
src/internal/file_operatio_compress_test.go (5)
1-16: Well-structured imports and package declaration.The file has a clean organization with properly grouped imports. This aligns with Go best practices.
165-241: Good test coverage for single file compression.This test effectively validates the single file compression scenario, which is an important use case. The validation is thorough, checking both the filename and content in the resulting ZIP.
243-273: Proper edge case testing for empty source list.This test correctly verifies that the function handles an empty list of sources gracefully by creating an empty ZIP file rather than returning an error.
291-315: Error case testing for invalid target path.The test correctly verifies that an error is returned when the target path is invalid. Similar to the previous test, it could be enhanced with more specific error checking.
1-315: Excellent test coverage for the zipSources functionality.The test suite provides comprehensive coverage of the zipSources function, including:
- Complex directory structures with nested files
- Single file compression
- Edge case handling (empty source list)
- Error cases (non-existent source, invalid target)
This thorough testing aligns perfectly with the PR objective of enabling compression of all selected files into a single zip archive. The tests effectively validate that the implementation works correctly for various scenarios.
🧰 Tools
🪛 golangci-lint (1.64.8)
54-54: shadow: declaration of "err" shadows declaration at line 27
(govet)
So, earlier there was process management, and now you have removed it in this PR. I dont know if we should remove an existing feature. |
|
"some things seem broken in the dev-branch" - What is broken ? And what is this dev branch you are referring to ? You are merging to main branch. |
|
Yes but the test "model_file_operations_test.go" you were refering to does only exist in the "develop"-branch, with its dependencies in other files. |
|
Process Management will be added soon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/internal/file_operation_compress_test.go (7)
76-76: Rename unused parameter to underscore.The
tempDirparameter is not used in this setup function. Rename it to_to indicate it's intentionally unused.- setupFunc: func(tempDir string) ([]string, error) { + setupFunc: func(_ string) ([]string, error) {🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 76-76: unused-parameter: parameter 'tempDir' seems to be unused, consider removing or renaming it as _
(revive)
84-84: Rename unused parameter to underscore.The
tempDirparameter is not used in this setup function. Rename it to_to indicate it's intentionally unused.- setupFunc: func(tempDir string) ([]string, error) { + setupFunc: func(_ string) ([]string, error) {🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 84-84: unused-parameter: parameter 'tempDir' seems to be unused, consider removing or renaming it as _
(revive)
108-108: Use require instead of assert for critical assertions.For error checks that subsequent operations depend on, use
require.NoErrorinstead ofassert.NoError. This will immediately fail the test if the error occurs, preventing potential nil pointer dereferences.- assert.NoError(t, err, "zipSources should not return error") + require.NoError(t, err, "zipSources should not return error")Remember to add the import:
import ( "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" )🧰 Tools
🪛 golangci-lint (1.64.8)
108-108: require-error: for error assertions use require
(testifylint)
111-113: Use require instead of assert for opening the ZIP file.When opening the ZIP file, use
require.NoErrorinstead ofassert.NoErrorsince subsequent operations depend on this succeeding.- assert.NoError(t, err, "should be able to open ZIP file") + require.NoError(t, err, "should be able to open ZIP file")🧰 Tools
🪛 golangci-lint (1.64.8)
111-111: require-error: for error assertions use require
(testifylint)
121-122: Use require instead of assert for opening files from the ZIP.When opening files from the ZIP, use
require.NoErrorinstead ofassert.NoErrorsince subsequent operations depend on this succeeding.- assert.NoError(t, err, "should be able to open file %s in ZIP", file.Name) + require.NoError(t, err, "should be able to open file %s in ZIP", file.Name)🧰 Tools
🪛 golangci-lint (1.64.8)
121-121: require-error: for error assertions use require
(testifylint)
124-126: Use require instead of assert for reading file content.When reading file content, use
require.NoErrorinstead ofassert.NoErrorsince subsequent operations depend on this succeeding.- assert.NoError(t, err, "should be able to read file %s", file.Name) + require.NoError(t, err, "should be able to read file %s", file.Name)🧰 Tools
🪛 golangci-lint (1.64.8)
125-125: require-error: for error assertions use require
(testifylint)
150-152: Use require instead of assert for test file creation.When creating a test file, use
require.NoErrorinstead ofassert.NoErrorsince the test depends on this file existing.- assert.NoError(t, err, "should be able to create test file") + require.NoError(t, err, "should be able to create test file")🧰 Tools
🪛 golangci-lint (1.64.8)
151-151: require-error: for error assertions use require
(testifylint)
src/internal/file_operations_compress.go (1)
69-74: Fix inconsistent formatting.There's inconsistent spacing in the
getZipArchiveNamefunction with an extra newline before the closing brace.func getZipArchiveName(base string) (string, error) { zipName := strings.TrimSuffix(base, filepath.Ext(base)) + ".zip" zipName, err := renameIfDuplicate(zipName) return zipName, err - }🧰 Tools
🪛 golangci-lint (1.64.8)
74-74: unnecessary trailing newline
(whitespace)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/internal/file_operation_compress_test.go(1 hunks)src/internal/file_operations_compress.go(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
src/internal/file_operation_compress_test.go
[warning] 76-76: unused-parameter: parameter 'tempDir' seems to be unused, consider removing or renaming it as _
(revive)
[warning] 84-84: unused-parameter: parameter 'tempDir' seems to be unused, consider removing or renaming it as _
(revive)
151-151: require-error: for error assertions use require
(testifylint)
108-108: require-error: for error assertions use require
(testifylint)
111-111: require-error: for error assertions use require
(testifylint)
121-121: require-error: for error assertions use require
(testifylint)
125-125: require-error: for error assertions use require
(testifylint)
🔇 Additional comments (4)
src/internal/file_operation_compress_test.go (2)
14-145: Tests look good overall.The test cases provide comprehensive coverage for the
zipSourcesfunction, including:
- Compressing multiple directories with nested subdirectories
- Compressing a single file
- Handling empty source lists
- Error handling for non-existent sources
The verification logic thoroughly checks both file structure and content in the resulting ZIP archive.
🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 76-76: unused-parameter: parameter 'tempDir' seems to be unused, consider removing or renaming it as _
(revive)
[warning] 84-84: unused-parameter: parameter 'tempDir' seems to be unused, consider removing or renaming it as _
(revive)
108-108: require-error: for error assertions use require
(testifylint)
111-111: require-error: for error assertions use require
(testifylint)
121-121: require-error: for error assertions use require
(testifylint)
125-125: require-error: for error assertions use require
(testifylint)
147-156: Good error case testing.This test properly verifies that the
zipSourcesfunction returns an error when trying to create a ZIP file in an invalid location. This is an important edge case to test.🧰 Tools
🪛 golangci-lint (1.64.8)
151-151: require-error: for error assertions use require
(testifylint)
src/internal/file_operations_compress.go (2)
21-61: Consider improving path handling for multiple sources.When compressing multiple sources from different locations, using the parent directory of each source (
srcParentDir) for relative path calculation might lead to unexpected path structures in the ZIP. This approach works but might not always produce the most intuitive archive structure.Would it be better to maintain the full path structure or use a common ancestor directory for all sources?
11-67:❓ Verification inconclusive
Function implementation looks good, but consider process management.
The implementation of
zipSourcescorrectly handles multiple sources and maintains proper file structure in the ZIP archive. However, the previous review comments mention that process management was present in the original function but is missing here.If progress tracking is a required feature, consider adding it back or documenting why it was removed.
🏁 Script executed:
#!/bin/bash # Check if there's any reference to process management in the codebase rg -l "process.*management|progress.*track" --type goLength of output: 126
Let’s inspect how “process management” or “progress tracking” is implemented in the original files to determine what was removed.
🏁 Script executed:
#!/bin/bash echo "=== handle_file_operations.go context ===" rg -nE "progress|process" -C3 src/internal/handle_file_operations.go echo "" echo "=== file_operations.go context ===" rg -nE "progress|process" -C3 src/internal/file_operations.goLength of output: 425
Let’s retry the search with correct ripgrep flags to inspect any progress- or process-management code in those files:
🏁 Script executed:
#!/bin/bash echo "=== Searching for progress tracking ===" rg -n "progress" -C3 src/internal/handle_file_operations.go src/internal/file_operations.go echo "" echo "=== Searching for process management ===" rg -n "process management|ProcessManager" -C3 src/internal/handle_file_operations.go src/internal/file_operations.goLength of output: 5105
Let's locate where
zipSourcesis invoked to see if it’s integrated with the existing progress/process management:
🏁 Script executed:
#!/bin/bash echo "=== References to zipSources ===" rg -n "zipSources" -C3 .Length of output: 3276
Review zipSources integration and formatting
The
zipSourcesimplementation is correct (thedefer file.Close()inside the Walk closure closes each file on return), but it’s currently a “black‐box” operation with no progress reporting. If user feedback or cancellation is required, wire in your existing progress machinery or document why it’s omitted.• src/internal/file_operations_compress.go (zipSources):
– Consider accepting a progress callback orprocess/progress.Modelparameter.
– Alternatively, expose hooks so callers can track bytes/files processed.• src/internal/handle_file_operations.go (lines 629 & 646):
– These call sites invokezipSourcesbut don’t update the UI’s progress bar. Pass through or initialize aprogress.Modelhere.• Formatting:
– Rungofmt/goimportsonfile_operations_compress.goto eliminate any stray blank lines or spacing inconsistencies.Let me know if you’d rather document the synchronous behavior instead of adding progress tracking.
| defer file.Close() | ||
| _, err = io.Copy(hw, file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Resource leak risk with defer inside loop.
Using defer file.Close() inside a loop can lead to resource leaks if processing many files, as all file handles will remain open until the function returns.
- defer file.Close()
- _, err = io.Copy(hw, file)
- return err
+ _, err = io.Copy(hw, file)
+ closeErr := file.Close()
+ if err == nil {
+ err = closeErr
+ }
+ return err🤖 Prompt for AI Agents
In src/internal/file_operations_compress.go around lines 58 to 59, the use of
defer file.Close() inside a loop risks leaking file descriptors because deferred
calls accumulate until the function exits. Replace defer with an explicit
file.Close() call immediately after io.Copy completes for each file to ensure
timely release of resources within the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo - Add a todo comment to prevent that and to make sure fix this in the codebase everywhere -
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
I'll help you add a TODO comment and search for similar patterns in the codebase to ensure we fix this everywhere.
Task: Add TODO comment and fix defer file.Close() in loop pattern
- Add TODO comment in the current file about the resource leak
- Search for similar patterns in the codebase
- Fix the specific issue in zipSources function
- Fix any other instances found
@lazysegtree The TODO comment will help track this resource leak issue and ensure we fix it everywhere in the codebase. I'm also searching for similar patterns to address them comprehensively.
🏁 Script executed:
#!/bin/bash
# Search for defer file.Close() patterns that might be inside loops
echo "=== Searching for defer file.Close() patterns ==="
rg "defer.*\.Close\(\)" -A 2 -B 2
echo -e "\n=== Searching for file.Close() patterns in general ==="
rg "file\.Close\(\)" -A 1 -B 1
echo -e "\n=== Looking for filepath.Walk patterns that might have similar issues ==="
rg "filepath\.Walk" -A 10 -B 2
Length of output: 7479
🏁 Script executed:
# Search more specifically for defer statements inside loops or Walk functions
echo "=== Searching for defer patterns inside filepath.Walk ==="
ast-grep --pattern $'filepath.Walk($_, func($_, $_, $_) error {
$$$
defer $_.Close()
$$$
})'
echo -e "\n=== Searching for defer patterns inside for loops ==="
ast-grep --pattern $'for $_ {
$$$
defer $_.Close()
$$$
}'
echo -e "\n=== Searching for defer patterns inside range loops ==="
ast-grep --pattern $'for $_ := range $_ {
$$$
defer $_.Close()
$$$
}'
Length of output: 6757
Perfect! I found two instances of this defer file.Close() pattern in loops that need to be fixed. Let me update the plan to address both issues:
Task: Fix defer file.Close() in loops pattern across codebase
- Add TODO comments in both affected files about the resource leak risk
- Fix the defer pattern in zipSources function (filepath.Walk callback)
- Fix the defer pattern in load_config.go (for range loop)
- Ensure proper error handling for all Close() operations
@lazysegtree I found two instances of this problematic pattern in the codebase:
src/internal/file_operations_compress.go(line 100) -defer file.Close()insidefilepath.Walkcallbacksrc/internal/common/load_config.go(line 226) -defer curThemeFile.Close()insidefor rangeloop
Both will accumulate file descriptors until the function exits, potentially causing resource exhaustion when processing many files. The code changes above will add TODO comments first, then fix both instances with proper explicit file closing and error handling.
🧬 Code Graph Analysis Results
Relevant snippets for understanding src/internal/file_operations_compress.go:
From src/internal/common/style.go (line 19):
FooterStyle lipgloss.Style- Defines a style used for progress percentage display in the footer, referenced as
common.FooterStyleinzipSources.
From src/config/icon/icon.go:
- Line 10:
Space = " "-
Defines a space character string used for formatting messages.
-
Line 24:
CompressFile = "\U000f05c4"-
Unicode icon string representing compression, used as a prefix in progress messages during zipping.
-
Line 34:
Error = "\uf530"-
Unicode icon string representing an error, potentially used elsewhere for error display.
-
Line 26:
Copy = "\U000f018f"- Unicode icon string representing copy operation, not directly used in the provided file but part of icon definitions.
From src/internal/ui/sidebar/sidebar.go (lines 110-122):
func New() Model {
res := Model{
renderIndex: 0,
directories: getDirectories(),
searchBar: common.GenerateSearchBar(),
}
// Excluding borders(2), Searchbar Prompt(2), and one extra character than is appended
// by searchBar.View()
res.searchBar.Width = common.Config.SidebarWidth - 2 - 2 - 1
res.searchBar.Placeholder = "(" + common.Hotkeys.SearchBar[0] + ")" + " Search"
return res
}- Shows initialization of a UI sidebar model with a search bar, including width calculation and placeholder text.
- Not directly related to compression but may provide context on UI components and usage of
commonpackage.
These snippets provide context on styling, icon usage, and some UI initialization that relate to the progress reporting and messaging in the zipSources function in file_operations_compress.go.
✏️ Learnings added
Learnt from: lazysegtree
PR: yorukot/superfile#821
File: src/internal/file_operations_compress.go:0-0
Timestamp: 2025-06-05T12:42:45.110Z
Learning: Pattern to watch for: defer file.Close() inside loops (for, range, filepath.Walk callbacks) causes resource leaks by accumulating file descriptors until function exits. Should be replaced with explicit Close() calls immediately after file operations with proper error handling.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⏳ Preparing changes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A summary of the edits CodeRabbit can apply:
Will add a TODO comment in zipSources function in src/internal/file_operations_compress.go to flag the defer file.Close() resource leak and outline follow‑up search and fix tasks
Add a TODO comment above the
defer file.Close()inzipSourcesinsrc/internal/file_operations_compress.goto track the resource leak issue.
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):
✏️ src/internal/file_operations_compress.go
Insert immediately above the existing defer file.Close() call in zipSources the following lines:
// TODO: Fix resource leak - defer file.Close() inside filepath.Walk callback causes file descriptors
// to accumulate until the function returns. Replace with explicit file.Close() after io.Copy.
// Ensure this pattern is fixed everywhere in the codebase to prevent resource exhaustion.Run a repository‑wide search to locate other occurrences of defer‑Close calls inside loops:
rg "defer .*\.Close\(\)" -A2 -B2For each match that appears within a for‑loop or filepath.Walk callback, add the same TODO comment above the defer Close() call to flag it for later refactoring.
Create follow‑up tasks (e.g., GitHub issues or TODO tickets) to replace each flagged defer Close() with an explicit Close() immediately after the io operation, capturing and handling any Close() errors.
Open src/internal/file_operations_compress.go and navigate to line 58 (the line containing defer file.Close() inside the filepath.Walk callback of zipSources).
Replace the single defer file.Close() line with the following block to add a TODO comment and preserve the defer for now:
// TODO: Fix resource leak – defer file.Close() inside filepath.Walk callback
// causes file descriptors to accumulate until function exits.
// Replace with an explicit file.Close() immediately after io.Copy.
// This pattern should be fixed everywhere in the codebase to prevent resource exhaustion.
defer file.Close()Save your changes, run existing tests or perform a quick local build to ensure there are no syntax errors, then commit with a message such as “chore: add TODO for defer file.Close() leak in zipSources”.
A summary of the context that CodeRabbit is considering across the codebase:
Locate the exact line number of the problematic
defer file.Close()inside thezipSourcesfilepath.Walk callback so we know where to insert the TODO comment.
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create an issue to track this fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A summary of the edits CodeRabbit can apply:
An issue file will be created to track the resource leak from defer file.Close() inside loops
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):
📄 issues/0001-track-resource-leak-defer-file-close-in-loops.md
Create a new file at issues/0001-track-resource-leak-defer-file-close-in-loops.md.
Populate the file with the following content:
# Track resource leak from defer file.Close() in loops
**Status**: Open
**Priority**: High
## Description
The use of `defer file.Close()` inside loops and callback functions (such as `filepath.Walk` and `for range` loops) causes file descriptors to remain open until the parent function returns, leading to resource exhaustion when processing many files.
## Affected Files
- src/internal/file_operations_compress.go
- src/internal/common/load_config.go
## Proposed Solution
Replace each `defer file.Close()` inside loops or callbacks with an explicit `file.Close()` call immediately after the corresponding `io.Copy` or file operations. Capture and handle any errors returned by `Close()`.
## Steps to Reproduce
1. Instrument file descriptor usage.
2. Run compression or config loading on a large set of files.
3. Observe file descriptors not being released until program exit.
## Acceptance Criteria
- All instances of `defer file.Close()` inside loops are replaced with explicit close calls.
- Proper error handling is added for each `Close()`.
- Code compiles and existing functionality is unaffected.Commit the new file with the message "docs: add issue to track resource leak from defer file.Close() in loops".
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/internal/file_operations_compress.go (1)
102-103: Resource leak risk with defer inside loop.Using
defer file.Close()inside thefilepath.Walkcallback can cause resource leaks when processing many files, as all file handles remain open until the function returns.This issue was previously identified. The
defer file.Close()should be replaced with explicit close handling:- defer file.Close() - _, err = io.Copy(headerWriter, file) - if err != nil { - return err - } + _, err = io.Copy(headerWriter, file) + closeErr := file.Close() + if err != nil { + return err + } + if closeErr != nil { + return closeErr + }
🧹 Nitpick comments (2)
src/internal/file_operations_compress.go (2)
22-22: Remove unnecessary error initialization.The explicit
nilinitialization is redundant sinceerrortypes have a zero value ofnil.- var err error = nil + var err error🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 22-22: var-declaration: should drop = nil from declaration of var err; it is the zero value
(revive)
130-135: Remove unnecessary trailing newline.The function has an extra blank line that should be removed for consistency.
func getZipArchiveName(base string) (string, error) { zipName := strings.TrimSuffix(base, filepath.Ext(base)) + ".zip" zipName, err := renameIfDuplicate(zipName) return zipName, err - }🧰 Tools
🪛 golangci-lint (1.64.8)
135-135: unnecessary trailing newline
(whitespace)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/internal/file_operations_compress.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/internal/file_operations_compress.go (2)
src/internal/common/style.go (1)
FooterStyle(19-19)src/config/icon/icon.go (2)
CompressFile(24-24)Space(10-10)
🪛 golangci-lint (1.64.8)
src/internal/file_operations_compress.go
[warning] 22-22: var-declaration: should drop = nil from declaration of var err; it is the zero value
(revive)
25-25: shadow: declaration of "err" shadows declaration at line 22
(govet)
32-32: shadow: declaration of "err" shadows declaration at line 22
(govet)
135-135: unnecessary trailing newline
(whitespace)
🔇 Additional comments (2)
src/internal/file_operations_compress.go (2)
67-122: Verify process management implementation.Based on past review comments, there were concerns about missing process management. The current implementation appears to include progress tracking and channel communication, but please confirm this addresses the previous concerns about process management functionality.
Could you verify that this implementation includes all the process management features that were present in the original
zipSourcefunction?
18-128: Overall implementation looks solid for multi-source compression.The refactoring from single-source to multi-source compression is well-implemented. The function properly:
- Validates all source paths before processing
- Counts total files for accurate progress tracking
- Maintains relative path structure in the archive
- Provides progress updates through the channel system
- Handles errors appropriately
The core logic correctly iterates through multiple sources and creates a unified archive structure.
🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 22-22: var-declaration: should drop = nil from declaration of var err; it is the zero value
(revive)
25-25: shadow: declaration of "err" shadows declaration at line 22
(govet)
32-32: shadow: declaration of "err" shadows declaration at line 22
(govet)
|
@lazysegtree Thank you for your help :) |
There was a problem hiding this 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)
testsuite/ReadMe.md (3)
65-65: Use markdown header for Notes
The "Notes" line should include a markdown header marker (e.g., "### Notes") to match the existing section styles.
74-74: Typo and punctuation correction
Fix spelling of "enforing" → "enforcing" and adjust to "Note:" (no space before colon).🧰 Tools
🪛 LanguageTool
[typographical] ~74-~74: Consider adding a comma here.
Context: ...ait time for superfile to close. Note : For now we have enforing superfile to close wit...(FOR_NOW_COMMA)
78-78: Hyphenate 'up-to-date'
Add hyphens in "up-to-date" for grammatical correctness.🧰 Tools
🪛 LanguageTool
[uncategorized] ~78-~78: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ...ile']` Make sure your python version is up to date(UP_TO_DATE_HYPHEN)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/internal/file_operations_compress.go(3 hunks)src/internal/handle_file_operations.go(1 hunks)testsuite/ReadMe.md(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/internal/file_operations_compress.go
- src/internal/handle_file_operations.go
🧰 Additional context used
🪛 LanguageTool
testsuite/ReadMe.md
[typographical] ~74-~74: Consider adding a comma here.
Context: ...ait time for superfile to close. Note : For now we have enforing superfile to close wit...
(FOR_NOW_COMMA)
[uncategorized] ~77-~77: Possible missing preposition found.
Context: ...--teststo only run specific tests - Examplepython main.py -d -t RenameTest CopyTe...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~78-~78: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ...ile']` Make sure your python version is up to date
(UP_TO_DATE_HYPHEN)
🔇 Additional comments (3)
testsuite/ReadMe.md (3)
2-2: Trailing spaces removal approved
The trailing spaces on this list item are removed, improving consistency. LGTM.
12-12: Nested list formatting intact
The nested list indentation is consistent and clear.
15-15: Header trailing spaces removed
Removing trailing spaces from the "Setup" header is correct and cleans up the markdown.
…t old zipSource function
831789c to
5b4dc0e
Compare
|
Rebased the branch with latest main. |
|
Self Note - Manually validated that multi file compression works. I will clean up file_operation_compress_test.go and add some tests for in handle_file_operations.go. |
There was a problem hiding this 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 (2)
testsuite/ReadMe.md (2)
74-74: Refine punctuation and correct typo.
Replace "Note :" with "Note:" and correct "enforing" to "enforcing".
Apply this diff:- Note : For now we have enforing superfile to close within a specific time window in tests to reduce test flakiness + Note: For now we have enforcing superfile to close within a specific time window in tests to reduce test flakiness🧰 Tools
🪛 LanguageTool
[typographical] ~74-~74: Consider adding a comma here.
Context: ...ait time for superfile to close. Note : For now we have enforing superfile to close wit...(FOR_NOW_COMMA)
78-78: Hyphenate "up-to-date".
Add hyphens to the adjective “up-to-date” for proper grammar.
Apply this diff:- Make sure your python version is up to date + Make sure your python version is up-to-date🧰 Tools
🪛 LanguageTool
[uncategorized] ~78-~78: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ...ile']` Make sure your python version is up to date(UP_TO_DATE_HYPHEN)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/internal/file_operation_compress_test.go(1 hunks)src/internal/file_operations_compress.go(3 hunks)src/internal/handle_file_operations.go(1 hunks)src/internal/key_function.go(1 hunks)testsuite/ReadMe.md(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/internal/key_function.go
- src/internal/handle_file_operations.go
- src/internal/file_operation_compress_test.go
- src/internal/file_operations_compress.go
🧰 Additional context used
🪛 LanguageTool
testsuite/ReadMe.md
[typographical] ~74-~74: Consider adding a comma here.
Context: ...ait time for superfile to close. Note : For now we have enforing superfile to close wit...
(FOR_NOW_COMMA)
[uncategorized] ~78-~78: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ...ile']` Make sure your python version is up to date
(UP_TO_DATE_HYPHEN)
🔇 Additional comments (4)
testsuite/ReadMe.md (4)
2-2: Approve whitespace cleanup.
Trailing spaces removed from this line, improving consistency.
12-12: Approve whitespace cleanup.
Removed trailing spaces for improved formatting.
15-15: Approve whitespace cleanup.
Whitespace removal aligns with the style guidelines.
65-65: Approve whitespace cleanup.
Cleared trailing spaces, making the "Notes" heading clean.
|
Alright |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
testsuite/ReadMe.md (1)
74-74: Refine punctuation and phrasing
Adjust the colon spacing and add a comma for clarity in the note.- Note : For now we have enforcing superfile to close within a specific time window in tests to reduce test flakiness + Note: For now, we are enforcing superfile to close within a specific time window in tests to reduce test flakiness🧰 Tools
🪛 LanguageTool
[typographical] ~74-~74: Consider adding a comma here.
Context: ...ait time for superfile to close. Note : For now we have enforcing superfile to close wi...(FOR_NOW_COMMA)
src/internal/handle_file_operation_test.go (4)
36-36: Address the TODO comment.The TODO comment indicates incomplete work from copying CopyTest. Consider completing this work or removing the TODO if it's no longer relevant.
Would you like me to help identify what specific work needs to be done here, or should this TODO be removed?
134-137: Consider more robust timing for async operations.The test uses
assert.Eventuallywith 1 second timeout and 10ms intervals. Consider if these values are appropriate for all environments, especially slower CI systems.Consider making the timeout configurable or slightly longer for CI environments:
- }, time.Second, 10*time.Millisecond) + }, 3*time.Second, 50*time.Millisecond)
140-142: Address the TODO comment about panel refresh.The TODO comment indicates a design issue where file panel refresh should happen automatically after operation completion. This suggests a potential architectural improvement.
This TODO highlights an important UX issue. Would you like me to help investigate how to implement automatic panel refresh after operations complete?
154-163: Improve error reporting in extraction validation.The eventual assertion for extraction validation could provide better error messages when files are missing.
Consider improving the error reporting:
assert.Eventually(t, func() bool { for _, f := range tt.expectedFilesAfterExtract { - _, err := os.Stat(filepath.Join(extractedDir, f)) - if err != nil { + fullPath := filepath.Join(extractedDir, f) + _, err := os.Stat(fullPath) + if err != nil { + t.Logf("Missing expected file: %s (error: %v)", fullPath, err) return false } } return true }, time.Second, 10*time.Millisecond, "Extraction of files failed Required - [%s]+%v", extractedDir, tt.expectedFilesAfterExtract)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/internal/handle_file_operation_test.go(1 hunks)testsuite/ReadMe.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
testsuite/ReadMe.md
[typographical] ~74-~74: Consider adding a comma here.
Context: ...ait time for superfile to close. Note : For now we have enforcing superfile to close wi...
(FOR_NOW_COMMA)
[uncategorized] ~77-~77: Possible missing preposition found.
Context: ...--teststo only run specific tests - Examplepython main.py -d -t RenameTest CopyTe...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~78-~78: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ...ile']` Make sure your python version is up to date
(UP_TO_DATE_HYPHEN)
🔇 Additional comments (8)
testsuite/ReadMe.md (4)
2-2: Formatting cleanup looks good
Trailing spaces were removed as intended without altering content.
12-12: Formatting cleanup looks good
Removed extraneous whitespace on this line.
15-15: Formatting cleanup looks good
Trailing spaces removed from the section header.
65-65: Formatting cleanup looks good
Whitespace trimmed from the “Notes” line.src/internal/handle_file_operation_test.go (4)
1-13: LGTM! Well-structured imports and package declaration.The imports are appropriate for a comprehensive test file, including standard library packages, testify for assertions, and internal project packages.
22-23:✅ Verification successful
Verify helper function implementations.
The test calls
setupDirectoriesandsetupFileshelper functions that are not defined in this file. Ensure these functions exist and work correctly.
🏁 Script executed:
#!/bin/bash # Search for the helper function implementations in the codebase ast-grep --pattern 'func setupDirectories($$$) { $$$ }' ast-grep --pattern 'func setupFiles($$$) { $$$ }'Length of output: 671
Helper functions verified in test_utils.go – no action needed
I found both
setupDirectoriesandsetupFilesimplemented insrc/internal/test_utils.go:
- Lines 21–27 define
setupDirectories(t *testing.T, dirs ...string)- Lines 37–39 define
setupFiles(t *testing.T, files ...string)Both include the necessary helpers (
t.Helper(), error checks, etc.), so no further changes are required here.
131-131:✅ Verification successful
Verify TeaUpdateWithErrCheck helper function.
The test relies on
TeaUpdateWithErrCheckwhich should be verified for existence and correctness.
🏁 Script executed:
#!/bin/bash # Search for TeaUpdateWithErrCheck function implementation ast-grep --pattern 'func TeaUpdateWithErrCheck($$$) { $$$ }'Length of output: 64
#!/bin/bash
Search for the TeaUpdateWithErrCheck helper across the repository
rg -n "TeaUpdateWithErrCheck"
Broader search for ErrCheck helpers to locate variations or definitions
rg -n "ErrCheck"
🏁 Script executed:
#!/bin/bash # Search for any references or definitions of TeaUpdateWithErrCheck in Go files rg -n "TeaUpdateWithErrCheck" -g "*.go" # Broader search for ErrCheck helper patterns in Go code rg -n "ErrCheck" -g "*.go"Length of output: 14394
#!/bin/bash
Show the implementation of TeaUpdateWithErrCheck to verify its behavior
rg -n -A10 "func TeaUpdateWithErrCheck" src/internal/test_utils.go | sed -e 's/^([0-9]+:)//'
#!/bin/bash
Display the implementation of TeaUpdateWithErrCheck for verification
rg -n -A20 "func TeaUpdateWithErrCheck" src/internal/test_utils.go | sed -E 's/^[0-9]+://'
🏁 Script executed:
#!/bin/bash # Display implementation of TeaUpdateWithErrCheck for verification rg -n -A20 "func TeaUpdateWithErrCheck" src/internal/test_utils.goLength of output: 585
TeaUpdateWithErrCheck is present and correct
Confirmed the helper is implemented in
src/internal/test_utils.goas:func TeaUpdateWithErrCheck(t *testing.T, m *model, msg tea.Msg) tea.Cmd { cmd, err := TeaUpdate(m, msg) require.NoError(t, err) return cmd }It properly calls
TeaUpdate, asserts no error, and returns the command. No further action needed.
120-129:✅ Verification successful
Verify helper function implementations for test model setup.
The test calls
defaultTestModelandchangeFilePanelModemethods that need verification.
🏁 Script executed:
#!/bin/bash # Search for the defaultTestModel function and related methods ast-grep --pattern 'func defaultTestModel($$$) { $$$ }' # Search for changeFilePanelMode method ast-grep --pattern 'func ($_) changeFilePanelMode($$$) { $$$ }' # Search for getFocusedFilePanel method ast-grep --pattern 'func ($_) getFocusedFilePanel($$$) { $$$ }'Length of output: 847
🏁 Script executed:
#!/bin/bash # Locate the defaultTestModel helper definition rg -n "func .*defaultTestModel" . # Locate the getFocusedFilePanel method implementation rg -n "func .*getFocusedFilePanel" .Length of output: 308
🏁 Script executed:
#!/bin/bash # Display defaultTestModel implementation to verify initial file panel setup sed -n '1,200p' src/internal/test_utils.go # Display getFocusedFilePanel implementation for context sed -n '410,470p' src/internal/function.go # Display changeFilePanelMode implementation for reference sed -n '1,200p' src/internal/handle_panel_movement.goLength of output: 9481
🏁 Script executed:
#!/bin/bash # Locate and display defaultModelConfig definition file=$(rg -l "func defaultModelConfig" .) echo "Found defaultModelConfig in $file" sed -n '1,200p' "$file"Length of output: 5671
Helper function implementations verified
All three helpers used in the test are present and behave as expected:
defaultTestModel(in src/internal/test_utils.go) callsdefaultModelConfigand sends aWindowSizeMsgviaTeaUpdate, correctly initializing and populating the file panel.getFocusedFilePanel(in src/internal/function.go) returns a pointer to the activefilePanel.changeFilePanelMode(in src/internal/handle_panel_movement.go) togglespanelModebetweenbrowserMode(zero value) andselectMode, clearingselectedwhen exiting select mode.The test’s assumptions about
elementlength, initialpanelMode, and selection behavior are all valid—no changes required.
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [yorukot/superfile](https://github.com/yorukot/superfile) | patch | `v1.3.1` -> `v1.3.3` | 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.3`](https://github.com/yorukot/superfile/releases/tag/v1.3.3) [Compare Source](yorukot/superfile@v1.3.2...v1.3.3) Hey folks. Releasing v1.3.3 with bug fix related to config file, and improvements in metadata panel feature. #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - Fixed a bug in config file correction that would make config file invalid. Thanks [@​estebandpena](https://github.com/estebandpena) for reporting the issue, and [@​wassup05](https://github.com/wassup05) for the fix. #### Detailed Change Summary <details><summary>Details</summary> <p> ##### Update - feat: Metadata loading via bubbletea's tea.Cmd method, removed usage channels and custom goroutines [#​947](yorukot/superfile#947) by @​lazysegtree - feat: Metadata panel into separate package, UI bug fixes, Code improvements [#​950](yorukot/superfile#950) by @​lazysegtree - feat: add support for Python virtual environment in testsuite setup [#​956](yorukot/superfile#956) by @​Khitoshi ##### Bug fix - fix: windows test ci [#​941](yorukot/superfile#941) by @​claykom - fix: fixing `config.toml` [#​952](yorukot/superfile#952) by @​wassup05 </p> </details> **Full Changelog**: yorukot/superfile@v1.3.2...v1.3.3 ### [`v1.3.2`](https://github.com/yorukot/superfile/releases/tag/v1.3.2) [Compare Source](yorukot/superfile@v1.3.1...v1.3.2) Hey folks. Releasing v1.3.2 with multiple new features including implementation image preview using kitty protocol, and various bug fixes and internal improvements. ##### **You can now preview images in high quality — no more pixelation!** <img width="1267" height="670" alt="image" src="https://github.com/user-attachments/assets/0f7ec48e-9386-4716-92a5-4a02783f6612" /> #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - Added image preview via kitty protocol. Thanks @​JassonCordones and @​yorukot for the implementation. - Added Zoxide support for path resolution in initial configuration. Thanks @​yorukot for this. - New 0x96f Theme by @​filipjanevski - Various usability and layout bug fixes. See "Detailed Change Summary" below. Thanks to @​larsn03 @​Frost-Phoenix @​TheLazron @​vkhatsiur @​Khitoshi @​claykom ##### Developer Improvements - Added CI Action to run unit tests and build for windows. - Updated contribution guidelines and MR template - Added a dev.sh script for building, linting and running tests for faster development workflows. #### Detailed Change Summary <details><summary>Details</summary> <p> ##### Update - Normalize user-facing naming to superfile [#​880](yorukot/superfile#880) by @​yorukot - Add kitty protocol for image preview [#​841](yorukot/superfile#841) by @​JassonCordones - feat: add Zoxide support for path resolution in initial configuration [#​892](yorukot/superfile#892) by @​yorukot - feat: update superfile's help output [#​908](yorukot/superfile#908) by @​yorukot - feat: Add Action to Publish to Winget [#​925](yorukot/superfile#925) by @​claykom - feat: update superfile build test for the windows and macOS [#​922](yorukot/superfile#922) by @​yorukot - Theme: add 0x96f theme [#​860](yorukot/superfile#860) by @​filipjanevski ##### Bug fix - fix: outdated and broken nix flake [#​846](yorukot/superfile#846) by @​Frost-Phoenix - fix: handle UTF-8 BOM in file reader [#​865](yorukot/superfile#865) by @​vkhatsiur - fix icon displayed on spf prompt when nerdfont disabled [#​878](yorukot/superfile#878) by @​claykom - fix: create item check for dot-entries [#​817](yorukot/superfile#817) by @​TheLazron - fix: prevent pasting a directory into itself, avoiding infinite loop [#​887](yorukot/superfile#887) by @​yorukot - fix: clear search bar value on parent directory reset [#​906](yorukot/superfile#906) by @​yorukot - fix: enhance terminal pixel detection and response handling [#​904](yorukot/superfile#904) by @​yorukot - fix: Cannot Build superfile on Windows [#​921](yorukot/superfile#921) by @​claykom - fix: Improve command tokenization to handle quotes and escapes [#​931](yorukot/superfile#931) by @​Khitoshi - fix: Dont read special files, and prevent freeze [#​932](yorukot/superfile#932) by @​lazysegtree - Compress all files selected [#​821](yorukot/superfile#821) by @​larsn03 ##### Optimization - Metadata and filepanel rendering refactor [#​867](yorukot/superfile#867) by @​JassonCordones - refactor: simplify panel mode handling in file movement logic [#​907](yorukot/superfile#907) by @​yorukot - refactor: standardize TODO comments and ReadMe to README [#​913](yorukot/superfile#913) by @​yorukot ##### Documentation - enhance: add detailed documentation for InitIcon function and update … [#​879](yorukot/superfile#879) by @​yorukot - docs: add documentation for image preview [#​882](yorukot/superfile#882) by @​yorukot - docs: update contributing guide and MR template [#​885](yorukot/superfile#885) by @​yorukot - docs: update README and plugin documentation for clarity and structure [#​902](yorukot/superfile#902) by @​yorukot - feat(docs): Update arch install package docs [#​929](yorukot/superfile#929) by @​booth-w ##### CI/CD - ci: add MR title linting with semantic-pull-request action [#​884](yorukot/superfile#884) by @​yorukot - ci: improve MR workflows with contributor greeting and title linter fix [#​886](yorukot/superfile#886) by @​yorukot ##### Dependencies - build(deps): bump prismjs from 1.29.0 to 1.30.0 in /website [#​786](yorukot/superfile#786) by @​dependabot[bot] - fix(deps): update dependency astro to v5.8.0 [#​787](yorukot/superfile#787) by @​renovate[bot] - chore(deps): bump vite from 6.3.3 to 6.3.5 in /website [#​822](yorukot/superfile#822) by @​dependabot[bot] - fix(deps): update dependency sharp to v0.34.2 [#​909](yorukot/superfile#909) by @​renovate[bot] - fix(deps): update astro monorepo [#​894](yorukot/superfile#894) by @​renovate[bot] - fix(deps): update fontsource monorepo to v5.2.6 [#​910](yorukot/superfile#910) by @​renovate[bot] ##### Misc - chore(license): update copyright year [#​895](yorukot/superfile#895) by @​yorukot - feat: add ignore missing field flag [#​881](yorukot/superfile#881) by @​claykom - feat: add sitemap integration and update giscus input position [#​912](yorukot/superfile#912) by @​yorukot </p> </details> #### New Contributors * @​filipjanevski made their first contribution in yorukot/superfile#860 * @​larsn03 made their first contribution in yorukot/superfile#821 * @​vkhatsiur made their first contribution in yorukot/superfile#865 * @​claykom made their first contribution in yorukot/superfile#878 * @​TheLazron made their first contribution in yorukot/superfile#817 * @​Khitoshi made their first contribution in yorukot/superfile#931 **Full Changelog**: yorukot/superfile@v1.3.1...v1.3.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **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:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->

We worked out a fix to compress all selected files (if there are any) into a single zip, which is named after the first file.
Summary by CodeRabbit