Project

General

Profile

Actions

Bug #5536

closed

String#start_with? and end_with? ignore arguments convertible to a String [PATCH]

Added by Eregon (Benoit Daloze) over 12 years ago. Updated about 11 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 2.0.0dev (2011-11-01 trunk 33605) [x86_64-linux]
Backport:
[ruby-core:40623]

Description

Hi,

Currently, String#start_with? and String#end_with? ignore arguments not convertible to String.

I believe it should instead raise an error,
as it may lead to false expectations from the user and
it is inconsistent with the rest of the API.

For example, if I try to use start_with? with a RegExp (which would be a nice feature BTW):
"str".start_with? /s/ # => false
I believe it should be:
"str".start_with? /s/ # => TypeError: can't convert Regexp into String

If you prefer the current behavior, could you explain me why?

P.S.: There is no test for String#{start,end}_with? in test/, should I add one or is it enough to change RubySpec (which I'll do when this gets accepted)?


Files


Related issues 1 (0 open1 closed)

Is duplicate of Ruby master - Feature #3388: regexp support for start_with? and end_with?Feedbacknaruse (Yui NARUSE)Actions

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

  • Status changed from Open to Assigned
  • Assignee set to matz (Yukihiro Matsumoto)

(11/11/01 21:43), Benoit Daloze wrote:

Currently, String#start_with? and String#end_with? ignore arguments not convertible to String.

I believe it should instead raise an error,
as it may lead to false expectations from the user and
it is inconsistent with the rest of the API.

Indeed.

If you prefer the current behavior, could you explain me why?

It hasn't changed from the origin, and I'm curious too.

P.S.: There is no test for String#{start,end}_with? in test/, should I add one or is it enough to change RubySpec (which I'll do when this gets accepted)?

See test/ruby/test_m17n_comb.rb.

Updated by Eregon (Benoit Daloze) over 12 years ago

Issue #5536 has been updated by Nobuyoshi Nakada.

(11/11/01 21:43), Benoit Daloze wrote:

P.S.: There is no test for String#{start,end}_with? in test/, should I add one or is it enough to change RubySpec (which I'll do when this gets accepted)?

See test/ruby/test_m17n_comb.rb.

Sorry, I missed it, I only looked at test/ruby/test_string.rb.
But I'm not sure the error tests belong there, as test_m17n_comb.rb mainly focus on Encoding interactions.

Should I add in test_string.rb or test_m17n_comb.rb?

Here is the diff for test_m17n_comb.rb:

diff --git a/test/ruby/test_m17n_comb.rb b/test/ruby/test_m17n_comb.rb
index 79016af..9c6d5f3 100644
--- a/test/ruby/test_m17n_comb.rb
+++ b/test/ruby/test_m17n_comb.rb
@@ -1551,6 +1551,7 @@ class TestM17NComb < Test::Unit::TestCase
end
assert_equal(false, enccall(s1, :end_with?, s2), desc)
}

  • assert_raise(TypeError) { "str".end_with? :not_convertible_to_string }
    end

def test_str_start_with?
@@ -1572,6 +1573,7 @@ class TestM17NComb < Test::Unit::TestCase
end
assert_equal(false, enccall(s1, :start_with?, s2), desc)
}

  • assert_raise(TypeError) { "str".start_with? :not_convertible_to_string }
    end

def test_str_ord

Updated by nobu (Nobuyoshi Nakada) over 12 years ago

Should I add in test_string.rb or test_m17n_comb.rb?

test_string.rb, I think.

Actions #6

Updated by nobu (Nobuyoshi Nakada) almost 12 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r35213.
Benoit, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • string.c (rb_str_start_with, rb_str_end_with): raise an error if
    an argument is not convertible to a String.
    [ruby-core:40623][Bug #5536]

Updated by prijutme4ty (Ilya Vorontsov) about 11 years ago

=begin
Can you please explain, why not just make (({"str".start_with(/s/)})) behave as (({"str".match(/^s/)})) ?
=end

Updated by Eregon (Benoit Daloze) about 11 years ago

prijutme4ty (Ilya Vorontsov) wrote:

=begin
Can you please explain, why not just make (({"str".start_with(/s/)})) behave as (({"str".match(/^s/)})) ?
=end

See #3388 for this feature (this issue is closed).
It is the approach I initially took, but building and compiling a Regexp is expensive, and so it should be cached or another way be used. For #start_with?, there is a C-level API for matching only at start, but there is no corresponding API for #end_with?.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0