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

Added ability to include/exclude paths via real regex #536

Merged
merged 7 commits into from Mar 7, 2020

Conversation

Armaxis
Copy link
Contributor

@Armaxis Armaxis commented Jan 8, 2020

SimpleRelocator uses plexus's SelectorUtils under the hood to perform string matching. This utility in fact supports "real" regex - as long as they are wrapped with %regex[]

SimpleRelocator didn't had the need to utilize this functionality yet, and provided include/exclude methods that supported "normalized patterns" - these are eventually parsed by SelectorUtils.matchAntPathPattern. These patterns allow using * for wildcard matches, but lack the power of regex.

In our project we needed a custom regex to match certain paths, hence we added methods includeRegex/excludeRegex. This might be useful API for somebody else as well.
See new unit test for example usage.

Note: this PR is built on top of #535 since it uses new signature of canRelocatePath. If both PRs are approved, #535 is to be merged first. If only this one approved - I can rework the code to not require the new signature of method. I just really didn't wanted to have two PRs that would break things if merged together.

@Armaxis Armaxis changed the title Added ability to include/exclude paths/classes via real Regex Added ability to include/exclude paths via real regex Jan 8, 2020
@johnrengelman johnrengelman added this to the 6.0 milestone Jan 14, 2020
…mance boost on large project (5:48 -> 2:02 min total duration).

- Changed signature of methods to accept String as path instead of RelocatePathContext/RelocateClassContext to prevent unnecessary allocations for builder objects
- Added extra string length checks to avoid string operations where possible
- Reordered checks in canRelocateClass so that expensive string replacement is the last operation
- (Main Optimization) Avoided string concatenation (which led to creating StringBuilder instances and expensive array copy operations) by adding check for first character being '/': if so - perform a startsWith check with an offset.

- Removed an incorrect length check
- Moved context creation outside the loop
…mance boost on large project (5:48 -> 2:02 min total duration).

- Changed signature of methods to accept String as path instead of RelocatePathContext/RelocateClassContext to prevent unnecessary allocations for builder objects
- Added extra string length checks to avoid string operations where possible
- Reordered checks in canRelocateClass so that expensive string replacement is the last operation
- (Main Optimization) Avoided string concatenation (which led to creating StringBuilder instances and expensive array copy operations) by adding check for first character being '/': if so - perform a startsWith check with an offset.
Moved context creation outside the loop

(cherry picked from commit 8476da7)
(cherry picked from commit 076df6211d01a1a9904a741566d645e9ab36c118)
@Armaxis
Copy link
Contributor Author

Armaxis commented Jan 22, 2020

Resolved merge conflict

@johnrengelman
Copy link
Owner

@Armaxis - couldn’t we just document that if you want to use a real regex that you can use the existing include/exclude but wrap it with the %regex[] syntax?

It looks like that’s all this is doing, right?

@Armaxis
Copy link
Contributor Author

Armaxis commented Feb 2, 2020

@johnrengelman that won't work because existing include/exclude call normalizePatterns on the patterns which replaces all . with / and removes last /* if there's any. This effectively breaks a regular regex.

I can tweak normalizePatterns to skip an entry if it begins with %regex and drop having to add two more methods. This might be a cleaner way to do things, dunno why I didn't think of this before!

@Armaxis
Copy link
Contributor Author

Armaxis commented Feb 3, 2020

Updated implementation to filter out Regex in normalizePatterns. Added section in documentation on usage and updated existing one to mention that it uses Ant Path Matcher

@johnrengelman johnrengelman merged commit 2468c0b into johnrengelman:master Mar 7, 2020
@Armaxis Armaxis deleted the armaxis/regex-support branch May 29, 2020 16:15
@Armaxis
Copy link
Contributor Author

Armaxis commented Jun 15, 2020

@johnrengelman Hi! Is this PR in the 6.0.0? It's not in the release notes.

Congrats on the 6.0.0 release by the way!

@johnrengelman
Copy link
Owner

Yes. It is in 6.0.0

@johnrengelman
Copy link
Owner

I’ll update the release notes later tonight. Thanks for catching that.

@Armaxis
Copy link
Contributor Author

Armaxis commented Jun 15, 2020

@johnrengelman awesome, going to upgrade to 6.0.0 soon then! Thanks again for all the effort put into this plugin!

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.

None yet

2 participants