Skip to content

Conversation

@capitalistspz
Copy link

@capitalistspz capitalistspz commented Oct 15, 2025

If a sourcePath "foo/" is provided, basePath = "foo/".

Which results in basePath.Length + 1 being one character past the directory separator.
So for a file, bar, in source directory, foo, the argument to FormatArchivePath is $"{destDirectory}foo/ar" instead of the likely intended $"{destDirectory}foo/bar".

@coderabbitai
Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

Updated archive planning in ThunderstoreCLI/Commands/BuildCommand.cs to compute relative file paths using Path.GetRelativePath(basePath, filename) instead of substring operations. This adjusts targetPath construction for files added from directories to produce correct relative zip entry paths. No changes were made to control flow, error handling, or public APIs.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request lacks any description, so there is no information relating to the changeset or its purpose. Please provide a brief description summarizing the path resolution change and the bug fix to clarify the purpose of this pull request.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly describes the specific issue being addressed—the missing first character in file paths when the source directory ends with a slash—and accurately reflects the main change in the code.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1116dc7 and 58d13f6.

📒 Files selected for processing (1)
  • ThunderstoreCLI/Commands/BuildCommand.cs (1 hunks)
🔇 Additional comments (1)
ThunderstoreCLI/Commands/BuildCommand.cs (1)

228-228: LGTM! Using Path.GetRelativePath is the correct fix.

This properly handles the edge case where basePath ends with /, avoiding the substring pitfalls. The idiomatic .NET approach ensures cross-platform compatibility and correct path normalization.

Consider adding a test case to verify the fix works when the source directory path ends with /. Something like:

// Test with trailing slash
AddPathToArchivePlan(plan, "src/", "dest/");
// Verify files are copied with correct paths (no missing first character)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant