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

Simplify implementation of String#mocha_inspect. #223

Merged
merged 1 commit into from Oct 24, 2016

Conversation

floehopper
Copy link
Member

This is an alternative attempt at fixing the problem identified
and addressed in #215 regarding the "pretty printing" of single- and
double-quotes.

In this attempt, I asked myself why we ever needed to do the gsub
replacement of escaped double-quote characters with single-quote characters.
I couldn't come up with a reason other than to make the tests more readable.

Thus in this commit I've removed the custom implementation of
String#mocha_inspect so it falls back to the default String#inspect
implementation. As far as I can see the error messages are still sensibly
formatted and by using the %{} string literal construction, I think the
tests are still perfectly readable.

I hope that this is a simpler and more robust way to solve the problems
that @neonichu was seeing.

@neonichu
Copy link

Sorry, I overlooked this, but it looks good to me 👍

@chrisroos
Copy link
Member

I've just rebased this on master and force pushed. I'm planning to merge if/when the tests pass.

@chrisroos chrisroos self-assigned this Oct 18, 2016
This is an alternative attempt at fixing the problem identified
and addressed in #215 regarding the "pretty printing" of single- and
double-quotes.

In this attempt, I asked myself why we *ever* needed to do the `gsub`
replacement of escaped double-quote characters with single-quote characters.
I couldn't come up with a reason other than to make the tests more readable.

Thus in this commit I've removed the custom implementation of
`String#mocha_inspect` so it falls back to the default `String#inspect`
implementation. As far as I can see the error messages are still sensibly
formatted and by using the `%{}` string literal construction, I think the
tests are still perfectly readable.

I hope that this is a simpler and more robust way to solve the problems
that @neonichu was seeing.
@chrisroos
Copy link
Member

I've rebased and force-pushed again. I'll merge once the tests pass.

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

Successfully merging this pull request may close these issues.

None yet

3 participants