-
-
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
Changes from all commits
adf3ada
0d1ae63
f2b5a5f
2e9e3ec
17cf63b
e56ffce
62202cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,8 @@ Please ensure that your Magento installation meets this requirement before insta | |
| | `mageforge:theme:list` | Lists all available themes | `m:t:l` | | ||
| | `mageforge:theme:build` | Builds selected themes (CSS/TailwindCSS) | `m:t:b`, `frontend:build` | | ||
| | `mageforge:theme:watch` | Starts watch mode for theme development | `m:t:w`, `frontend:watch` | | ||
| | `mageforge:static:clean` | Clean static files, cache and generated files for a theme | `frontend:clean` | | ||
| | `mageforge:hyva:tokens` | Generate Hyvä design tokens (Hyvä themes only) | `m:h:t` | | ||
dermatz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| | `mageforge:static:clean` | Clean static files, cache and generated files for a theme | `m:st:c`,`frontend:clean` | | ||
|
|
||
| --- | ||
|
|
||
|
|
@@ -79,7 +80,15 @@ Please ensure that your Magento installation meets this requirement before insta | |
| bin/magento mageforge:theme:watch <theme-code> | ||
| ``` | ||
|
|
||
| 4. Enjoy automatic CSS rebuilding you work on your theme files! | ||
| 4. Generate Hyvä design tokens (for Hyvä themes): | ||
|
|
||
| ```bash | ||
| bin/magento mageforge:hyva:tokens <theme-code> | ||
|
||
| ``` | ||
|
|
||
| This creates a `generated/hyva-tokens.css` file from your design tokens configuration. | ||
|
|
||
| 5. Enjoy automatic CSS rebuilding as you work on your theme files! | ||
|
|
||
| ## Additional Documentation | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,164 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| declare(strict_types=1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace OpenForgeProject\MageForge\Console\Command\Theme; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Laravel\Prompts\SelectPrompt; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Magento\Framework\Console\Cli; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Magento\Framework\Filesystem\Driver\File; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Magento\Framework\Shell; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use OpenForgeProject\MageForge\Console\Command\AbstractCommand; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use OpenForgeProject\MageForge\Model\ThemeList; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use OpenForgeProject\MageForge\Model\ThemePath; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use OpenForgeProject\MageForge\Service\ThemeBuilder\BuilderPool; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Symfony\Component\Console\Input\InputArgument; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Symfony\Component\Console\Input\InputInterface; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Symfony\Component\Console\Output\OutputInterface; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Command for generating Hyvä design tokens | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class TokensCommand extends AbstractCommand | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param ThemeList $themeList | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param ThemePath $themePath | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param BuilderPool $builderPool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param File $fileDriver | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param Shell $shell | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public function __construct( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private readonly ThemeList $themeList, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private readonly ThemePath $themePath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private readonly BuilderPool $builderPool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private readonly File $fileDriver, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private readonly Shell $shell, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parent::__construct(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * {@inheritdoc} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| protected function configure(): void | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->setName($this->getCommandName('hyva', 'tokens')) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ->setAliases(['m:h:t']) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+46
to
+47
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->setName($this->getCommandName('hyva', 'tokens')) | |
| ->setAliases(['m:h:t']) | |
| $this->setName($this->getCommandName('theme', 'tokens')) | |
| ->setAliases(['m:t: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.
Inconsistency between command name and file location. The command is located in the Theme/ directory but uses 'hyva' as the group name. According to the project structure, commands in src/Console/Command/Theme/ should use 'theme' as the group (e.g., 'mageforge:theme:tokens'), while commands in src/Console/Command/Hyva/ use 'hyva'. This creates confusion with the di.xml registration name 'mageforge_theme_tokens' and the PR title which mentions 'mageforge:theme:tokens'. Either the command should be moved to the Hyva directory or the group name should be changed to 'theme'.
| $this->setName($this->getCommandName('hyva', 'tokens')) | |
| ->setAliases(['m:h:t']) | |
| $this->setName($this->getCommandName('theme', 'tokens')) | |
| ->setAliases(['m:t: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.
Missing environment protection pattern for Laravel Prompts. Other commands in the codebase (BuildCommand, CompatibilityCheckCommand) use setPromptEnvironment() and resetPromptEnvironment() methods to safely manage environment variables around Laravel Prompts usage. This command directly uses SelectPrompt without this protection, which could lead to environment pollution or interference with other processes.
| $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; | |
| $this->setPromptEnvironment(); | |
| try { | |
| $themeCodePrompt = new SelectPrompt( | |
| label: 'Select theme to generate tokens for', | |
| options: $options, | |
| scroll: 10, | |
| hint: 'Arrow keys to navigate, Enter to confirm', | |
| ); | |
| $themeCode = $themeCodePrompt->prompt(); | |
| \Laravel\Prompts\Prompt::terminal()->restoreTty(); | |
| } catch (\Exception $e) { | |
| $this->io->error('Interactive mode failed: ' . $e->getMessage()); | |
| return Cli::RETURN_FAILURE; | |
| } finally { | |
| $this->resetPromptEnvironment(); |
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.
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; |
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; | |
| } |
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.
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.