Skip to content

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

Merged
johnrengelman merged 7 commits intoGradleUp:masterfrom
Armaxis:armaxis/regex-support
Mar 7, 2020
Merged

Added ability to include/exclude paths via real regex#536
johnrengelman merged 7 commits intoGradleUp:masterfrom
Armaxis:armaxis/regex-support

Conversation

@Armaxis
Copy link
Copy Markdown
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
@Armaxis Armaxis force-pushed the armaxis/regex-support branch from 700b4ef to 6b97759 Compare January 8, 2020 20:43
@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 Armaxis force-pushed the armaxis/regex-support branch from 6b97759 to 2791ec6 Compare January 22, 2020 00:03
@Armaxis
Copy link
Copy Markdown
Contributor Author

Armaxis commented Jan 22, 2020

Resolved merge conflict

@johnrengelman
Copy link
Copy Markdown
Collaborator

@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
Copy Markdown
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 Armaxis force-pushed the armaxis/regex-support branch from 0c0810f to 7a94fca Compare February 3, 2020 15:59
@Armaxis
Copy link
Copy Markdown
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 GradleUp:master Mar 7, 2020
@Armaxis Armaxis deleted the armaxis/regex-support branch May 29, 2020 16:15
@Armaxis
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

Yes. It is in 6.0.0

@johnrengelman
Copy link
Copy Markdown
Collaborator

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

@Armaxis
Copy link
Copy Markdown
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.

2 participants