sprintf of version objects #8243
Comments
From @gisleCreated by @gisleVersion objects are special cased in perl's sprintf() code, but the result $ perl -e 'printf "%vd\n", $^V' Perl Info |
From @JohnPeacockGisle Aas (via RT) wrote:
sv_vcatpvfn() being a good example of overloaded operations gone John -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @gisleJohn Peacock <jpeacock@rowman.com> writes:
I'm just looking at Perl_sv_vcatpvfn and was kind of surprised by the
There are some examples of how the vector flag can be useful in the --Gisle |
From @TuxOn 12 Dec 2005 09:52:37 -0800, Gisle Aas <gisle@ActiveState.com> wrote:
lt09:/pro/3gl/CPAN/perl-current 117 > ./perl -e '(
-- |
From @JohnPeacockGisle Aas wrote:
Hysterical Raisins, and all that. $^V was documented as returning a printf("%vd", $^V); exists in the wild so we have to carefully neuter that code path so that John -- |
From @petdanceOn Mon, Dec 12, 2005 at 07:17:13PM +0100, H.Merijn Brand (h.m.brand@xs4all.nl) wrote:
If that doesn't belong in t/op/sprintf.t, I don't know what does. Are you going to patch it or should I? -- |
From @JohnPeacockJohn Peacock wrote:
I realized last night what I need to do (but didn't have the brainpower IIF the term is a version object, then I'll convert that back to its Thoughts? Complaints? John -- |
From @gisleJohn Peacock <jpeacock@rowman.com> writes:
Sounds good to me. --Gisle |
From @gisleJohn Peacock <jpeacock@rowman.com> writes:
How about something like the following patch? It allow any array $ perl -e 'printf "%vd\n", $^V' I find this kind of cool, but I'm not sure if this is really useful $str = sprintf "%vX", \@array; is short for: $str = join(".", map sprintf("%X", $_), @array); --Gisle ==== //depot/perl/sv.c#1056 - /home/gisle/perl/blead/sv.c ==== uns_integer: |
From @JohnPeacockGisle Aas wrote:
I don't know if there is any utility to this more general patch, since John -- |
From @JohnPeacockGisle Aas wrote:
OK, here's my alternate patch, along with tests. This simply recreates Since the only situation that should be in code in the wild refers to John -- |
From @JohnPeacocksv.c.diff=== sv.c
==================================================================
--- sv.c (revision 18901)
+++ sv.c (local)
@@ -8000,24 +8000,41 @@
}
else if (efix ? (efix > 0 && efix <= svmax) : svix < svmax) {
vecsv = svargs[efix ? efix-1 : svix++];
- vecstr = (U8*)SvPV_const(vecsv,veclen);
- vec_utf8 = DO_UTF8(vecsv);
- /* if this is a version object, we need to return the
- * stringified representation (which the SvPVX_const has
- * already done for us), but not vectorize the args
+
+ /* if this is a version object, we need to convert
+ * back into v-string notation and then let the
+ * vectorize happen normally
*/
- if ( *q == 'd' && sv_derived_from(vecsv,"version") )
+ if ( sv_derived_from(vecsv,"version") )
{
- q++; /* skip past the rest of the %vd format */
- eptr = (const char *) vecstr;
- elen = veclen;
- if (elen && *eptr == 'v') {
- eptr++;
- elen--;
- }
- vectorize=FALSE;
- goto string;
+ I32 i, len;
+ UV digit;
+ U8 tmpbuf[UTF8_MAXBYTES+1];
+ U8 *tmpend;
+ SV *sv = sv_newmortal();
+ AV *av =
+ (AV *)SvRV(*hv_fetch((HV*)SvRV(vecsv),
+ "version", 7, FALSE));
+ len = av_len(av);
+ sv_setpvn(sv, "", 0);
+ vecsv = sv_mortalcopy(vecsv);
+
+ digit = (UV)SvIV(*av_fetch(av, 0, 0));
+ tmpend = uvchr_to_utf8(tmpbuf, digit);
+ sv_setpvn(vecsv, (const char*)tmpbuf, tmpend - tmpbuf);
+ veclen = tmpend - tmpbuf;
+
+ for ( i = 1 ; i <= len ; i++ ) {
+ digit = (UV)SvIV(*av_fetch(av, i, 0));
+ tmpend = uvchr_to_utf8(tmpbuf, digit);
+ sv_catpvn(vecsv, (const char*)tmpbuf, tmpend - tmpbuf);
+ veclen += tmpend - tmpbuf;
+ if (!UNI_IS_INVARIANT(NATIVE_TO_UNI(digit)))
+ SvUTF8_on(vecsv);
+ }
}
+ vecstr = (U8*)SvPV_const(vecsv,veclen);
+ vec_utf8 = DO_UTF8(vecsv);
}
else {
vecstr = (U8*)"";
=== t/op/sprintf.t
==================================================================
--- t/op/sprintf.t (revision 18901)
+++ t/op/sprintf.t (local)
@@ -245,8 +245,10 @@
>%vd< >[version::qv("1.2.3")]< >1.2.3<
>%v.3d< >"\01\02\03"< >001.002.003<
>%0v3d< >"\01\02\03"< >001.002.003<
+>%v.3d< >[version::qv("1.2.3")]< >001.002.003<
>%-v3d< >"\01\02\03"< >1 .2 .3 <
>%+-v3d< >"\01\02\03"< >+1 .2 .3 <
+>%+-v3d< >[version::qv("1.2.3")]< >+1 .2 .3 <
>%v4.3d< >"\01\02\03"< > 001. 002. 003<
>%0v4.3d< >"\01\02\03"< >0001.0002.0003<
>%0*v2d< >['-', "\0\7\14"]< >00-07-12<
@@ -257,6 +259,7 @@
>%v*.*d< >["\01\02\03", 4, 3]< > 001. 002. 003<
>%0v*.*d< >["\01\02\03", 4, 3]< >0001.0002.0003<
>%0*v*d< >['-', "\0\7\13", 2]< >00-07-11<
+>%0*v*d< >['-', version::qv("0.7.11"), 2]< >00-07-11<
>%e< >1234.875< >1.234875e+03<
>%e< >0.000012345< >1.234500e-05<
>%e< >1234567E96< >1.234567e+102<
@@ -391,6 +394,8 @@
>%-010x< >2**32-1< >ffffffff <
>%0-10x< >2**32-1< >ffffffff <
>%0*x< >[-10, ,2**32-1]< >ffffffff <
+>%vx< >[version::qv("1.2.3")]< >1.2.3<
+>%vx< >[version::qv("1.20.300")]< >1.14.12c<
>%y< >''< >%y INVALID<
>%z< >''< >%z INVALID<
>%2$d %1$d< >[12, 34]< >34 12<
|
From @JohnPeacockJohn Peacock wrote:
Nah, ignore that; it encapsulates too much knowledge about how version objects John -- |
From @JohnPeacock=== sv.c--- sv.c (revision 18901)
|
From @gisleJohn Peacock <jpeacock@rowman.com> writes:
This calculation is meaningless as it is overwritten in the next
I suggest you make it: ==== //depot/perl/sv.c#1059 - /home/gisle/perl/blead/sv.c ==== |
From @gisleGisle Aas <gisle@ActiveState.com> writes:
This cast is also useless. A plain (char*) can always be passed to a --Gisle |
From @JohnPeacockGisle Aas wrote:
Duh! I misread that and thought that SvPV_const(vecsv,veclen) required Attached is a new path with Gisle's suggested changes as well as John -- |
From @JohnPeacock=== sv.c--- sv.c (revision 18901)
|
From @ysthOn Wed, Dec 14, 2005 at 10:12:56AM -0500, John Peacock wrote:
Patches welcome... |
From @ysthOn Tue, Dec 13, 2005 at 12:09:39PM -0800, Gisle Aas wrote:
I like this idea a lot. |
From @gisleJohn Peacock <jpeacock@rowman.com> writes:
I applied this as change 26365 and added some extra test cases. I was >%vd< >[version::qv("1.2")]< >1.2.0< Please advice what the expected output of these should be? --Gisle |
From @JohnPeacockGisle Aas wrote:
!!!!! IMPORTANT NOTE: qv() is *not* equivalent to new() !!!!! qv() forces the input value to be interpreted as if it were an "Extended If you replace 'version::qv' with 'version->new' in all of those tests, $v = version->new("1.02_03") => stringifies to 1.02_0300 I suppose we could simply error out if the version object was an alpha hv_exists((HV*)SvRV(vecsv), "alpha", 5) but that doesn't check to make sure that the vecsv is actually an RV, sv_derived_from(vecsv,"version") even to get into that block. Let me sync up to what you applied and see John -- |
From @JohnPeacockYitzchak Scott-Thoennes wrote:
This is a grey area, though: Documented in perlapi == Public API Do we want to document all of these macros, just because they are used John -- |
From @rgsGisle Aas (via RT) wrote:
Currently we have : $ cat t.pl $ bleadperl t.pl Those results are satisfying to me, can we close this bug ? |
From @JohnPeacockRafael Garcia-Suarez wrote:
Ay ay, captain! John -- |
|
@rgs - Status changed from 'open' to 'resolved' |
From @cpansproutOn Thu Dec 15 07:35:12 2005, jpeacock@rowman.com wrote:
I see that this change was made, in commit 34ba632, but as a warning. Unfortunately, in causes sprintf("[%vd]", new version v1.1_1) to return That doesn’t make sense to me. I think it should simply turn into the -- Father Chrysostomos |
From @JohnPeacockOn 09/13/2012 01:26 PM, Father Chrysostomos via RT wrote:
I don't understand this paragraph. Could you show us with code what you $ perl -e 'sprintf("[%vd]", new version v1.1_1)' which is precisely what was agreed to in the discussion. I would also note that this ticket was marked as resolved 6 1/2 years John |
From @cpansproutOn Thu Sep 13 18:02:14 2012, john.peacock@havurah-software.org wrote:
Try it without the first s. :-) $ perl5.16.0 -e 'printf "[%vd]\n", new version v1.1_1' I expect to see anything but %vd. Turn on -w and the extra warning is telling: $ perl5.16.0 -we 'printf "[%vd]\n", new version v1.1_1' %vd is not invalid. It’s the argument that is invalid. I think the vector warning and an empty string for output would be best. -- Father Chrysostomos |
From @JohnPeacockOn 09/13/2012 11:42 PM, Father Chrysostomos via RT wrote:
Like the attached? LD_LIBRARY_PATH=. ./perl -we 'printf "[%vd]\n", new version v1.1_1' I haven't figured out how to patch t/op/sprintf.t to cope with the John |
From @JohnPeacockvector_alpha.diffdiff -r d7334029637c sv.c
--- a/sv.c Tue Sep 11 13:58:24 2012 -0700
+++ b/sv.c Sat Sep 15 11:00:16 2012 -0400
@@ -10380,13 +10380,17 @@
if ( hv_exists(MUTABLE_HV(SvRV(vecsv)), "alpha", 5 ) ) {
Perl_warner(aTHX_ packWARN(WARN_INTERNAL),
"vector argument not supported with alpha versions");
- goto unknown;
+ vecsv = sv_newmortal();
+ vecstr = (U8*)"";
+ veclen = 0;
}
- vecsv = sv_newmortal();
- scan_vstring(version, version + veclen, vecsv);
- vecstr = (U8*)SvPV_const(vecsv, veclen);
- vec_utf8 = DO_UTF8(vecsv);
- Safefree(version);
+ else {
+ vecsv = sv_newmortal();
+ scan_vstring(version, version + veclen, vecsv);
+ vecstr = (U8*)SvPV_const(vecsv, veclen);
+ vec_utf8 = DO_UTF8(vecsv);
+ Safefree(version);
+ }
}
}
else {
|
From @cpansproutOn Sat Sep 15 08:08:19 2012, john.peacock@havurah-software.org wrote:
Oops. Sorry. I have already fixed this, but I forgot to say so. The commit id is -- Father Chrysostomos |
From @cpansproutOn Sat Sep 15 08:08:19 2012, john.peacock@havurah-software.org wrote:
Oops. Sorry. I have already fixed this, but I forgot to say so. The commit id is -- Father Chrysostomos |
From [Unknown Contact. See original ticket]On Sat Sep 15 08:08:19 2012, john.peacock@havurah-software.org wrote:
Oops. Sorry. I have already fixed this, but I forgot to say so. The commit id is -- Father Chrysostomos |
Migrated from rt.perl.org#37897 (status was 'resolved')
Searchable as RT37897$
The text was updated successfully, but these errors were encountered: