Re: [PATCH] Iconv#iconv(str, start, length) doesn't really convert str[start, length]

Hi,

I Agree. That patch should have been created for 1.9 or later version
(if the core team
decided to have this change.)

I was going to create a documentation patch for ruby_1_8, and another patch for
trunk with incompatible changes, but the latest revision 16809 just
shows up today.
It doesn't handle default values and upper bound well:

irb(main):002:0> c=Iconv.open 'ascii','ascii'
=> #<Iconv:0x282c74>
irb(main):015:0> c.iconv 'abc'
=> ""
irb(main):022:0> c.iconv 'abcde', 2, 5
=> "cde\000\004"
irb(main):023:0> c.iconv 'abcde', 2, 1
=> "c"

The last one shows that length wasn't treated as an index.

So I think I'll not submit the documentation patch for now.

Thanks,
Vincent

On Wed, Jun 4, 2008 at 3:04 AM, Vladimir Sizikov <[e-mail]> wrote:
> Hi,
>
> So, this is clearly a backwards-incompatible change, since all Ruby
> versions up to the revision with the patch, treat second argument as
> an index, that's how Iconv#iconv was  always working (despite the
> documentation stating otherwise).
>
> In fact, we have RubySpec tests for this behavior, since that was
> de-facto behavior.
>
> I'm not really saying that the change is bad/wrong, but I"m saying
> that every incompatible change *is* incompatible change and somehow
> needs to be better documented and with tests enforcing that behavior.
>
> Also, what would happen with 1.8.6/1.8.7 lines, will they get the
> update, or will they continue to exhibit current behavior? You see,
> any incompatible changes quickly lead to  very messy situations. If
> somebody is relying on older behavior, they will get *silent* , hidden
> errors when upgrading.
>
> Maybe, only docs need to be changed to match the current de-facto
> behavor, rather than vice versa?
>
> Thanks,
>  --Vladimir
>
> On Wed, Jun 4, 2008 at 8:39 AM, Vincent <[e-mail]> wrote:
>> Hi,
>>
>> Here is a patch that handles both argument check and default value.
>> ChangeLog:
>>
>> * ext/iconv/iconv.c : Iconv#iconv's length parameter default value changed
>> to nil, which means to the end of the string. Any negative length, including -1,
>> makes iconv return "".
>> * iconv_convert : treats length as conversion length rather than an end index
>> to match String#[fixnum, fixnum].
>>
>> Thanks,
>> Vincent
>>
>
>