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.

Sign up for GitHub

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

Jump to bottom

RString default encoding is ASCII-8Bit #67

Open
mjc-gh opened this issue Feb 24, 2017 · 10 comments · May be fixed by #101
Open

RString default encoding is ASCII-8Bit #67

mjc-gh opened this issue Feb 24, 2017 · 10 comments · May be fixed by #101

Comments

@mjc-gh
Copy link
Contributor

mjc-gh commented Feb 24, 2017

I made a small example to show the "issue". It is available here: https://github.com/mikeycgto/ruru-rstring-encoding#example.

While this isn't incorrect it does feel a little odd since in modern Ruby and Rust both default to UTF-8 encoding.

Perhaps ruby-sys should expose rb_utf8_str_new as well? The RString struct could then maybe have a new_from_utf8 function which will ultimately call this Ruby C function. I can make the necessary PRs but I wanted some feedback first.
mjc-gh added a commit to mjc-gh/ruby-sys that referenced this issue Feb 27, 2017
@mjc-gh
Add string::rb_utf8_str_new function
5fa1332 
This function will return a new Ruby string with UTF8 encoding. This
commit is for the issue described in d-unsed/ruru#67.
mjc-gh added a commit to mjc-gh/ruby-sys that referenced this issue Feb 27, 2017
@mjc-gh
Add string::rb_utf8_str_new function
58fc970 
This function will return a new Ruby string with UTF8 encoding. This
commit is for the issue described in d-unsed/ruru#67.
mjc-gh added a commit to mjc-gh/ruru that referenced this issue Feb 27, 2017
@mjc-gh
Add RString#new_utf8 and string::new_utf8 (d-unsed#67)
351d6e7 
These functions will return a new Ruby string with UTF8 encoding.
mjc-gh added a commit to mjc-gh/ruru that referenced this issue Feb 27, 2017
@mjc-gh
Add RString#new_utf8 and string::new_utf8
6a98907 
These functions will return a new Ruby string with UTF8 encoding. This
is for the issue described in d-unsed#67
@mjc-gh
Copy link
Contributor Author

mjc-gh commented Feb 27, 2017

Created two commits to deal with this issue:

@mjc-gh mjc-gh mentioned this issue Feb 27, 2017
Open
This was referenced Apr 29, 2017
Merged
Merged
@d-unsed
Copy link
Owner

d-unsed commented May 24, 2017

#71 will be merged after we release new version of ruby-sys

@mjc-gh
Copy link
Contributor Author

mjc-gh commented Jul 13, 2017

Any update on releasing a new version of ruby-sys with steveklabnik/ruby-sys/pull/23 merged? Thanks!

@steveklabnik
Copy link
Collaborator

I've published ruby-sys 0.3.0

@danielpclark
Copy link
Contributor

On encoding I'd be interested if we could implement the same encoding support that Ruby has. I'm playing around with some ideas. The main issue to test against is this test https://github.com/ruby/spec/blob/29b472cf57946ce271533fc6e03716908a313aaf/core/file/basename_spec.rb#L162-L166 which simply tests the same encoding goes in and out.

mjc-gh added a commit to mjc-gh/ruru that referenced this issue Dec 18, 2017
@mjc-gh
Add RString#new_utf8 and string::new_utf8
e3639c9 
These functions will return a new Ruby string with UTF8 encoding. This
is for the issue described in d-unsed#67
@turboladen
Copy link
Contributor

I suppose this could be closed per #71, yeah?

@d-unsed
Copy link
Owner

d-unsed commented Dec 20, 2017

I have one more question regarding encodings. Since Ruby by default creates a string with UTF-8 encoding ('str'.encoding => #<Encoding:UTF-8>), what do you think about changing the default behavior on the ruru side?

I mean the following steps:

  1. Create a new method RString::new_ascii which returns ascii-encoded string (behaves the same as current RString::new using rb_str_new)

  2. Change RString::new to return a UTF8-encoded string (behaves the same as current RString::new_utf8 using rb_utf8_str_new) and remove RString::new_utf8.

Then ruru and ruby will have the same default behavior. This can be implemented in 0.10.0

@danielpclark
Copy link
Contributor

and remove RString::new_utf8

I think changing the RString::new method to expected behavior is a good idea. But I think you should keep the RString::new_utf8 method around as it will help people who want to be more explicit with their code. Anyone currently using RString::new_utf8 will be better off if it stays.

@mjc-gh
Copy link
Contributor Author

mjc-gh commented Dec 20, 2017

I agree with making the default for RString::new be a UTF8 string since that's how Ruby behaves.

I think keeping RString::new_utf8 around is good idea as well. Perhaps we could also add RString::new_ascii which would returns an a ASCII-8bit encode string.

@turboladen
Copy link
Contributor

I agree with making the default for RString::new be a UTF8 string since that's how Ruby behaves.

This totally makes sense to me.

@danielpclark danielpclark mentioned this issue Aug 2, 2018
Merged
@Palladinium Palladinium linked a pull request Jun 6, 2019 that will close this issue
Open
xmas7 pushed a commit to RubyOnWorld/ruby-sys that referenced this issue Sep 28, 2022
@jacksonmiller0900
Add string::rb_utf8_str_new function
96ee714 
This function will return a new Ruby string with UTF8 encoding. This
commit is for the issue described in d-unsed/ruru#67.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

RString utf8 fixes Palladinium/ruru
5 participants
@steveklabnik @mjc-gh @turboladen @danielpclark @d-unsed