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

stringify frames per as_string (#19) #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bpj
Copy link

@bpj bpj commented Jul 9, 2018

See #19.

I had to give overload an anonymous sub which calls as_string on the object rather than giving it \&as_string or 'as_string' or arguments wouldn't be shown properly in the stringification. Probably because of generated accessors which don't exist yet?

I added a very basic test checking that the stringification is identical to the return value of $frame->as_string. It caught the above problem so apparently not too simple!

@bpj
Copy link
Author

bpj commented Jul 9, 2018

I have no idea why tests fail. They succeed locally.

@autarch
Copy link
Member

autarch commented Jul 9, 2018

I think you used an older version of Perl::Tidy locally to tidy your code. You need to use the latest version from CPAN.

@bpj
Copy link
Author

bpj commented Jul 9, 2018

Yes perltidy was the problem, but the real problem was that perlbrew was using the wrong perl. I must have said perlbrew switch instead of perlbrew use at some point.

Copy link
Member

@autarch autarch left a comment

Choose a reason for hiding this comment

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

There's no need to check in the generated files. This will be done automatically after each release.

@@ -3,6 +3,8 @@ package Devel::StackTrace::Frame;
use strict;
use warnings;

use DDP;
Copy link
Member

Choose a reason for hiding this comment

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

This should not be in here.

Copy link
Author

Choose a reason for hiding this comment

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

Quick replies while I'm on holiday. I'll fix the problems next week when I get back.

I wasn't aware that I had committed generated files. Sorry!

Also apologies for forgetting to weed out (all) my debugging stuff.

@@ -28,6 +30,11 @@ BEGIN {
}
}

# XXX -- Arguments are garbled if giving overload \&as_string or 'as_string' directly.
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean?

Copy link
Author

Choose a reason for hiding this comment

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

It means that the test for the stringification being equal to the return value of the as_string method being equal fails if the value of the '""' key of use overload is \&as_string or 'as_string': part of the string appears as a ? in the stringification. I guess it's because some of the generated subs don't exist yet when use overload is executed. The form wrapping the method call in an anonymous sub does not have this problem and the test succeeds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants