-
Notifications
You must be signed in to change notification settings - Fork 2
Add mageforge:hyva:tokens command for Hyvä design token generation #58
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
…sign tokens Co-authored-by: dermatz <[email protected]>
Co-authored-by: dermatz <[email protected]>
…onsistency Co-authored-by: dermatz <[email protected]>
Co-authored-by: dermatz <[email protected]>
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 command to generate Hyvä design tokens via the npx hyva-tokens utility. The command validates that the target theme is a Hyvä theme, checks for required dependencies, executes the token generation, and handles different output locations for vendor vs. custom themes.
Changes:
- Added
TokensCommandfor generating Hyvä design tokens from design.tokens.json or hyva.config.json - Registered command in di.xml as
mageforge_theme_tokens - Updated documentation (README.md, docs/commands.md, CHANGELOG.md) with command usage and examples
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Console/Command/Theme/TokensCommand.php | New command implementation for Hyvä token generation with theme validation, interactive selection, and vendor theme handling |
| src/etc/di.xml | Registered the new TokensCommand in Magento's dependency injection configuration |
| docs/commands.md | Added comprehensive documentation for the tokens command including usage, examples, and implementation details |
| README.md | Added tokens command to the command list table and quick start guide |
| CHANGELOG.md | Documented the new feature in the unreleased section |
| $varGeneratedPath = $currentDir . '/var/generated/hyva-token/' . str_replace('/', '/', $themeCode); | ||
|
|
||
| if (!$this->fileDriver->isDirectory($varGeneratedPath)) { | ||
| $this->fileDriver->createDirectory($varGeneratedPath, 0755); |
Copilot
AI
Jan 12, 2026
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.
Potential directory creation failure not handled. The createDirectory call can throw exceptions, but they're not caught separately from the main try-catch block. If directory creation fails in the vendor theme path, the error message will say "Failed to generate Hyvä design tokens" which is misleading since the tokens were already generated but the copy operation failed. Consider adding specific error handling for directory operations.
| $this->fileDriver->createDirectory($varGeneratedPath, 0755); | |
| try { | |
| $this->fileDriver->createDirectory($varGeneratedPath, 0755); | |
| } catch (\Exception $e) { | |
| chdir($currentDir); | |
| $this->io->error( | |
| 'Failed to prepare vendor directory for Hyvä design tokens: ' | |
| . $e->getMessage() | |
| ); | |
| return Cli::RETURN_FAILURE; | |
| } |
| $this->setName($this->getCommandName('hyva', 'tokens')) | ||
| ->setAliases(['m:h:t']) |
Copilot
AI
Jan 12, 2026
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.
Alias inconsistency. The alias 'm:h:t' suggests this is a hyva command, but if the command is corrected to 'mageforge:theme:tokens' (as indicated by its location in the Theme/ directory and the di.xml registration), the alias should be 'm:t:t' to follow the pattern of other theme commands (m:t:l, m:t:b, m:t:w).
| $this->setName($this->getCommandName('hyva', 'tokens')) | |
| ->setAliases(['m:h:t']) | |
| $this->setName($this->getCommandName('theme', 'tokens')) | |
| ->setAliases(['m:t:t']) |
|
|
||
| ### Added | ||
|
|
||
| - feat: add `mageforge:theme:tokens` command to generate Hyvä design tokens from design.tokens.json or hyva.config.json |
Copilot
AI
Jan 12, 2026
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.
Inconsistent command name in changelog. The changelog mentions 'mageforge:theme:tokens' which contradicts the actual implementation that uses 'mageforge:hyva:tokens'. While this might be the intended correct name (matching the di.xml and PR title), the actual code implementation needs to be updated to match.
| - feat: add `mageforge:theme:tokens` command to generate Hyvä design tokens from design.tokens.json or hyva.config.json | |
| - feat: add `mageforge:hyva:tokens` command to generate Hyvä design tokens from design.tokens.json or hyva.config.json |
| 4. Generate Hyvä design tokens (for Hyvä themes): | ||
|
|
||
| ```bash | ||
| bin/magento mageforge:hyva:tokens <theme-code> |
Copilot
AI
Jan 12, 2026
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.
Inconsistent command name in usage example. The command name should match the actual implementation and the project structure. If this command is in the Theme/ directory, it should use 'mageforge:theme:tokens' not 'mageforge:hyva:tokens'.
| if (empty($themeCode)) { | ||
| $themes = $this->themeList->getAllThemes(); | ||
| $options = array_map(fn($theme) => $theme->getCode(), $themes); | ||
|
|
||
| $themeCodePrompt = new SelectPrompt( | ||
| label: 'Select theme to generate tokens for', | ||
| options: $options, | ||
| scroll: 10, | ||
| hint: 'Arrow keys to navigate, Enter to confirm', | ||
| ); | ||
|
|
||
| try { | ||
| $themeCode = $themeCodePrompt->prompt(); | ||
| \Laravel\Prompts\Prompt::terminal()->restoreTty(); | ||
| } catch (\Exception $e) { | ||
| $this->io->error('Interactive mode failed: ' . $e->getMessage()); | ||
| return Cli::RETURN_FAILURE; | ||
| } | ||
| } |
Copilot
AI
Jan 12, 2026
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 interactive theme selection does not check if the environment supports interactive terminals, unlike BuildCommand which uses isInteractiveTerminal() to handle non-interactive environments gracefully. In non-interactive CI/CD environments, this will fail with an exception. Consider adding a check similar to BuildCommand's implementation to handle non-interactive environments by displaying available themes and returning gracefully instead of attempting to show an interactive prompt.
|
|
||
| if ($isVendorTheme) { | ||
| // Store in var/generated/hyva-token/{ThemeCode}/ for vendor themes | ||
| $varGeneratedPath = $currentDir . '/var/generated/hyva-token/' . str_replace('/', '/', $themeCode); |
Copilot
AI
Jan 12, 2026
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 str_replace call is replacing '/' with '/', which is a no-op and doesn't change the string. This appears to be a mistake. If the intention is to keep the slash, this line can be simplified to just concatenate themeCode. If the intention is to replace slashes with a different character (like '-' or '_'), the second parameter should be updated accordingly.
| $varGeneratedPath = $currentDir . '/var/generated/hyva-token/' . str_replace('/', '/', $themeCode); | |
| $varGeneratedPath = $currentDir . '/var/generated/hyva-token/' . $themeCode; |
| $sourceFilePath = $tailwindPath . '/generated/hyva-tokens.css'; | ||
|
|
||
| if ($isVendorTheme) { | ||
| // Store in var/generated/hyva-token/{ThemeCode}/ for vendor themes | ||
| $varGeneratedPath = $currentDir . '/var/generated/hyva-token/' . str_replace('/', '/', $themeCode); | ||
|
|
||
| if (!$this->fileDriver->isDirectory($varGeneratedPath)) { | ||
| $this->fileDriver->createDirectory($varGeneratedPath, 0755); | ||
| } | ||
|
|
||
| $generatedFilePath = $varGeneratedPath . '/hyva-tokens.css'; | ||
|
|
||
| // Copy file to var/generated location | ||
| if ($this->fileDriver->isExists($sourceFilePath)) { | ||
| $this->fileDriver->copy($sourceFilePath, $generatedFilePath); | ||
| } | ||
|
|
||
| $this->io->success('Hyvä design tokens generated successfully.'); | ||
| $this->io->note('This is a vendor theme. Tokens have been saved to var/generated/hyva-token/ instead.'); | ||
| $this->io->text('Generated file: ' . $generatedFilePath); | ||
| } else { | ||
| $generatedFilePath = $sourceFilePath; | ||
| $this->io->success('Hyvä design tokens generated successfully.'); | ||
| $this->io->text('Generated file: ' . $generatedFilePath); | ||
| } |
Copilot
AI
Jan 12, 2026
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.
Missing error handling if the source file doesn't exist after npx hyva-tokens execution. The code only checks if the file exists before copying for vendor themes but doesn't handle the case where the token generation succeeds but doesn't create the expected file. For non-vendor themes, the code assumes the file exists and displays it as the generated file path without verification. Consider adding explicit validation that the file was created before reporting success.
Implements
mageforge:theme:tokenscommand to generate Hyvä design tokens vianpx hyva-tokens, supporting design.tokens.json, Figma tokens, inline configurations, and custom CSS selectors.Implementation
New Command:
TokensCommandatsrc/Console/Command/Theme/TokensCommand.phpmageforge:theme:tokens(shortcode:m:t:t)web/tailwinddirectory using Shell serviceRegistration: Added
mageforge_theme_tokenstosrc/etc/di.xmlDocumentation: Updated README.md, docs/commands.md, and CHANGELOG.md with command details and token configuration examples
Usage
Token Configuration Examples
Supports multiple token sources in
hyva.config.json:Generates
generated/hyva-tokens.cssin the theme's tailwind directory.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/repos/magento/magento-coding-standard/zipball/f24dd98f46a98cd7081e4648b19299c27db35264/usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/JEqquD /usr/bin/composer create-project magento/magento-coding-standard --stability=dev /tmp/magento-coding-standard(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original 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.