Code review comment for lp:~mbp/subunit/408821-filter-re

A few thoughts. Broadly this is fine and I like the feature.

Rather than document in README, perhaps extending the help (module
docstring) of subunit-filter would be more accessible and provide a home
for other such one-command things. OTOH we probably need a manual at
some point anyway, so up to you.

Rather than --[anti]-match, I'd rather something like:
 --with RE
 --without RE

Related to that, I wonder if without is really needed - the (?!...) can
embed negative patterns. I'm fine with it, just speculating.

In the internals though, I think it would make sense to have a single
re:
patterns = []
for pattern in options.with_patterns:
  patterns.append(pattern)
for pattern in options.without_patterns:
  patterns.append("^(?!%s)$" % pattern)
rule = "|".join(patterns)

The interface is a little awkward - you use a functional style, but it
doesn't fit all that well. For instance - "str(None) -> 'None'". So,
your code will incorrectly match on a pattern like 'one'.than

I think I'd prefer it if, in the Filter result you did:
should_filter = self._check_predicates(test, reason|error)

and that function (not a lambda) does
if not callable(self._filter_predicate):
    return False
else:
    if info is None:
        info = ''
    else:
        info = str(info)
    return self._filter_predicate(test, info)

And that way folk writing predicates can be sure they are dealing with
strings - we centralise that knowledge.

Lastly, and this can come later if you like, I'd really like the ability
to specify multiple patterns, which optparse can do quite easily with
store="append".

-Rob