-
-
Notifications
You must be signed in to change notification settings - Fork 28
Fixed installer not replacing hyphen-delimited module prefix. #1993
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
WalkthroughAdds hyphenated placeholder replacements ('ys-base' => "$v-base", 'ys-search' => kebab($v) . "-search") and corresponding rename operations (including Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2).vortex/installer/src/Prompts/Handlers/ModulePrefix.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (2)
108-112: Hyphenated placeholders in file/dir names are not renamedContent is replaced for ys-*, but filenames/dirs named with hyphens (e.g., ys-base) won’t be renamed, potentially leaving stale scaffolding.
Apply this diff:
File::renameInDir($t . sprintf('/%s/modules/custom', $w), 'ys_base', $v . '_base'); File::renameInDir($t . sprintf('/%s/modules/custom', $w), 'ys_search', $v . '_search'); + File::renameInDir($t . sprintf('/%s/modules/custom', $w), 'ys-base', str_replace('_', '-', $v) . '-base'); + File::renameInDir($t . sprintf('/%s/modules/custom', $w), 'ys-search', str_replace('_', '-', $v) . '-search'); File::renameInDir($t . sprintf('/%s/modules/custom', $w), 'YsBase', Converter::pascal($v) . 'Base'); File::renameInDir($t . sprintf('/%s/sites/default/includes', $w), 'ys_base', $v . '_base');Optionally mirror hyphen renames under sites/default/includes if such entries exist.
102-106: Uppercase mappings likely drop suffixesMapping YSBASE/YSSEARCH to Converter::cobol($v) removes BASE/SEARCH entirely (YSBASE → MS). If placeholders are intended to expand to full tokens, include the suffix in the conversion.
Apply this diff:
- 'YSBASE' => Converter::cobol($v), - 'YSSEARCH' => Converter::cobol($v), + 'YSBASE' => Converter::cobol($v . '_base'), + 'YSSEARCH' => Converter::cobol($v . '_search'),If, instead, YSBASE/YSSEARCH are meant to represent just the uppercase prefix, please ignore and confirm usage sites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
.vortex/installer/tests/Fixtures/install/_baseline/tests/behat/features/counter.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/css/sw_base.cssis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/js/sw_base.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/sw_base.moduleis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/templates/ys-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (1)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (1)
97-106: Replacement mapping is comprehensive. No other hyphenated or uppercase stub tokens detected by the scan.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1993 +/- ##
===========================================
+ Coverage 77.83% 77.85% +0.01%
===========================================
Files 90 90
Lines 5550 5555 +5
Branches 44 44
===========================================
+ Hits 4320 4325 +5
Misses 1230 1230 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
09b732d to
10c5940
Compare
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (1)
55-68: Discovery misses hyphen-delimited modules — match both '_' and '-'discover() only searches _base/_core and strips underscores, so hyphenated modules (e.g., my-site-base) won’t be discovered or normalized. Match both delimiters and strip both.
Apply:
- $this->dstDir . sprintf('/%s/modules/custom/*_base', $this->webroot), - $this->dstDir . sprintf('/%s/modules/custom/*_core', $this->webroot), - $this->dstDir . sprintf('/%s/sites/all/modules/custom/*_base', $this->webroot), - $this->dstDir . sprintf('/%s/sites/all/modules/custom/*_core', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/*/modules/*_base', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/*/modules/*_core', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/*/modules/custom/*_base', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/*/modules/custom/*_core', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/custom/*/modules/*_base', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/custom/*/modules/*_core', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/custom/*/modules/custom/*_base', $this->webroot), - $this->dstDir . sprintf('/%s/profiles/custom/*/modules/custom/*_core', $this->webroot), + $this->dstDir . sprintf('/%s/modules/custom/*[_-]base', $this->webroot), + $this->dstDir . sprintf('/%s/modules/custom/*[_-]core', $this->webroot), + $this->dstDir . sprintf('/%s/sites/all/modules/custom/*[_-]base', $this->webroot), + $this->dstDir . sprintf('/%s/sites/all/modules/custom/*[_-]core', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/*/modules/*[_-]base', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/*/modules/*[_-]core', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/*/modules/custom/*[_-]base', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/*/modules/custom/*[_-]core', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/custom/*/modules/*[_-]base', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/custom/*/modules/*[_-]core', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/custom/*/modules/custom/*[_-]base', $this->webroot), + $this->dstDir . sprintf('/%s/profiles/custom/*/modules/custom/*[_-]core', $this->webroot),And:
- return empty($path) ? NULL : str_replace(['_base', '_core'], '', basename($path)); + return empty($path) ? NULL : preg_replace('/[_-](base|core)$/', '', basename($path));Also applies to: 72-72
♻️ Duplicate comments (2)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (2)
99-99: Hyphen token now kebab-cased — resolves prior nitUsing Converter::kebab($v) for 'ys-base' is correct and fixes underscore leakage (e.g., my_site → my-site-base).
101-101: Consistent kebab-casing for 'ys-search'Good parity with the base mapping; my_site → my-site-search is handled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (19)
.vortex/installer/tests/Fixtures/install/_baseline/tests/behat/features/counter.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/css/sw_base.cssis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/js/sw_base.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/sw_base.moduleis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/templates/sw-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/templates/ys-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/css/sw_base.cssis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/js/sw_base.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/sw_base.moduleis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/templates/sw-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/templates/ys-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/web/modules/custom/sw_base/templates/-sw-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/tests/behat/features/counter.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/sw_base/templates/-sw-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/css/the_force_base.cssis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/js/the_force_base.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/templates/the-force-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/templates/ys-base-counter-block.html.twigis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/the_force_base.moduleis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (1)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (2)
.vortex/installer/src/Utils/Converter.php (1)
Converter(9-17).vortex/installer/src/Utils/File.php (1)
File(10-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (2)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (2)
109-111: Added hyphenated renames under modules/customRenaming 'ys-base' and 'ys-search' to kebab($v) variants looks correct and aligns with content replacements.
113-113: Class rename parity for SearchAdding 'YsSearch' → Pascal($v).'Search' keeps case-mapped renames consistent with 'YsBase'.
| File::replaceContentAsync([ | ||
| 'ys_base' => $v . '_base', | ||
| 'ys-base' => Converter::kebab($v) . '-base', | ||
| 'ys_search' => $v . '_search', | ||
| 'ys-search' => Converter::kebab($v) . '-search', | ||
| 'YsBase' => Converter::pascal($v) . 'Base', | ||
| 'YsSearch' => Converter::pascal($v) . 'Search', | ||
| 'YSBASE' => Converter::cobol($v), | ||
| 'YSSEARCH' => Converter::cobol($v), | ||
| ]); |
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.
💡 Verification agent
🧩 Analysis chain
Confirm Converter::kebab() availability and behavior
If kebab() is missing or differs from expectations (e.g., handling spaces/underscores), replacements could regress. Please verify the method exists and hyphenizes underscores and spaces.
🏁 Script executed:
#!/bin/bash
# Verify kebab() is defined and review its implementation.
rg -nC2 -P '\bfunction\s+kebab\s*\(' .vortex/installer/src | sed -n '1,120p'
rg -nC2 -P '\bConverter::kebab\s*\(' .vortex | sed -n '1,120p'Length of output: 2851
🏁 Script executed:
#!/bin/bash
# Locate the Converter class and its kebab() method across the repository
rg -nC3 'class\s+Converter\b' -g '*.php'
rg -nC5 'function\s+kebab\s*\(' -g '*.php'Length of output: 87
🏁 Script executed:
#!/bin/bash
# Locate the Converter class file and inspect its namespace/imports
find . -type f -name '*Converter.php'
# Show namespace and imports for ModulePrefix.php to resolve Converter reference
sed -n '1,50p' .vortex/installer/src/Prompts/Handlers/ModulePrefix.phpLength of output: 1155
🏁 Script executed:
#!/bin/bash
# Extract the kebab() method implementation from Converter.php
sed -n '1,200p' .vortex/installer/src/Utils/Converter.php | sed -n '/function kebab/,/}/p'Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for kebab method and count lines in Converter.php
rg -n 'kebab' -C2 .vortex/installer/src/Utils/Converter.php || echo 'No matches'
wc -l .vortex/installer/src/Utils/Converter.phpLength of output: 185
Implement or replace calls to missing Converter::kebab()
The Converter class at .vortex/installer/src/Utils/Converter.php does not define a kebab() method, so calls in ModulePrefix.php (e.g. lines 99, 101, 109, 111) will fail. Add a kebab() implementation or update these references to use a supported converter method.
🤖 Prompt for AI Agents
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php lines 97-106: the code
calls Converter::kebab() which doesn't exist in
.vortex/installer/src/Utils/Converter.php and will cause runtime errors; either
add a kebab(string $s): string method to Converter that normalizes the input
(trim, convert underscores/camel/Pascal to lower-case words separated by '-')
and returns kebab-case, or change these call sites to an existing Converter
method that produces kebab-case (e.g., a slug/normalize method) and pass the
same input; implement the new method in Converter.php and update its
docblock/tests, or replace the four calls in ModulePrefix.php to use the
existing supported converter function consistently.
Summary by CodeRabbit