FAQ

On Thu Jun 30 13:26:01 2011, claes wrote:

Hi,

saw a 0.00 release uploaded today and out of curiosity tried using a
negative $VERSION in a module and then C<use Module VERSION> and it
reported a 'Invalid version format (non-numeric data)' which is
slightly confusing. Attached patch checks if it begings with a dash
and then reports a better error. Also updated docs that version is
expected to be a positive number.
Thank you. Applied as f39335f98.

Search Discussions

  • John Peacock at Jul 1, 2011 at 11:17 am

    On 06/30/2011 10:33 PM, Father Chrysostomos via RT wrote:
    On Thu Jun 30 13:26:01 2011, claes wrote:
    Hi,

    saw a 0.00 release uploaded today and out of curiosity tried using a
    negative $VERSION in a module and then C<use Module VERSION> and it
    reported a 'Invalid version format (non-numeric data)' which is
    slightly confusing. Attached patch checks if it begings with a dash
    and then reports a better error. Also updated docs that version is
    expected to be a positive number.
    Thank you. Applied as f39335f98.
    I'll send a diff this evening (have to run to work), but it would have
    been good to let the patch hang out there for comments before
    committing. That is the wrong place in the code to make that change
    (plus adding a new error without a regression test is a Bad Thing).

    John
  • Claes Jakobsson at Jul 1, 2011 at 4:16 pm

    On 1 jul 2011, at 13.17, John Peacock wrote:
    On 06/30/2011 10:33 PM, Father Chrysostomos via RT wrote:
    On Thu Jun 30 13:26:01 2011, claes wrote:
    Hi,

    saw a 0.00 release uploaded today and out of curiosity tried using a
    negative $VERSION in a module and then C<use Module VERSION> and it
    reported a 'Invalid version format (non-numeric data)' which is
    slightly confusing. Attached patch checks if it begings with a dash
    and then reports a better error. Also updated docs that version is
    expected to be a positive number.
    Thank you. Applied as f39335f98.
    I'll send a diff this evening (have to run to work), but it would have been good to let the patch hang out there for comments before committing. That is the wrong place in the code to make that change (plus adding a new error without a regression test is a Bad Thing).
    Ok, but if Perl_prescan_version isn't the right place then where is? The same error is reported for example when doing C<perl -e 'require -1'> so I figured this is where it should go.

    Actually I can't find really any tests for the various errors that Perl_prescan_version reports. I'll write such a test tonight.

    Thanks,
    Claes
  • John Peacock at Jul 2, 2011 at 3:21 am

    On 07/01/2011 12:16 PM, Claes Jakobsson wrote:
    Ok, but if Perl_prescan_version isn't the right place then where is?
    The same error is reported for example when doing C<perl -e 'require
    -1'> so I figured this is where it should go.
    Oh, it is the correct function, just the wrong spot. If you look a few
    lines above, the test for a negative sign should be prior to the comment
    "consume all of the integer part" because that is the only place where
    the negative sign should exist when parsing the input.
    Actually I can't find really any tests for the various errors that Perl_prescan_version reports. I'll write such a test tonight.
    lib/version.t contains all of the tests (and is based on the
    coretests.pm fromthe CPAN release).

    Patch attached, which also bumps the $VERSION to match what will be
    hitting CPAN once I finish it...

    John
  • Father Chrysostomos via RT at Jul 2, 2011 at 5:00 am

    On Fri Jul 01 20:21:48 2011, john.peacock@havurah-software.org wrote:
    On 07/01/2011 12:16 PM, Claes Jakobsson wrote:
    Ok, but if Perl_prescan_version isn't the right place then where is?
    The same error is reported for example when doing C<perl -e 'require
    -1'> so I figured this is where it should go.
    Oh, it is the correct function, just the wrong spot. If you look a
    few
    lines above, the test for a negative sign should be prior to the
    comment
    "consume all of the integer part" because that is the only place where
    the negative sign should exist when parsing the input.
    Actually I can't find really any tests for the various errors that
    Perl_prescan_version reports. I'll write such a test tonight.

    lib/version.t contains all of the tests (and is based on the
    coretests.pm fromthe CPAN release).

    Patch attached, which also bumps the $VERSION to match what will be
    hitting CPAN once I finish it...

    John
    Thank you. Applied as c8c8e589d7.
  • Claes Jakobsson at Jul 2, 2011 at 10:46 pm

    On 2 jul 2011, at 05.21, John Peacock wrote:
    On 07/01/2011 12:16 PM, Claes Jakobsson wrote:
    Ok, but if Perl_prescan_version isn't the right place then where is? The same error is reported for example when doing C<perl -e 'require -1'> so I figured this is where it should go.
    Oh, it is the correct function, just the wrong spot. If you look a few lines above, the test for a negative sign should be prior to the comment "consume all of the integer part" because that is the only place where the negative sign should exist when parsing the input.
    Ok. I placed it further down since that's where there are a bunch of other tests but yes, having the check before scanning for digits makes more sense.
    Actually I can't find really any tests for the various errors that Perl_prescan_version reports. I'll write such a test tonight.
    lib/version.t contains all of the tests (and is based on the coretests.pm fromthe CPAN release).
    Ah, that explains it.. I was only looking in t/.
    Patch attached, which also bumps the $VERSION to match what will be hitting CPAN once I finish it...
    Thanks for your help.

    /Claes

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupperl5-porters @
categoriesperl
postedJul 1, '11 at 2:33a
activeJul 2, '11 at 10:46p
posts6
users3
websiteperl.org

People

Translate

site design / logo © 2022 Grokbase