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

[WW-5077] DMI aware pattern #425

Merged
merged 3 commits into from
Jul 17, 2020
Merged

[WW-5077] DMI aware pattern #425

merged 3 commits into from
Jul 17, 2020

Conversation

lukaszlenart
Copy link
Member

Refs WW-5077

@coveralls
Copy link

coveralls commented Jul 11, 2020

Coverage Status

Coverage increased (+0.005%) to 47.102% when pulling 9b3a23e on WW-5077-dmi-aware into 89e02a4 on struts-2-5-x.

@@ -35,7 +37,11 @@
private static final Logger LOG = LogManager.getLogger(DefaultAcceptedPatternsChecker.class);

public static final String[] ACCEPTED_PATTERNS = {
"\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"
"\\w+((\\.\\w+)|(\\[\\d+])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"
Copy link
Member

Choose a reason for hiding this comment

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

Looks strange for me. Closing escaped [ brackets with not-escaped ] brackets?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was suggested by IDEA, restored the previous version :)

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry it was right.

And I spotted it's nice I think to refactor ((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))* part e.g. named EXTENSION_PATTERN or so because it's same in both.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but I would like to keep them as is, it's easier to understand what's going on. Also future changes to the patterns want affect each other.

@yasserzamani yasserzamani merged commit 4504e61 into struts-2-5-x Jul 17, 2020
@lukaszlenart lukaszlenart deleted the WW-5077-dmi-aware branch July 17, 2020 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants