-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Unify be_identical_string/be_diffed_as in spec/string_matcher.rb #176
Conversation
# check for string encoding. | ||
def expected_encoding? | ||
if defined?(@expect_same_encoding) && @expect_same_encoding | ||
actual.encoding == expected.encoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this blow up on 1.8.7?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really not sure how to get the best semantics from the matchers and also the least hacky implementation of the matcher. I'm not married to this implementation, but I liked the semantics of be_identical_string(str).with_same_encoding
better than adding a new be_identical_encoded_string
that is called from be_identical_string
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just missed the encoding check above
Changed meaning of be_identical_string not to include an encoding comparison. Encoding comparison is now only turned on by chaining 'with_same_encoding'. This leaves open the possibility of chaining 'with_encoding(enc)', but there's currently not need for that.
8b9d490
to
92c3e0e
Compare
forgot to require "rspec/matchers" at the top of the file. amended and force pushed. |
LGTM |
Unify be_identical_string/be_diffed_as in spec/string_matcher.rb
Thanks @bf4 :) |
Wow, that was quite a weekend. Thanks @JonRowe @myronmarston |
Changed meaning of be_identical_string not to include an encoding comparison.
Encoding comparison is now only turned on by chaining
'with_same_encoding'.
This leaves open the possibility of chaining 'with_encoding(enc)', but there's
currently not need for that.