Replace special chars in InformationalVersion with dashes #2014
Conversation
… when a custom format is used.
| @@ -56,7 +57,7 @@ public VersionVariables GetVariablesFor(SemanticVersion semanticVersion, Effecti | |||
| var semverFormatValues = new SemanticVersionFormatValues(semanticVersion, config); | |||
|
|
|||
| var informationalVersion = CheckAndFormatString(config.AssemblyInformationalFormat, semverFormatValues, | |||
| environment, semverFormatValues.DefaultInformationalVersion, "AssemblyInformationalVersion"); | |||
| environment, semverFormatValues.DefaultInformationalVersion, "AssemblyInformationalVersion").RegexReplace("[^0-9A-Za-z-.+]", "-"); | |||
There was a problem hiding this comment.
Is it not possible for CheckAndFormatString to perform the RegexReplace? The problem can in theory occur for assemblyFileSemVer and assemblySemVer as well if their formats or schemes include the prerelease label (tag)?
There was a problem hiding this comment.
Hmm.
I see someone removed the fields assembly-versioning-format and assembly-file-versioning-format from the documentation but not from the code.
I would say it not needed to move the regex because for assembly version and assembly file version the rules are way more restricting.
Basicly only the schemes described in
and
are allowed by Microsoft specs.
I guess that is also the reason why the fields are no longer in the documentation for GitVersion 4.x and 5.x.
Also see Microsoft docs
There was a problem hiding this comment.
Good point. Would moving the RegexReplace hurt, though? I would prefer it inside the method if it doesn't actually destroy anything.
There was a problem hiding this comment.
Nop would not hurt i guess.
Just a theoretical performance hit. But i don't think that will be noticeable.
Edit: actually the performance would be better as the RegexReplace will only be executed if a format has been given. I committed the change
|
Thank you so much for your contributions, @Skoucail! 🙏 |
Replacing all characters not allowed in SemVer 2.0 by dashes in the InformationVersion field.
This was already the case for the default format but not if a custom format was used.
2 tests were added to check the fix: one with and one without a format.
Discussion in #1520