Skip to content

Comments

Standardize extraction API to WriteToDirectory with IProgress support#1080

Merged
adamhathcock merged 9 commits intomasterfrom
copilot/refactor-write-to-directory-consistency
Dec 19, 2025
Merged

Standardize extraction API to WriteToDirectory with IProgress support#1080
adamhathcock merged 9 commits intomasterfrom
copilot/refactor-write-to-directory-consistency

Conversation

Copy link
Contributor

Copilot AI commented Dec 19, 2025

Standardize on WriteToDirectory Naming

This PR resolves the naming inconsistency between WriteToDirectory and ExtractToDirectory methods as requested in the issue.

Changes Made

1. ✅ Removed ExtractToDirectory method

  • Deleted from IArchiveExtensions.cs to standardize on WriteToDirectory naming

2. ✅ Enhanced WriteToDirectory overloads

  • Basic overload: Now intelligently handles both solid and non-solid archives:
    • Solid archives (Rar, 7Zip): Uses ExtractAllEntries() for optimal performance
    • Non-solid archives: Directly iterates entries
  • Progress overload: Uses IProgress<ProgressReport> for consistent progress reporting (without CancellationToken for sync methods)

3. ✅ Added WriteToDirectoryAsync

  • New async method for IArchive with full async/await support
  • Supports ExtractionOptions, IProgress<ProgressReport> progress reporting, and cancellation tokens
  • Follows existing async patterns in the codebase

4. ✅ Updated Tests

  • Modified ArchiveExtractToDirectory test helper to use WriteToDirectory
  • Added comprehensive test for new WriteToDirectoryAsync method
  • Added test for progress reporting with IProgress<ProgressReport>
  • All tests passing (481+ tests across all archive types)

5. ✅ Code Quality

  • Renamed misleading variable emptyDirectory to parentDirectory
  • Added proper XML documentation for all methods
  • Follows existing code conventions and patterns

6. ✅ Merged master branch

  • Successfully merged latest master branch
  • No merge conflicts
  • All tests still passing after merge

7. ✅ Standardized on IProgress

  • Replaced Action<double> with IProgress<ProgressReport> for consistency with the rest of the codebase
  • Progress reports now include entry path, bytes transferred, and total bytes
  • More informative progress reporting aligned with existing patterns

8. ✅ Code Formatting

  • Ran CSharpier formatting to ensure consistent code style
  • Documented check-format process in AGENTS.md with clear commands
  • Added instructions for dotnet csharpier check . and dotnet csharpier format .

9. ✅ Removed CancellationToken from sync method

  • Removed CancellationToken parameter from synchronous WriteToDirectory(string, ExtractionOptions?, IProgress<ProgressReport>?)
  • Cancellation tokens are for async operations; sync methods don't need them
  • Keeps API clean and consistent with standard .NET patterns

API Consistency Verification

All archive and reader types now have consistent WriteTo* naming patterns:

IArchive (IArchiveExtensions):

  • WriteToDirectory(string, ExtractionOptions?)
  • WriteToDirectory(string, ExtractionOptions?, IProgress?)
  • WriteToDirectoryAsync(string, ExtractionOptions?, IProgress?, CancellationToken)

IArchiveEntry (IArchiveEntryExtensions):

  • WriteTo, WriteToAsync, WriteToDirectory, WriteToDirectoryAsync, WriteToFile, WriteToFileAsync

IReader (IReaderExtensions):

  • WriteEntryTo, WriteAllToDirectory, WriteAllToDirectoryAsync, WriteEntryToDirectory, WriteEntryToDirectoryAsync, WriteEntryToFile, WriteEntryToFileAsync

ArchiveFactory (static):

  • WriteToDirectory(string, string, ExtractionOptions?)

Breaking Change Notice

⚠️ Breaking Change: ExtractToDirectory method has been removed.

Migration Guide:

// Old code:
archive.ExtractToDirectory(destination, progressCallback, cancellationToken);

// New code with IProgress (sync):
var progress = new Progress<ProgressReport>(report => 
{
    Console.WriteLine($"Progress: {report.PercentComplete:F2}%");
});
archive.WriteToDirectory(destination, options: null, progress);

// Or async (with cancellation):
await archive.WriteToDirectoryAsync(destination, options: null, progress, cancellationToken);

Test Results

  • ✅ 129 Zip archive tests passed
  • ✅ 35 Zip async tests passed (including new progress test)
  • ✅ 221 general archive tests passed
  • ✅ 119 Rar archive tests passed
  • ✅ 17 Tar archive tests passed
  • ✅ CSharpier formatting check passes

Files Modified

  • src/SharpCompress/Archives/IArchiveExtensions.cs - Main API changes with IProgress and formatting
  • tests/SharpCompress.Test/ArchiveTests.cs - Test helper update
  • tests/SharpCompress.Test/Zip/ZipArchiveAsyncTests.cs - New async tests with progress
  • AGENTS.md - Added detailed documentation for check-format process
Original prompt

This section details on the original issue you should resolve

<issue_title>WriteToDirectory v ExtractToDirectory consistency</issue_title>
<issue_description>The static method WriteToDirectory now internally calls ExtractToDirectory, which creates a naming inconsistency. Consider either renaming the public method to ExtractToDirectory for consistency, or documenting why WriteToDirectory internally uses ExtractToDirectory to help future maintainers understand the design choice.

I like WriteToDirectory (and async variants) but unsure of ExtractToDirectory.

ExtractToDirectory should probably be removed in favor of things named WriteToDirectory.

All archive and reader types need to have WriteTo options via itself or a parent class</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits December 19, 2025 11:57
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor WriteToDirectory for naming consistency Standardize extraction API to WriteToDirectory, add async support Dec 19, 2025
Copilot AI requested a review from adamhathcock December 19, 2025 12:11
@adamhathcock
Copy link
Owner

@copilot merge master and check for issues

Copy link
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot merge master and check for issues

Merged master branch successfully (commit 17bde8d). All tests passing:

  • ✅ 221 archive tests
  • ✅ 129 Zip tests
  • ✅ Build succeeds with no errors

The merge included recent progress reporting changes from master which integrate cleanly with the WriteToDirectory refactoring.

@adamhathcock
Copy link
Owner

@copilot remove usages of Action or anything that isn't IProgress for progress

…porting

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot remove usages of Action or anything that isn't IProgress for progress

Replaced Action<double> with IProgress<ProgressReport> for progress reporting (commit 2f0eb0b). The API now uses the standard IProgress<ProgressReport> pattern consistent with the rest of the codebase. Progress reports include entry path, bytes transferred, and total bytes. All tests passing (221 archive tests, 35 Zip async tests including new progress test).

Copilot AI changed the title Standardize extraction API to WriteToDirectory, add async support Standardize extraction API to WriteToDirectory with IProgress support Dec 19, 2025
@adamhathcock
Copy link
Owner

@copilot run check-format and document it

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot run check-format and document it

Ran dotnet csharpier check . and formatted code with dotnet csharpier format . (commit c701bbb). All 707 files now pass formatting check. Documented the check-format process in AGENTS.md with clear commands:

  • dotnet csharpier check . - Check formatting without modifying files
  • dotnet csharpier format . - Format all files to match CSharpier style

The formatting changes consolidated multi-line ProgressReport constructions to single lines for consistency.

@adamhathcock
Copy link
Owner

@copilot WriteToDirectory(string, ExtractionOptions?, IProgress?, CancellationToken) shouldn't be an overload as there doesn't need to be a CancellationToken for non-async methods. There should only be WriteToDirectory(string, ExtractionOptions?, IProgress?)

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WriteToDirectory v ExtractToDirectory consistency

2 participants