Add support for capturing group in tag inclusion. #21
Conversation
|
Sounds like a useful feature, will review it! Even though the tests pass, one should not rely on them too much when working on extractors, as there are currently no specific tests for covering the extractors. Of course you are welcome to write tests for this extractor, should you feel inclined to. 😉 |
There was a problem hiding this comment.
This change does not seem relevant to the requested feature.
| match = self.include.match(tag) | ||
| if match: | ||
| # Check if there are capturing groups | ||
| if len(match.groups()) > 0: |
There was a problem hiding this comment.
if match.groups(): would be a simpler way to express the same condition.
| if match: | ||
| # Check if there are capturing groups | ||
| if len(match.groups()) > 0: | ||
| tag = ''.join(match.groups()) |
There was a problem hiding this comment.
I'm afraid this could be problematic if the regular expression happens to have multiple or nested groups. It looks like the user would need a way to tell the extractor which group to use (group index, or group name, for regular expressions in the form of (?P<name>...)).
This PR allows the use of capturing group when using regular expression to determine item inclusion by tags.
If
tag_include_recontains capturing group, the tags will be the concatenation of captured substrings.For example:
tag_include_reto "-kerko-(.*)". Enablekerko.facets.tag.The changes pass all tests. Works as expected.