Regex #30
Regex #30
Conversation
|
Resolves #19 |
| else 0 | ||
|
|
||
| private fun comparePreReleases(preRelease: String, other: String): Int { | ||
| val qualifiers: Map<String, Int> = mapOf( | ||
| Pair("alpha", 1), |
There was a problem hiding this comment.
fyi: you can use "alpha" to 1 syntax
| preRelease.startsWith('a') -> "alpha" | ||
| preRelease.startsWith('b') -> "beta" | ||
| preRelease.startsWith('m') -> "milestone" | ||
| else -> qualifiers.keys.firstOrNull { preRelease.contains(it) } |
There was a problem hiding this comment.
is it safe to look for it like that? I think that both map and its keys may be unordered so it might not be predictable what results will you get.
Also, shouldn't you first look for the qualifiers in map and then check first letters? (I don't know the specs and it will not change anything now, but if it matters and another qualifier with low priority would be added and it would start with a, b or m it could break.
There was a problem hiding this comment.
Yes, as maps retain order of the value so results are predictable
And I need to check one thing about qualifiers and trimming of 0's as I noticed one bug in this logic
There was a problem hiding this comment.
Bugs was fixed. Creating issue about trimming null values
| else 0 | ||
|
|
||
| private fun comparePreReleases(preRelease: String, other: String): Int { |
There was a problem hiding this comment.
preRelease and other are names suggesting that we are dealing with 2 completely different things, while we are dealing with two preRelease components. More common names like
a, b
preRelease1, preRelease2
this, that
first, second
would make it easier to follow
| } else it | ||
| } | ||
|
|
||
| return compareQualifiers |
There was a problem hiding this comment.
you could inline this variable
| else -> qualifiers[firstQualifier]!!.compareTo(qualifiers[otherQualifier]!!) | ||
| }.let { | ||
| if (it == 0) { | ||
| val preReleaseNoQualifier = getPreReleaseNoQualifier(firstQualifier!!, preRelease) |
There was a problem hiding this comment.
Fixed as it wasn't safe
| } | ||
| } | ||
|
|
||
| private fun getPreReleaseNoQualifier(qualifier: String, preRelease: String): String { |
There was a problem hiding this comment.
what is the logic of this function?
There was a problem hiding this comment.
Changed name to better show logic of a function
| values[3].toInt(), | ||
| values[4], | ||
| values[5] | ||
| values["majorRegex"]?.value?.toIntOrNull() ?: 0, |
There was a problem hiding this comment.
Do we want to keep group names with suffix "regex" ? To keep it consistent users in the config will have to do it in the same way
There was a problem hiding this comment.
Right, I missed this part. Please remove the regex suffix.
There was a problem hiding this comment.
Removed suffix "Regex" from groups
Changes in regex of Semantic Version, parsing from string and in comparing two prerelease versions. Added tests