Improve Linux path safety - #12
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves cross-platform compatibility by replacing Windows-specific hard-coded backslash path separators with OS-safe Path.Combine method calls, enabling the code to run correctly on Linux and other non-Windows operating systems.
Changes:
- Replaced hard-coded Windows path separators (
\\) withPath.Combine()calls across multiple plugin files - Modified path construction in catalog and lookup functionality to use OS-safe path combination methods
- Added
using System.Runtime.InteropServicesimport to LookupPlugin.cs
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ValidatePlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| SqlPlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| SplitPlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| SortPlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| RankPlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| Program.cs | Updated plugin folder path construction using Path.Combine and Path.DirectorySeparatorChar |
| ParsePlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| MapPlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| LookupPlugin.cs | Updated multiple path concatenations to use Path.Combine and added System.Runtime.InteropServices import |
| HashPlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| ExportPlugin.cs | Updated multiple path concatenations to use Path.Combine for file path construction |
| CutPlugin.cs | Updated path concatenation to use Path.Combine for file path construction |
| CatalogPlugin.cs | Updated multiple path concatenations using Path.Combine and Path.DirectorySeparatorChar |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| foreach (var de in output) | ||
| { | ||
| var path = $"{options.OutputFolder}\\xref\\{options.Prefix}-xref-{de.Key}.tmp"; | ||
| var path = Path.Combine(options.OutputFolder, "xref", "${options.Prefix}-xref-{de.Key}.tmp"); |
There was a problem hiding this comment.
The string interpolation syntax is incorrect. The opening quote is a dollar sign followed by a double quote, but should be a dollar sign with the interpolation expression enclosed in curly braces. The variable reference "${options.Prefix}" should be "{options.Prefix}" to properly interpolate the value.
| var path = Path.Combine(options.OutputFolder, "xref", "${options.Prefix}-xref-{de.Key}.tmp"); | |
| var path = Path.Combine(options.OutputFolder, "xref", $"{options.Prefix}-xref-{de.Key}.tmp"); |
| // Navigate up to the solution root | ||
| //var pluginFolder = $"{Directory.GetCurrentDirectory()}\\Plugins\\"; | ||
| var pluginFolder = $"{AppDomain.CurrentDomain.BaseDirectory}\\Plugins\\{name}\\"; | ||
| var pluginFolder = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Plugins", name) + Path.DirectorySeparatorChar; |
There was a problem hiding this comment.
Inconsistent approach to path construction. While Path.Combine is used, a trailing directory separator is manually appended using Path.DirectorySeparatorChar. This defeats the purpose of using Path.Combine for OS-safe paths. Consider using Path.Combine for the full path construction and only append the trailing separator if it's truly necessary for the subsequent logic.
| var pluginFolder = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Plugins", name) + Path.DirectorySeparatorChar; | |
| var pluginFolder = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Plugins", name); |
| private static async Task DoXReference(CatalogOptions options) | ||
| { | ||
| var xrefFolder = $"{options.OutputFolder}\\xref\\"; | ||
| var xrefFolder = $"{options.OutputFolder}{Path.DirectorySeparatorChar}xref{Path.DirectorySeparatorChar}"; |
There was a problem hiding this comment.
Inconsistent approach to path construction. String interpolation with Path.DirectorySeparatorChar is used instead of Path.Combine. For consistency with the rest of the PR and proper OS-safe path handling, use Path.Combine(options.OutputFolder, "xref") and add the trailing separator separately if needed, or better yet, avoid the trailing separator altogether if possible.
| var xrefFolder = $"{options.OutputFolder}{Path.DirectorySeparatorChar}xref{Path.DirectorySeparatorChar}"; | |
| var xrefFolder = Path.Combine(options.OutputFolder, "xref"); |
Use OS-safe concatenation of paths using Path.Combine instead of combining manually for compatibility with non-Windows OSes