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

Extracting separate ArgumentMatcher ifc with no FailureMessage #114

Merged
merged 4 commits into from
Dec 9, 2020

Conversation

yhrn
Copy link

@yhrn yhrn commented Dec 7, 2020

Since FailureMessage() is only used for invocation count matching this will significantly simplify writing custom argument matchers - one less method to implement, no need for mutable state and synchronization.

@yhrn yhrn changed the title Extracting separate ArgumentMatcher with no FailureMessage Extracting separate ArgumentMatcher ifc with no FailureMessage Dec 7, 2020
@petergtz
Copy link
Owner

petergtz commented Dec 7, 2020

I think that's a great idea! I think when I introduced the matcher concept it wasn't clear yet, that the 2 use cases "argument match" and "invocation count" matcher are completely distinct. With that knowledge, we should really separate them.

A couple comments. Will add them in the diff.

Copy link
Owner

@petergtz petergtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like matcher_factories.go is not changed to use ArgumentMatcher. So that needs adjustment. BTW, instead of writing this file manually, there is internal/generate_matchers/matcher_generation.go.

As the comment there implies, you can run

go generate github.com/petergtz/pegomock/internal/generate_matchers

to generate all the default matchers. You should probably add your XThat functions there. Sorry, overlooked this in your previous PR, so I'd like to fill this back here, if that's okay with you.

dsl.go Outdated Show resolved Hide resolved
dsl.go Outdated Show resolved Hide resolved
@petergtz petergtz merged commit 8753b60 into petergtz:develop Dec 9, 2020
@petergtz
Copy link
Owner

petergtz commented Dec 9, 2020

Nice! Thanks again, @yhrn.

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