std.string.chomp error
The documentation says
/***
* Returns s[] sans trailing delimiter[], if any.
* If delimiter[] is null, removes trailing CR, LF, or CRLF, if any.
*/
To adhere to the documentation, chomp must be changed from:
C[] chomp(C, C1)(C[] s, in C1[] delimiter)
{
if (endsWith(s, delimiter))
{
return s[0 .. $ - delimiter.length];
}
return s;
}
to:
C[] chomp(C, C1)(C[] s, in C1[] delimiter)
{
if (delimiter == null)
return chomp(s);
else if (endsWith(s, delimiter))
return s[0 .. $ - delimiter.length];
else
return s;
}
Re: std.string.chomp error
On Mon, 09 Aug 2010 18:58:36 +0200, simendsjo wrote:
> The documentation says
> /***
> * Returns s[] sans trailing delimiter[], if any. * If delimiter[] is
> null, removes trailing CR, LF, or CRLF, if any. */
>
> To adhere to the documentation, chomp must be changed from:
>
> C[] chomp(C, C1)(C[] s, in C1[] delimiter) {
> if (endsWith(s, delimiter))
> {
> return s[0 .. $ - delimiter.length];
> }
> return s;
> }
>
> to:
>
> C[] chomp(C, C1)(C[] s, in C1[] delimiter) {
> if (delimiter == null)
>return chomp(s);
> else if (endsWith(s, delimiter))
>return s[0 .. $ - delimiter.length];
> else
>return s;
> }
Either that, or the documentation for it needs to be changed. Anyway, it
would be great if you'd report this.
http://d.puremagic.com/issues/
Thanks!
-Lars
Re: std.string.chomp error
Lars T. Kyllingstad:
> Either that, or the documentation for it needs to be changed. Anyway, it
> would be great if you'd report this.
A really basic unit testing is able to catch an error like this. This means
Phobos needs more unit tests.
Stuff that may be added to that unittest:
import std.stdio, std.string;
void main() {
assert(chomp("hello") == "hello");
assert(chomp("hello\n") == "hello");
assert(chomp("hello\r\n") == "hello");
assert(chomp("hello", "") == "hello");
assert(chomp("hello\n", "") == "hello");
assert(chomp("hello\r\n", "") == "hello");
assert(chomp("hello\r\n", "") == "hello");
assert(chomp("helloxxx", "x") == "helloxx");
}
But instead of:
if (delimiter == null)
Is better to write:
if (delimiter.length == 0)
Or:
if (delimiter == "")
See http://d.puremagic.com/issues/show_bug.cgi?id=3889
Bye,
bearophile
Re: std.string.chomp error
On 09.08.2010 19:16, Lars T. Kyllingstad wrote:
On Mon, 09 Aug 2010 18:58:36 +0200, simendsjo wrote:
The documentation says
/***
* Returns s[] sans trailing delimiter[], if any. * If delimiter[] is
null, removes trailing CR, LF, or CRLF, if any. */
To adhere to the documentation, chomp must be changed from:
C[] chomp(C, C1)(C[] s, in C1[] delimiter) {
if (endsWith(s, delimiter))
{
return s[0 .. $ - delimiter.length];
}
return s;
}
to:
C[] chomp(C, C1)(C[] s, in C1[] delimiter) {
if (delimiter == null)
return chomp(s);
else if (endsWith(s, delimiter))
return s[0 .. $ - delimiter.length];
else
return s;
}
Either that, or the documentation for it needs to be changed. Anyway, it
would be great if you'd report this.
http://d.puremagic.com/issues/
Thanks!
-Lars
Ok; http://d.puremagic.com/issues/show_bug.cgi?id=4608
Re: std.string.chomp error
simendsjo: > Ok; http://d.puremagic.com/issues/show_bug.cgi?id=4608 You seem to have missed my answer :-) Bye, bearophile
Re: std.string.chomp error
On 09.08.2010 23:58, bearophile wrote: simendsjo: Ok; http://d.puremagic.com/issues/show_bug.cgi?id=4608 You seem to have missed my answer :-) Bye, bearophile No, but I don't know if it's the documentation or implementation that's correct.
Re: std.string.chomp error
On 09.08.2010 19:34, bearophile wrote:
Lars T. Kyllingstad:
Either that, or the documentation for it needs to be changed. Anyway, it
would be great if you'd report this.
A really basic unit testing is able to catch an error like this. This means
Phobos needs more unit tests.
Stuff that may be added to that unittest:
import std.stdio, std.string;
void main() {
assert(chomp("hello") == "hello");
assert(chomp("hello\n") == "hello");
assert(chomp("hello\r\n") == "hello");
assert(chomp("hello", "") == "hello");
assert(chomp("hello\n", "") == "hello");
assert(chomp("hello\r\n", "") == "hello");
assert(chomp("hello\r\n", "") == "hello");
assert(chomp("helloxxx", "x") == "helloxx");
}
But instead of:
if (delimiter == null)
Is better to write:
if (delimiter.length == 0)
Or:
if (delimiter == "")
See http://d.puremagic.com/issues/show_bug.cgi?id=3889
Bye,
bearophile
Ahem.. :) Yes, I did miss your answer! How I got fooled by the preview
pane and never noticed the scrollbar.
I cannot see how your other bug report relates to this though. For
chomps part it's just an implementation vs. documentation issue.
Re: std.string.chomp error
simendsjo: > Ahem.. :) Yes, I did miss your answer! How I got fooled by the preview > pane and never noticed the scrollbar. No problem, it happens, don't worry. > I cannot see how your other bug report relates to this though. My other bug report is about this line in your code: if (delimiter == null) I don't like it :-) Bye, bearophile
Re: std.string.chomp error
On Monday, August 09, 2010 16:59:07 bearophile wrote: > simendsjo: > > Ahem.. :) Yes, I did miss your answer! How I got fooled by the preview > > pane and never noticed the scrollbar. > > No problem, it happens, don't worry. > > > I cannot see how your other bug report relates to this though. > > My other bug report is about this line in your code: > if (delimiter == null) > I don't like it :-) > > Bye, > bearophile Why, because it should be if(delimiter is null) or just if(!delimiter) - Jonathan M Davis
Re: std.string.chomp error
On 10.08.2010 02:09, Jonathan M Davis wrote: On Monday, August 09, 2010 16:59:07 bearophile wrote: simendsjo: Ahem.. :) Yes, I did miss your answer! How I got fooled by the preview pane and never noticed the scrollbar. No problem, it happens, don't worry. I cannot see how your other bug report relates to this though. My other bug report is about this line in your code: if (delimiter == null) I don't like it :-) Bye, bearophile Why, because it should be if(delimiter is null) or just if(!delimiter) - Jonathan M Davis Hehe.. You're a bit beyond my D level right now. At least I now know null == false and you can do reference is null :)
Re: std.string.chomp error
On Monday, August 09, 2010 17:09:03 simendsjo wrote: > On 10.08.2010 02:09, Jonathan M Davis wrote: > > On Monday, August 09, 2010 16:59:07 bearophile wrote: > >> simendsjo: > >>> Ahem.. :) Yes, I did miss your answer! How I got fooled by the preview > >>> pane and never noticed the scrollbar. > >> > >> No problem, it happens, don't worry. > >> > >>> I cannot see how your other bug report relates to this though. > >> > >> My other bug report is about this line in your code: > >> if (delimiter == null) > >> > >> I don't like it :-) > >> > >> Bye, > >> bearophile > > > > Why, because it should be > > > > if(delimiter is null) > > > > > > or just > > > > if(!delimiter) > > > > > > - Jonathan M Davis > > Hehe.. You're a bit beyond my D level right now. At least I now know > null == false and you can do reference is null :) IIRC, you're not really supposed to do "delim == null" but rather us "delim is null" or shorten to to just "!delim". Why, I don't recall off the top of my head, but it might be because "delim == null" would be calling Object.opEquals(), and there's no need for that function call (though fortunately "delim == null" translates to Object.opEquals(delim, null) rather than delim.opEquals(null) which avoids issues where the lhs is null and causes it to go boom). In either case, for null checks, I'd suggest either just using the reference by itself or to explictly use "is null" if you want the extra clarity. - Jonathan M Davis
Re: std.string.chomp error
Jonathan M Davis: > Why, because it should be > > if(delimiter is null) > > > or just > > if(!delimiter) if (delimiter.length == 0) Or if (!delimiter.length) Bye, bearophile
Re: std.string.chomp error
On 10.08.2010 03:08, bearophile wrote: Jonathan M Davis: Why, because it should be if(delimiter is null) or just if(!delimiter) if (delimiter.length == 0) Or if (!delimiter.length) Bye, bearophile Isn't that very different things? You cannot use .length if delimiter is null.
Re: std.string.chomp error
On Monday, August 09, 2010 18:12:11 simendsjo wrote: > On 10.08.2010 03:08, bearophile wrote: > > Jonathan M Davis: > >> Why, because it should be > >> > >> if(delimiter is null) > >> > >> > >> or just > >> > >> if(!delimiter) > > > > if (delimiter.length == 0) > > Or > > if (!delimiter.length) > > > > Bye, > > bearophile > > Isn't that very different things? You cannot use .length if delimiter is > null. If that's what you're looking for, then the proper thing to do would be to import std.array and do this if(delimiter.empty) It wille handle both null and length == 0 cases. Not to mention, it's much more range-y that way. - Jonathan M Davis
Re: std.string.chomp error
On Mon, 09 Aug 2010 17:35:56 -0700, Jonathan M Davis wrote:
> On Monday, August 09, 2010 17:09:03 simendsjo wrote:
>> On 10.08.2010 02:09, Jonathan M Davis wrote:
>> > On Monday, August 09, 2010 16:59:07 bearophile wrote:
>> >> simendsjo:
>> >>> Ahem.. :) Yes, I did miss your answer! How I got fooled by the
>> >>> preview pane and never noticed the scrollbar.
>> >>
>> >> No problem, it happens, don't worry.
>> >>
>> >>> I cannot see how your other bug report relates to this though.
>> >>
>> >> My other bug report is about this line in your code:
>> >> if (delimiter == null)
>> >>
>> >> I don't like it :-)
>> >>
>> >> Bye,
>> >> bearophile
>> >
>> > Why, because it should be
>> >
>> > if(delimiter is null)
>> >
>> >
>> > or just
>> >
>> > if(!delimiter)
>> >
>> >
>> > - Jonathan M Davis
>>
>> Hehe.. You're a bit beyond my D level right now. At least I now know
>> null == false and you can do reference is null :)
>
> IIRC, you're not really supposed to do "delim == null" but rather us
> "delim is null" or shorten to to just "!delim". Why, I don't recall off
> the top of my head, but it might be because "delim == null" would be
> calling Object.opEquals(), and there's no need for that function call
> (though fortunately "delim == null" translates to Object.opEquals(delim,
> null) rather than delim.opEquals(null) which avoids issues where the lhs
> is null and causes it to go boom).
>
> In either case, for null checks, I'd suggest either just using the
> reference by itself or to explictly use "is null" if you want the extra
> clarity.
No, using 'is' won't work. Check this out:
int[] a;
assert (a == null);
assert (a is null);
a = new int[10];
a.length = 0;
assert (a == null);
assert (a !is null);
The thing is, '==' tests whether two arrays are equal, that is, that they
are equally long and that their elements are equal. Any empty array is
equal to null -- in fact, in this context 'null' is just a way of
denoting an empty array that doesn't point to any particular memory block
(i.e. hasn't been initialised yet).
// This is what '==' does
bool mimicEquals(int[] a, int[] b)
{
if (a.length != b.length) return false;
foreach (i; 0 .. a.length) if (a[i] != b[i]) return false;
return true;
}
'is', on the other hand, tests whether two arrays are identical, i.e.
that they have the same length and *refer to the same piece of memory*.
// This is (sort of) what 'is' does
bool mimicIs(int[] a, int[] b)
{
return (a.ptr == b.ptr && a.length == b.length);
}
-Lars
Re: std.string.chomp error
On Tuesday 10 August 2010 00:30:37 Lars T. Kyllingstad wrote:
> No, using 'is' won't work. Check this out:
>
> int[] a;
> assert (a == null);
> assert (a is null);
>
> a = new int[10];
> a.length = 0;
> assert (a == null);
> assert (a !is null);
>
> The thing is, '==' tests whether two arrays are equal, that is, that they
> are equally long and that their elements are equal. Any empty array is
> equal to null -- in fact, in this context 'null' is just a way of
> denoting an empty array that doesn't point to any particular memory block
> (i.e. hasn't been initialised yet).
>
> // This is what '==' does
> bool mimicEquals(int[] a, int[] b)
> {
> if (a.length != b.length) return false;
> foreach (i; 0 .. a.length) if (a[i] != b[i]) return false;
> return true;
> }
>
> 'is', on the other hand, tests whether two arrays are identical, i.e.
> that they have the same length and *refer to the same piece of memory*.
>
> // This is (sort of) what 'is' does
> bool mimicIs(int[] a, int[] b)
> {
> return (a.ptr == b.ptr && a.length == b.length);
> }
>
> -Lars
Actually, it looks to me that that's an argument for using is for checking for
null rather than ==, since == isn't really going to tell you. The fact that ==
doesn't care about whether an array is null makes it not work for checking for
whether an array is null.
1. As I understand it, using is instead of == is for all references, not just
arrays and their bizarre pseudo-null state. Using is with a class will avoid
calling opEquals() and does exactly what you want when checking whether a class
reference is null.
2. For arrays, if you want to check whether it really is null, then you _must_
use is, because == obviously isn't going to tell you. It'll just lump empty
arrays in with null ones. For instance, if you want to check that an array has
never been initialized or that it has been set to null and never set to
something else, then you need to use is.
3. On the other hand, if what you really care about is checking whether an
array
has any elements and you don't care about whether it's null or not, then the
empty function/property would be the better way to go. It's quite explicit, and
it's more generic, doing things the way that ranges are done.
Personally, I think that the way that null is handled with arrays and
associative arrays is a poor design choice (if they're null, they should be
null
until you assign to them with new rather than this whole null but not null
nonsense), but we're stuck with it I guess.
- Jonathan M Davis
Re: std.string.chomp error
On Tue, 10 Aug 2010 01:48:17 -0700, Jonathan M Davis wrote:
> On Tuesday 10 August 2010 00:30:37 Lars T. Kyllingstad wrote:
>> No, using 'is' won't work. Check this out:
>>
>> int[] a;
>> assert (a == null);
>> assert (a is null);
>>
>> a = new int[10];
>> a.length = 0;
>> assert (a == null);
>> assert (a !is null);
>>
>> The thing is, '==' tests whether two arrays are equal, that is, that
>> they are equally long and that their elements are equal. Any empty
>> array is equal to null -- in fact, in this context 'null' is just a way
>> of denoting an empty array that doesn't point to any particular memory
>> block (i.e. hasn't been initialised yet).
>>
>> // This is what '==' does
>> bool mimicEquals(int[] a, int[] b)
>> {
>> if (a.length != b.length) return false; foreach (i; 0 ..
>> a.length) if (a[i] != b[i]) return false; return true;
>> }
>>
>> 'is', on the other hand, tests whether two arrays are identical, i.e.
>> that they have the same length and *refer to the same piece of memory*.
>>
>> // This is (sort of) what 'is' does
>> bool mimicIs(int[] a, int[] b)
>> {
>> return (a.ptr == b.ptr && a.length == b.length);
>> }
>>
>> -Lars
>
> Actually, it looks to me that that's an argument for using is for
> checking for null rather than ==, since == isn't really going to tell
> you. The fact that == doesn't care about whether an array is null makes
> it not work for checking for whether an array is null.
I guess it depends on what behaviour you're after. In the present case,
if you want chomp(a, null) and chomp(a, "") to do the same thing, then
you should use '=='. If you want chomp(a, "") to simply do nothing, use
'is'. I just figured that the former was the desired behaviour here. If
it isn't, I agree with you. :)
> 1. As I understand it, using is instead of == is for all references, not
> just arrays and their bizarre pseudo-null state. Using is with a class
> will avoid calling opEquals() and does exactly what you want when
> checking whether a class reference is null.
Fun fact: Actually, 'is' works for any type.
assert (1 is 1);
As I've understood it, 'a is b' is true if the variables a and b contain
the exact same bits. If a and b are value types, this must mean they
have the same value, and if they are references (including arrays), it
means they refer to the same data.
> 2. For arrays, if you want to check whether it really is null, then you
> _must_ use is, because == obviously isn't going to tell you. It'll just
> lump empty arrays in with null ones. For instance, if you want to check
> that an array has never been initialized or that it has been set to null
> and never set to something else, then you need to use is.
>
> 3. On the other hand, if what you really care about is checking whether
> an array has any elements and you don't care about whether it's null or
> not, then the empty function/property would be the better way to go.
> It's quite explicit, and it's more generic, doing things the way that
> ranges are done.
I totally agree with you. Lately, I have started using "empty" (as well
as the other range primitives) for arrays myself. I just disagreed that
'is' would produce what I perceived to be the right behaviour for the
function in question. But that perception may well be wrong. ;)
> Personally, I think that the way that null is handled with arrays and
> associative arrays is a poor design choice (if they're null, they should
> be null until you assign to them with new rather than this whole null
> but not null nonsense), but we're stuck with it I guess.
There, I don't agree with you. Arrays are a sort of pseudo-reference
type, so I don't mind 'null' being a sort of pseudo-null in that
context. Actually, I find it to be quite elegant. It's a matter of
taste, I guess.
-Lars
Re: std.string.chomp error
Lars T. Kyllingstad: > There, I don't agree with you. Arrays are a sort of pseudo-reference > type, so I don't mind 'null' being a sort of pseudo-null in that > context. Actually, I find it to be quite elegant. It's a matter of > taste, I guess. I suggest you to write down the things you don't like and the design you desire here. Bye, bearophile
Re: std.string.chomp error
On Tue, 10 Aug 2010 07:50:34 -0400, bearophile wrote: > Lars T. Kyllingstad: >> There, I don't agree with you. Arrays are a sort of pseudo-reference >> type, so I don't mind 'null' being a sort of pseudo-null in that >> context. Actually, I find it to be quite elegant. It's a matter of >> taste, I guess. > > I suggest you to write down the things you don't like and the design you > desire here. ?? I like how it is designed now, that was my point. -Lars
Re: std.string.chomp error
Lars T. Kyllingstad: > I like how it is designed now, that was my point. Sorry, I misread you.
Re: std.string.chomp error
On Tuesday 10 August 2010 02:34:33 Lars T. Kyllingstad wrote: > I guess it depends on what behaviour you're after. In the present case, > if you want chomp(a, null) and chomp(a, "") to do the same thing, then > you should use '=='. If you want chomp(a, "") to simply do nothing, use > 'is'. I just figured that the former was the desired behaviour here. If > it isn't, I agree with you. :) I was really talking about the general case rather than chomp() in specific, but's fine with me if "" and null are treated the same here, I suppose. Still, I really don't like it when null and empty is treated the same way (in D or anything else). There's a difference between something not existing and it existing but not having anything in it. But, as I said, we're stuck with the way it is in D. - Jonathan M Davis
