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

Implement String#casecmp? and Symbol#casecmp? #4470

Merged
merged 2 commits into from
Feb 13, 2017

Conversation

kcdragon
Copy link
Contributor

@kcdragon kcdragon commented Feb 2, 2017

  • Adds implementation of casecmp? based on downcasing the string and comparing
  • Adds unit tests for casecmp? from MRI

There is a test failing in each of the tests (the last assertion in each method) I brought over. This is because the MRI implementation (ruby/ruby@ad619e0) of casecmp? relies on passing the :fold option to downcase however that option has not been added yet in JRuby. It is referenced in #4293 though.

I decided to keep the failing tests in for completeness since they will need to be fixed anyway for 2.4 support. I figured that this PR would be fine without those specific cases supported but maybe it would be better to wait until :fold is implemented.

See #4293

* Adds implementation of `casecmp?` based on downcasing the string and comparing
* Adds unit tests for `casecmp?` from MRI

See jruby#4293
@kares
Copy link
Member

kares commented Feb 2, 2017

excellent, any reason why the encoding compatible check is not being implemented (like in MRI) ?

@kares kares added the ruby 2.4 label Feb 2, 2017
@kcdragon
Copy link
Contributor Author

kcdragon commented Feb 2, 2017

excellent, any reason why the encoding compatible check is not being implemented (like in MRI) ?

@kares

There's no reason. I can add that. Are you referring to something like this being done?

Encoding enc = StringSupport.areCompatible(this, otherStr);
if (enc == null) return runtime.getNil();

https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyString.java#L1572-L1573

@kares
Copy link
Member

kares commented Feb 3, 2017

yy

@kcdragon
Copy link
Contributor Author

@kares I added the encoding compatibility check.

@headius
Copy link
Member

headius commented Feb 13, 2017

Looks ok to me!

@headius headius merged commit edbd1d5 into jruby:ruby-2.4 Feb 13, 2017
@kares kares added this to the JRuby 9.2.0.0 milestone Feb 14, 2017
headius added a commit to headius/jruby that referenced this pull request Jun 6, 2023
The original logic ended up duplicating the memory as managed,
and seemed to cause the underlying pointer to be collected too
soon. This triggered EFAULT errors in io-console when it duped a
Termios struct to pass to tcsetattr, but the memory sometimes was
already reclaimed.

Underlying cause of errors raised in jruby#4470
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants