uber-go / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
1.82k stars 104 forks source link

feat: add Regex() matcher #114

Closed merrett010 closed 8 months ago

merrett010 commented 8 months ago

Closes #113

Let me know if it's something you'd be interested in having - happy to make some tweaks if required

CLAassistant commented 8 months ago

CLA assistant check
All committers have signed the CLA.

favonia commented 8 months ago

Hi, I found this PR interesting---though I'm neutral about whether this feature should be added. If it's to be added, I wonder if we could make these two changes:

  1. Immediately compile the regular expression into a regexp.Regexp instead of compiling it during every check.
  2. Match []byte in addition to string. Maybe even io.RuneReader.

PS: I'm not an admin who can decide whether to merge this PR or not.

JacobOaks commented 8 months ago

Hi @merrett010 thanks for this PR. I'm not opposed to adding this feature if others aren't.

However, I agree with both of @favonia's suggestions. While in most cases I would imagine the matcher will be used a single time - it wouldn't hurt to be on the safe side and compile the regular expression once. Matching []byte would also be a nice touch.

merrett010 commented 8 months ago

Thanks both for your comments. I agree @favonia's suggestions are a nice addition. Have pushed up a new commit fyi @JacobOaks

favonia commented 8 months ago

@merrett010 I made three suggestions:

  1. Panic early when the the regular expression is invalid instead of silently ignore it.
  2. Use character literals instead of character codes.
  3. Print "matches ..." instead of "matching ...".

I still remain neutral about whether this feature should be included.

merrett010 commented 8 months ago

@favonia Thanks for reviewing, all seemed like good suggestions to me

merrett010 commented 8 months ago

Cheers @JacobOaks, have seen to those now