Support integer formatting with specifiers D and B #4789
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds support for Binary (B) and Decimal (D) format specifiers for integer formatting in GitVersion's custom formatting system. The change allows version components to be formatted as binary or zero-padded decimal numbers, complementing the existing Hexadecimal (X) support.
Key changes:
- Extended
NumericFormatterto handle "B" and "D" format specifiers alongside the existing "X" format - Added test cases to verify formatting behavior with precision specifiers (b8, d8, x8)
- Updated documentation to include the new Binary format specifier
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/GitVersion.Core/Formatting/NumericFormatter.cs | Extended integer formatting logic to handle B (binary), D (decimal), and X (hexadecimal) format specifiers with a unified code path |
| src/GitVersion.Core.Tests/Formatting/NumericFormatterTests.cs | Added test cases for binary, decimal, and hexadecimal formatting with precision specifiers; also removed BOM character from file |
| docs/input/docs/reference/custom-formatting.md | Added documentation for the B (binary) format specifier |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ("BDX".Contains(char.ToUpperInvariant(format[0])) && int.TryParse(s, NumberStyles.Integer, cultureInfo, out var hex)) | ||
| { | ||
| result = hex.ToString(format, cultureInfo); |
There was a problem hiding this comment.
The variable name 'hex' is misleading as it now handles Binary (B), Decimal (D), and Hexadecimal (X) format specifiers. Consider renaming it to something more generic like 'integer' or 'intValue' to better reflect its purpose across all three format types.
| if ("BDX".Contains(char.ToUpperInvariant(format[0])) && int.TryParse(s, NumberStyles.Integer, cultureInfo, out var hex)) | |
| { | |
| result = hex.ToString(format, cultureInfo); | |
| if ("BDX".Contains(char.ToUpperInvariant(format[0])) && int.TryParse(s, NumberStyles.Integer, cultureInfo, out var intValue)) | |
| { | |
| result = intValue.ToString(format, cultureInfo); |
|
| // Hexadecimal formatting | ||
| if (format.StartsWith("X", StringComparison.OrdinalIgnoreCase) && int.TryParse(s, NumberStyles.Integer, cultureInfo, out var hex)) | ||
| // Integer formatting with precision specifier | ||
| if ("BDX".Contains(char.ToUpperInvariant(format[0])) && int.TryParse(s, NumberStyles.Integer, cultureInfo, out var n)) |
There was a problem hiding this comment.
do you mind changing the variable from n to numericValue, and to make it consistent, please do the same for other similar variables in the method
2 participants
Description
Implement proper handling of format specifers "D" and "B".
Related Issue
Resolves #4788
Motivation and Context
I want to be able to format Major, Minor, Patch into one integer so it can be used as Android versionCode.
Format specifier "D" does the job. Format specifier "B" is added, because, why not, it comes for free.
How Has This Been Tested?
Tests have been added to this PR by expanding existing test group
GitVersion.Tests.Formatting.NumericFormatterTests.TryFormat_ValidFormats_ReturnsExpectedResult.Checklist: