Skip to content

chore(fileops): route ShowDirectory OS dispatch through ISystemEnvironment#9

Merged
ANcpLua merged 1 commit into
mainfrom
chore/fileops-os-dispatch-via-isystemenvironment
May 14, 2026
Merged

chore(fileops): route ShowDirectory OS dispatch through ISystemEnvironment#9
ANcpLua merged 1 commit into
mainfrom
chore/fileops-os-dispatch-via-isystemenvironment

Conversation

@ANcpLua
Copy link
Copy Markdown
Owner

@ANcpLua ANcpLua commented May 14, 2026

Summary

  • PR chore(ocr): finish ISystemEnvironment abstraction — read via the interface #6 ("finish ISystemEnvironment abstraction — read via the interface") routed Tesseract's file reads through ISystemEnvironment, but FileOperations.ShowDirectory was missed — it still called OperatingSystem.IsWindows() / IsMacOS() directly, leaving the abstraction half-closed on main.
  • Route those OS checks through RuntimeSystemEnvironment.Instance so RuntimeSystemEnvironment.cs is now the only production-code site that touches OperatingSystem.*, Environment.Is64BitOperatingSystem, File.Exists, or File.ReadAllTextAsync. That closure property is what "finish the abstraction" actually means.
  • Behavior is unchanged. ShowDirectory stays [ExcludeFromCodeCoverage] because Process.Start is the untestable shell-out, not the OS branch.

Verification

  • dotnet build — 0 warnings, 0 errors
  • dotnet test — 256/256 passing on net10.0, net9.0, net8.0
  • Abstraction-closure grep on CreatePdf.NET/:
    grep -rn 'OperatingSystem\.(IsMacOS|IsWindows|IsLinux|IsFreeBSD)|Environment\.Is64BitOperatingSystem|File\.Exists|File\.ReadAllText' CreatePdf.NET/Internal/*.cs
    
    → only RuntimeSystemEnvironment.cs matches as a direct caller; the one TesseractOcrProvider.cs hit (_systemEnvironment.Is64BitOperatingSystem) is interface access, not direct.

🤖 Generated with Claude Code

…nment

After PR #6 added ReadAllTextAsync to ISystemEnvironment, FileOperations.
ShowDirectory was still the one production-code site outside the
RuntimeSystemEnvironment implementation that called OperatingSystem.* /
File.* directly. Route the Windows/macOS detection through
RuntimeSystemEnvironment.Instance so the abstraction is actually closed:
RuntimeSystemEnvironment.cs is now the sole site touching OperatingSystem.*,
Environment.Is64BitOperatingSystem, File.Exists, and File.ReadAllTextAsync.

Behavior unchanged. ShowDirectory remains [ExcludeFromCodeCoverage] because
the Process.Start shell-out is the untestable part, not the OS check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 14, 2026 21:48
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: adf8d853-a679-4c68-b87f-5f1d9b3ce177

📥 Commits

Reviewing files that changed from the base of the PR and between 5d2f6b3 and bdf3294.

📒 Files selected for processing (1)
  • CreatePdf.NET/Internal/FileOperations.cs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: copilot-pull-request-reviewer
  • GitHub Check: build-test
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.cs: Use field-backed properties with C# 14 features for property definitions where validation is needed
Enable nullable reference types project-wide and use nullable annotations to prevent null reference exceptions
Use async-first approach for all I/O operations in file and PDF generation
Use conditional compilation directives (e.g., #if NET10_0_OR_GREATER) when implementing framework-specific features across .NET 10, 9, and 8 targets
Ensure GenerateDocumentationFile is enabled in .csproj and include comprehensive XML comments in public APIs for documentation generation

Files:

  • CreatePdf.NET/Internal/FileOperations.cs
🔇 Additional comments (1)
CreatePdf.NET/Internal/FileOperations.cs (1)

78-85: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Internal improvements to the directory access mechanism for enhanced maintainability and testability.

Walkthrough

ShowDirectory method now uses a RuntimeSystemEnvironment instance for OS detection instead of direct OperatingSystem static checks, enabling dependency injection and decoupling from the .NET runtime API.

Changes

OS Detection Abstraction

Layer / File(s) Summary
OS detection abstraction in ShowDirectory
CreatePdf.NET/Internal/FileOperations.cs
ShowDirectory obtains RuntimeSystemEnvironment instance and uses its IsWindows/IsMacOS properties for platform-specific command selection, replacing static OperatingSystem.IsWindows()/IsMacOS() checks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: routing OS dispatch through ISystemEnvironment in the ShowDirectory method.
Description check ✅ Passed The description is directly related to the changeset, explaining the abstraction closure, motivation, and verification performed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/fileops-os-dispatch-via-isystemenvironment
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch chore/fileops-os-dispatch-via-isystemenvironment

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

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the OS-dispatch abstraction for FileOperations.ShowDirectory by routing platform checks through RuntimeSystemEnvironment.Instance instead of calling OperatingSystem.* directly.

Changes:

  • Replaces direct OperatingSystem.IsWindows() / IsMacOS() checks with RuntimeSystemEnvironment.Instance properties.
  • Keeps existing shell-out behavior unchanged for Windows, macOS, and fallback platforms.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the ShowDirectory method in FileOperations.cs to utilize a RuntimeSystemEnvironment instance for operating system detection instead of static OperatingSystem checks. The review feedback suggests explicitly typing the environment variable as ISystemEnvironment to ensure the logic depends on the abstraction rather than the concrete implementation, which would also facilitate easier unit testing in the future.

[ExcludeFromCodeCoverage(Justification = "Interacts with OS shell")]
public static void ShowDirectory(string path)
{
var environment = RuntimeSystemEnvironment.Instance;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To fully adhere to the ISystemEnvironment abstraction and ensure the method depends only on the interface's contract, consider explicitly typing the environment variable as ISystemEnvironment. Using var here infers the concrete RuntimeSystemEnvironment type, which couples the logic to the implementation rather than the abstraction. Additionally, although this method is currently excluded from code coverage, depending on the interface rather than the singleton property would facilitate easier unit testing of the OS-selection logic in the future if the dependency were to be injected.

        ISystemEnvironment environment = RuntimeSystemEnvironment.Instance;

@ANcpLua ANcpLua merged commit bfbd976 into main May 14, 2026
4 checks passed
@ANcpLua ANcpLua deleted the chore/fileops-os-dispatch-via-isystemenvironment branch May 14, 2026 21:55
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.

2 participants