Implement #30 #72
Implement #30 #72
Conversation
The work in Method::Signatures::Parser::split_parameter() has become the Method::Sigantures::Parameter class. This does not work or compile right now, but I have to go. Remaning work... * Change all the places which use the %sig hash to use the object. * Actually make sure the new class compiles... * Using functions from MS::Parser * Test the new class directly (test code coverage is very good)
Had to switch almost every attribute over to read-write because they're setup during the BUILD step. Unfortunately Moose enforces ro during the BUILD step. Could change it all to be in BUILDARGS but that will complicate things and lose the advantage of having defaults and types during the extensive BUILD stage. Tests still failing extensively. Time to start writing unit tests.
Since it's a completely different grammar, yadayada can't go through the rest of the parsing and doesn't need to. Return immediately.
MS::Parameter loads it. The tests are moot.
|
@barefootcoder Could you review and merge this? It touches a lot of code and I'm afraid of it going stale. |
|
I will absolutely try to hit this this weekend. The family is out of town, so I should have a few free moments. :-) |
|
Well, tests fail on 5.8, 5.10, and 5.12. The error reporter sub apparently isn't getting all the way out of MS. Not sure why that would be diferent on <= 5.12 but okay on >= 5.14, but I'll look into it a bit deeper. |
|
Okay, I can't see any potential problems with the code after thorough review. Let me just figure out if I can fix this bug with the error reporting then we should be good to merge. |
apparently the call to new() for a Mouse object involves an eval in earlier Perl versions this exposed a bug which I'm rather surprised wasn't discovered before which is that the regex we build to determine what to skip demands something _inside_ one of the packages we specify we check both the method and the package against this when the package is exactly equal to the package we're checking against, that check fails happily, the method is still inside that package, so _that_ check succeeds except in this case the method was `(eval)`, so that failed too the solution was to allow exact package matches to succeed as well also made the regex a bit nicer via /x while I was in there also removed useless specification of Method::Signatures::Parser, since that's automatically caught by checking for Method::Signatures
|
Read the commit message for full gory details, but the good news is, it's fixed. I'll allow a bit of time for @schwern to comment if he so desires, then merge this perhaps tomorrow night. |
Implement #30, the parsed parameter is now an object, Method::Signatures::Parameter.
|
@barefootcoder Thanks for fixing it on older perls. |
Yeah, weird, right?
No worries. Glad to have this one put to bed. |
This branch implements #30, turning the parsed parameter into an object, Method::Signatures::Parameter.
This isn't ready to be merged, MS::Parameter needs unit testing. Just putting it in here to begin the review process and see if anyone else wants to work on it.
It also slows down Method::Signatures load time and signature parsing by about 10%. I'm not terribly worried about that. This was a rote refactoring and that's just what you lose switching from hash lookups to method calls. Once the parameter parsing makes more sense I'm sure we can optimize it back and then some.