Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support larger MAJOR version numbers #149

Merged
merged 3 commits into from
Aug 30, 2023
Merged

Support larger MAJOR version numbers #149

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e6e152
Support larger MAJOR version numbers
onethumb Feb 1, 2023
249b023
Reduce repetition count to a saner level to retry CI
Seldaek Feb 1, 2023
c42e6c2
Add new formats without breaking any existing ones
Seldaek Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/VersionParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public function normalize($version, $fullVersion = null)
}

// match classical versioning
if (preg_match('{^v?(\d{1,5})(\.\d++)?(\.\d++)?(\.\d++)?' . self::$modifierRegex . '$}i', $version, $matches)) {
if (preg_match('{^v?(\d{1,' . PHP_INT_MAX . '})(\.\d++)?(\.\d++)?(\.\d++)?' . self::$modifierRegex . '$}i', $version, $matches)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP_INT_MAX on 64bit (9223372036854775807) is only 19 characters long so if anything it should look something like this IMO as that's the char length and not the max number:

Suggested change
if (preg_match('{^v?(\d{1,' . PHP_INT_MAX . '})(\.\d++)?(\.\d++)?(\.\d++)?' . self::$modifierRegex . '$}i', $version, $matches)) {
if (preg_match('{^v?(\d{1,20})(\.\d++)?(\.\d++)?(\.\d++)?' . self::$modifierRegex . '$}i', $version, $matches)) {

The problem though is this is only meant to parse x.y.z versions <100000, and then below we have the code for datetime versions but which do already support many formats as you can see in the tests. I'm not entirely sure yet why your format isn't supported but I don't think this fix is appropriate as I believe it changes the behavior for existing formats.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about value vs char length. Open to whatever works best... just would like to use YYYYMMDD.y.z :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If other versions can be of any size - can they really, does composer support it fully? - then this should be simply \d++ to be consistent.

$version = $matches[1]
. (!empty($matches[2]) ? $matches[2] : '.0')
. (!empty($matches[3]) ? $matches[3] : '.0')
Expand Down
3 changes: 3 additions & 0 deletions tests/VersionParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ public function successfulNormalizedVersions()
'parses dates w/ . as classical' => array('2010.01.02', '2010.01.02.0'),
'parses dates y.m.Y as classical' => array('2010.1.555', '2010.1.555.0'),
'parses dates y.m.Y/2 as classical' => array('2010.10.200', '2010.10.200.0'),
'parses CalVer YYYYMMDD (as MAJOR) versions' => array('20230131.0.0', '20230131.0.0'),
'parses CalVer YYYYMMDDhhmm (as MAJOR) versions' => array('202301310000.0.0', '202301310000.0.0'),
'parses PHP_INT_MAX MAJOR version' => array(PHP_INT_MAX . '.0.0', PHP_INT_MAX . '.0.0'),
'strips v/datetime' => array('v20100102', '20100102'),
'parses dates w/ -' => array('2010-01-02', '2010.01.02'),
'parses dates w/ .' => array('2012.06.07', '2012.06.07.0'),
Expand Down