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

Fix for issue 2182 on jruby-1_7 : Struct#inspect with utf8 encode string member #2321

Merged
merged 1 commit into from Feb 2, 2015

Conversation

Projects
None yet
4 participants
@k77ch7
Copy link
Contributor

k77ch7 commented Dec 15, 2014

This commit fixes issue #2182 on jruby-1_7 branch. Inspect method of Struct with utf8 string member should return a utf8 string.

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Dec 15, 2014

Converting to a Java charset feels really wrong to me. For 1.9 support I would almost prefer working directly with a RubyString and using cat19 over that. The main problem is not all supported Encodings in Ruby have an equaivalent Java Charset (although this is pretty rare). I won't close this atm and perhaps @headius or @lopex may have a better solution. I feel like this entire method needs to be rewritten and split out from the 1.8 version on it atm.

@lopex

This comment has been minimized.

Copy link
Member

lopex commented Dec 15, 2014

MRI uses rb_str_append for members and rb_str_cat2 for fixed parts of the result

@k77ch7 k77ch7 force-pushed the k77ch7:GH-2182_struct_has_ascii_encoding branch from 4d99577 to 1a9ef29 Dec 16, 2014

@k77ch7

This comment has been minimized.

Copy link
Contributor Author

k77ch7 commented Dec 16, 2014

@enebo @lopex Thank you for your advice. I was refactoring the code. And I was adding the test case of a Struct with utf8 symbol. Please review 1a9ef29.

@headius

This comment has been minimized.

Copy link
Member

headius commented Jan 12, 2015

@k77ch7 Since your second commit corrects flaws (duplication of cat19 logic) in the first one, can you force push this all as a single commit please?

Thanks for your help!

@k77ch7 k77ch7 force-pushed the k77ch7:GH-2182_struct_has_ascii_encoding branch from 1a9ef29 to 4bf84a9 Jan 14, 2015

@k77ch7 k77ch7 force-pushed the k77ch7:GH-2182_struct_has_ascii_encoding branch from 4bf84a9 to 5879d5b Jan 14, 2015

@k77ch7

This comment has been minimized.

Copy link
Contributor Author

k77ch7 commented Jan 14, 2015

@headius Thanks for reviewing. I just pushed.

enebo added a commit that referenced this pull request Feb 2, 2015

Merge pull request #2321 from k77ch7/GH-2182_struct_has_ascii_encoding
Fix for issue 2182 on jruby-1_7 : Struct#inspect with utf8 encode string member

@enebo enebo merged commit 68bf136 into jruby:jruby-1_7 Feb 2, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@enebo enebo added this to the JRuby 1.7.20 milestone Feb 2, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.