Skip to content
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

Update RegExUtil.java #2348

Conversation

LouisdeLooze
Copy link
Contributor

@LouisdeLooze LouisdeLooze commented Oct 14, 2024

Updating theme regex to support dashes.

Description (*)
According to the Magento devdocs:

The folder name conventionally matches naming used in the theme's code: any alphanumeric set of characters, as the vendor sees fit, is acceptable. This convention is merely a recommendation, so nothing prevents naming this directory in another way.

The current regex limits the themes to:

match group is_participating start end content
1 0 yes 0 21 frontend/Vendor/theme
2 0 yes 44 74 frontend/Vendor/custom9_theme9
3 0 yes 106 136 frontend/Vendor/custom9_theme9
4 0 yes 137 159 frontend/Vendor/custom
5 0 yes 196 225 frontend/Vendor/custom9Theme9

The adapted regex allows the themes to be:

match group is_participating start end content
1 0 yes 0 21 frontend/Vendor/theme
2 0 yes 22 43 frontend/Vendor/Theme
3 0 yes 44 74 frontend/Vendor/custom9_theme9
4 0 yes 75 105 frontend/Vendor/Custom9_theme9
5 0 yes 106 136 frontend/Vendor/custom9_theme9
6 0 yes 137 165 frontend/Vendor/custom-theme
7 0 yes 166 195 frontend/Vendor/Custom9Theme9
8 0 yes 196 225 frontend/Vendor/custom9Theme9
9 0 yes 226 247 frontend/vendor/theme
10 0 yes 248 269 frontend/vendor/Theme
11 0 yes 270 300 frontend/vendor/custom9_theme9
12 0 yes 301 331 frontend/vendor/Custom9_theme9
13 0 yes 332 362 frontend/vendor/custom9_theme9
14 0 yes 363 391 frontend/vendor/custom-theme
15 0 yes 392 421 frontend/vendor/Custom9Theme9
16 0 yes 422 451 frontend/vendor/custom9Theme9

Thanks @sdouma for the regex!

Fixed Issues (if relevant)

  1. Fixes Custom theme couldn't be detected #1517

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with integration/functional tests (if applicable)
  • All automated tests passed successfully (all builds are green)

Updating theme regex to support dashes.
@LouisdeLooze LouisdeLooze reopened this Oct 14, 2024
@LouisdeLooze LouisdeLooze marked this pull request as ready for review October 14, 2024 07:28
@VitaliyBoyko VitaliyBoyko self-assigned this Oct 15, 2024
@VitaliyBoyko VitaliyBoyko self-requested a review October 15, 2024 11:22
@VitaliyBoyko VitaliyBoyko changed the base branch from 5.4.0-develop to 5.3.1-develop October 15, 2024 16:55
@VitaliyBoyko VitaliyBoyko changed the base branch from 5.3.1-develop to 5.4.0-develop October 15, 2024 16:55
Copy link
Contributor

@VitaliyBoyko VitaliyBoyko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LouisdeLooze thank you for your contribution.
Could you please check tests?

@LouisdeLooze
Copy link
Contributor Author

Hi @VitaliyBoyko
Just tried running the test again locally from both clean branches,

  • 5.3.0 -> no issues
  • 5.4.0 -> all tests fail because of aNullPointerException
    Tried to revert back the build.gradle and all test pass again. Seems that o be a deeper issue...

@VitaliyBoyko
Copy link
Contributor

@LouisdeLooze thank you. Will check it. Could you please create a backport PR to the 5.3.1-develop branch?

@LouisdeLooze LouisdeLooze changed the base branch from 5.4.0-develop to 5.3.1-develop October 16, 2024 08:21
@LouisdeLooze LouisdeLooze reopened this Oct 16, 2024
@LouisdeLooze LouisdeLooze changed the base branch from 5.3.1-develop to 5.4.0-develop October 16, 2024 08:23
@VitaliyBoyko VitaliyBoyko merged commit 3a8b981 into magento:5.4.0-develop Oct 20, 2024
2 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom theme couldn't be detected
2 participants