Refactor systemDetectionConfig for platform commands #2
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the platform detection logic and expands project detection capabilities for the Copilot CLI integration. However, it introduces Windows platform support that conflicts with the project's documented constraints.
Key Changes:
- Replaced simple command detection with platform-specific match expression for Darwin, Linux, and Windows
- Added new detection paths and files including
.github/instructionsdirectory and various AI assistant instruction files (AGENTS.md, CLAUDE.md, GEMINI.md) - Enhanced error handling with explicit JSON_THROW_ON_ERROR flags and added @throws annotations
| Platform::Windows => [ | ||
| 'command' => 'where copilot 2>nul', | ||
| ], |
There was a problem hiding this comment.
The addition of Windows platform detection conflicts with the project's documented constraint that native Windows is not supported (only WSL). According to the README and project guidelines, this package explicitly does not support native Windows. The Windows-specific command 'where copilot 2>nul' should not be added, as it would incorrectly suggest Windows compatibility. Users on Windows should use WSL, which would be detected as Platform::Linux.
| return match ($platform) { | ||
| Platform::Darwin, Platform::Linux => [ | ||
| 'command' => 'command -v copilot', | ||
| ], | ||
| Platform::Windows => [ | ||
| 'command' => 'where copilot 2>nul', | ||
| ], | ||
| }; |
There was a problem hiding this comment.
The new platform-specific detection logic lacks test coverage. Tests should be added to verify that Platform::Linux returns 'command -v copilot', and the Windows case should be removed or tested appropriately based on the resolution of the Windows support issue.
| return [ | ||
| 'files' => ['.github/copilot-instructions.md'], | ||
| 'paths' => ['.github/instructions'], | ||
| 'files' => ['.github/copilot-instructions.md', '.github/instructions/laravel-boost.instructions.md', 'AGENTS.md', 'CLAUDE.md', 'GEMINI.md'], | ||
| ]; |
There was a problem hiding this comment.
The new 'paths' key and additional files in the detection configuration lack test coverage. Tests should verify that the projectDetectionConfig includes the '.github/instructions' path and all the newly added files ('.github/instructions/laravel-boost.instructions.md', 'AGENTS.md', 'CLAUDE.md', 'GEMINI.md').
| 'paths' => ['.github/instructions'], | ||
| 'files' => ['.github/copilot-instructions.md', '.github/instructions/laravel-boost.instructions.md', 'AGENTS.md', 'CLAUDE.md', 'GEMINI.md'], |
There was a problem hiding this comment.
The newly added detection files ('AGENTS.md', 'CLAUDE.md', 'GEMINI.md') and the '.github/instructions' path are not documented in the README or other documentation. Users should be informed about what these files are for and how they relate to the project detection mechanism, particularly how they differ from the standard '.github/copilot-instructions.md' file.
|
There are no plans for native Windows support.
|
2 participants
This pull request updates the platform detection logic and project configuration for the Copilot CLI integration, improving compatibility and detection accuracy across different operating systems and project setups.
Platform detection improvements:
systemDetectionConfigmethod to use amatchexpression, providing platform-specific commands for detecting the Copilot CLI:command -v copilotfor Darwin/Linux andwhere copilot 2>nulfor Windows.Project detection enhancements:
projectDetectionConfigmethod to include a newpathskey for.github/instructionsand added additional files for detection:.github/instructions/laravel-boost.instructions.md,AGENTS.md,CLAUDE.md, andGEMINI.md.Dependency and import updates:
FileNotFoundExceptionandJsonExceptionto support enhanced error handling and JSON operations.