Re: Change 18682: [PATCH] version objects final(?) patch

---Neglected to send this to the list as well---

Philip Newton wrote:
> Some comments on this patch.
> 
> First of all, in several places, things are mentioned as working "in
> Perl > 5.8.0" (or 5.9.0, or whatever). Is that really ">" or should it
> be ">="?

No, I really mean > 5.8.0, since there are bits that won't work consistently
until 5.8.1 comes out (since I think I have Jarkko convinced that the magic
v-string code is a worthwhile addition).  Until magic v-strings, it is
impossible to use bare v-strings as initializers for version objects.

>>+newest Perl v-strings themselves can be used to initialize version
> 
> 
>>+newest Perl, v-strings themselves can be used to initialize version
> 
>               ^

Yes, that is better English usage; I'll add that to the next patch I submit.

>>+
>>+  $VERSION = new version "v2.6";
> 
> 
> 
> I assume you meant "v2.7" here. (A bit risky using a CVS variable in
> examples since you have to change the appropriate bits manually each
> time you check in, without fail.)

Yes, that did slip by me.  Unfortunately, I know of no way to escape the CVS
variable processing by CVS itself, but since I am moving to SubVersion, I can
deal with this by not using CVS myself anymore!

>>   $ver <  2.5     $ver lt            true
> 
> What's the second example supposed to be? '$ver lt "2.5"', presumably?

Oops.  Yes, it should be "2.5" there.

>> 	Perl_croak(aTHX_ "Usage: version::new(class, version)");
> 
> 
> Should this be amended if three items are now acceptable?

Yes, but I'm not sure how to word that.  If two items are passed, the second is
the version, but if three are passed, the version is the third item.

> 
> What if only one item was passed? Wouldn't that cause this bit
> 
> 
>>     SP -= items;
>>     {
>> /*	char *	class = (char *)SvPV_nolen(ST(0)); */
>>-	SV *	version = ST(1);
>>-
>>-{
>>-    PUSHs(new_version(version));
>>-}
>>+        SV *version = ST(1);
> 
> 
> to produce unwanted results?

Yes, but no one is supposed to use the version::new() function call, as opposed
to the version->new() OO call, where the class name (or existing object) is
always passed as the first term.  Your point is valid, however; I'll make this
more bulletproof.

Thanks for the feedback.

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD  20706
301-459-3366 x.5010
fax 301-429-5748

• Re: Change 18682: [PATCH] version objects final(?) patch by John Peacock