-
Notifications
You must be signed in to change notification settings - Fork 2
Add hyva-tokens command for design token CSS generation #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: dermatz <6103201+dermatz@users.noreply.github.com>
Co-authored-by: dermatz <6103201+dermatz@users.noreply.github.com>
Co-authored-by: dermatz <6103201+dermatz@users.noreply.github.com>
…nalysis warning Co-authored-by: dermatz <6103201+dermatz@users.noreply.github.com>
Confirmed - the code already follows Magento 2 coding standards: ✅ The Codacy warnings are from WordPress standards being incorrectly applied. In Magento 2 CLI context:
The implementation matches the patterns used in existing Magento 2 commands (BuildCommand, CheckCommand, etc.). |
There was a problem hiding this 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 implements a new mageforge:hyva:tokens command that generates CSS custom properties from Hyvä design token definitions, enabling consistent design system management across Hyvä themes. The implementation includes comprehensive token parsing for multiple formats (default and Figma), configuration management, and proper handling of vendor themes.
Key Changes
- New Hyva Tokens Command: Adds
mageforge:hyva:tokenswith support for multiple token sources (file-based, Figma format, and inline values), configurable CSS selectors for Tailwind v3/v4 compatibility, and special handling for vendor themes with temporary storage - Service Refactoring: Extracts environment management, system info, database info, and search engine detection into reusable services for better code organization and reusability
- Builder Improvements: Enhances watch mode consistency across theme builders by replacing
passthruwithshell->execute()and adds proper phpcs ignore comments for necessarychdirand shell operations
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Console/Command/Hyva/TokensCommand.php | New command implementing Hyvä design tokens generation with interactive theme selection and comprehensive error handling |
| src/Service/HyvaTokens/TokenProcessor.php | Orchestrates token generation workflow from reading configuration to writing CSS output |
| src/Service/HyvaTokens/TokenParser.php | Parses and normalizes design tokens from multiple formats (default/Figma) into flat structure |
| src/Service/HyvaTokens/CssGenerator.php | Generates CSS custom properties from tokens and handles file writing with directory creation |
| src/Service/HyvaTokens/ConfigReader.php | Reads hyva.config.json configuration with defaults and determines output paths for vendor/non-vendor themes |
| src/Service/ThemeSelectionService.php | Extracted theme selection logic supporting interactive prompts and validation for Hyvä themes |
| src/Service/EnvironmentService.php | Extracted secure environment variable management with sanitization and terminal detection |
| src/Service/SystemInfoService.php | Extracted system information retrieval (Node, Composer, NPM, Git versions, PHP extensions) |
| src/Service/DatabaseInfoService.php | Extracted database information retrieval with multiple fallback methods |
| src/Service/SearchEngineInfoService.php | Extracted Elasticsearch/OpenSearch detection and version reporting |
| src/Console/Command/Theme/BuildCommand.php | Refactored to use EnvironmentService for terminal detection and environment management |
| src/Console/Command/System/CheckCommand.php | Refactored to use new info services for cleaner separation of concerns |
| src/Service/ThemeBuilder/TailwindCSS/Builder.php | Updated watch mode to use shell->execute() and added phpcs ignore comments |
| src/Service/ThemeBuilder/HyvaThemes/Builder.php | Updated watch mode to use shell->execute() and added phpcs ignore comments |
| src/Service/ThemeBuilder/MagentoStandard/Builder.php | Added phpcs ignore comments and updated watch mode consistency |
| src/Service/StaticContentDeployer.php | Added phpcs ignore comment for escapeshellarg usage |
| src/Service/DependencyChecker.php | Removed unnecessary blank line for code cleanliness |
| src/etc/di.xml | Registered new TokensCommand in dependency injection configuration |
| phpcs.xml | New PHP CodeSniffer configuration using PSR-12 standard |
| README.md | Added documentation for new hyva:tokens command |
| docs/commands.md | Added comprehensive documentation for HyvaTokensCommand with configuration examples |
| docs/advanced_usage.md | Added detailed usage examples for Hyvä design tokens feature |
| $io->error('Failed to start watch mode: ' . $e->getMessage()); | ||
| // phpcs:ignore MEQP1.Security.DiscouragedFunction -- chdir is necessary to restore original directory | ||
| chdir($currentDir); | ||
| return false; |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The watch mode changes the directory to run npm watch but never restores the original directory on successful execution. This means the working directory remains changed after the command completes. While watch mode is typically a blocking operation that runs until interrupted, it's good practice to restore the directory in a finally block or ensure it's restored in all code paths for consistency and to prevent unexpected behavior if watch exits normally.
| $io->error('Failed to start watch mode: ' . $e->getMessage()); | |
| // phpcs:ignore MEQP1.Security.DiscouragedFunction -- chdir is necessary to restore original directory | |
| chdir($currentDir); | |
| return false; | |
| $io->error( | |
| 'Failed to start watch mode: ' . $e->getMessage() | |
| ); | |
| return false; | |
| } finally { | |
| // phpcs:ignore MEQP1.Security.DiscouragedFunction -- chdir is necessary to restore original directory | |
| chdir($currentDir); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit fe8d901. Added finally block to ensure directory is always restored, even on successful execution.
| try { | ||
| chdir($tailwindPath); | ||
| passthru('npm run watch'); | ||
| if ($isVerbose) { | ||
| $io->text('Starting watch mode...'); | ||
| } | ||
| $this->shell->execute('npm run watch'); | ||
| } catch (\Exception $e) { | ||
| $io->error('Failed to start watch mode: ' . $e->getMessage()); | ||
| // phpcs:ignore MEQP1.Security.DiscouragedFunction -- chdir is necessary to restore original directory | ||
| chdir($currentDir); | ||
| return false; | ||
| } |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The watch mode changes the directory to run npm watch but never restores the original directory on successful execution. This means the working directory remains changed after the command completes. While watch mode is typically a blocking operation that runs until interrupted, it's good practice to restore the directory in a finally block or ensure it's restored in all code paths for consistency and to prevent unexpected behavior if watch exits normally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit fe8d901. Added finally block to ensure directory is always restored, even on successful execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was already fixed in commit fe8d901. The finally block (lines 221-224) now ensures the directory is always restored to $currentDir, whether the watch command succeeds or fails. The same pattern was applied to HyvaThemes/Builder.php as well.
|
@copilot please edit summary the Implementation in the first comment |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…rds compliance - Replace manual path extraction with dirname() for better reliability - Add CSS value sanitization to prevent syntax issues from untrusted tokens - Use str_contains() instead of strpos() for modern PHP 8.3+ - Improve vendor theme path fallback logic with multi-level handling - Add finally blocks to restore working directory in watch mode - Add phpcs:ignore comment for shell_exec TTY detection Co-authored-by: dermatz <6103201+dermatz@users.noreply.github.com>
Updated PR description with comprehensive implementation summary including all features, changes, and architectural improvements made across the 20 commits in this PR. |
Implementation Plan for Hyva Tokens Command
HyvaTokensCommandin/src/Console/Command/Hyva/TokensCommand.php/src/etc/di.xml/src/Service/HyvaTokens/TokenProcessor.php/src/Service/HyvaTokens/ConfigReader.phpOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.