String repr of Dimension is system culture sensitive #53
Conversation
Gotenberg do not support decimal numbers with comma decimal separator.
WalkthroughThe changes add new configuration rules to handle files with extensions such as Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant HtmlRequestBuilder
Test->>HtmlRequestBuilder: Create instance
HtmlRequestBuilder->>HtmlRequestBuilder: Add HTML content and parameters (scale, paper dimensions, etc.)
HtmlRequestBuilder->>Test: Build API request message
Test->>Test: Verify correct formatting and decimal separator
sequenceDiagram
participant Test
participant Dimension
Test->>Dimension: Instantiate with value (e.g., 11.7)
Dimension->>Dimension: Convert value to string using InvariantCulture
Dimension-->>Test: Return formatted string "11.7in"
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Gotenberg.Sharp.Api.Client.Tests/Domain/Dimensions/DimensionTests.cs (1)
20-31: LGTM! Well-structured test case.The test effectively verifies the culture-invariant decimal separator formatting.
Consider adding more test cases to cover:
- Large numbers (e.g., 1234.5678)
- Small decimals (e.g., 0.001)
- Whole numbers (e.g., 42)
lib/Domain/Dimensions/Dimension.cs (1)
93-93: Consider using culture-invariant parsing.The
Parsemethod should also useCultureInfo.InvariantCultureto ensure consistent parsing of decimal values.- var value = double.Parse(match.Groups[1].Value); + var value = double.Parse(match.Groups[1].Value, CultureInfo.InvariantCulture);Gotenberg.Sharp.Api.Client.Tests/Gotenberg.Sharp.Api.Client.Tests.csproj (1)
24-26: Using Directive Inclusion – Verify Necessity.
The<Using Include="Xunit"/>entry is included here. Although implicit usings are enabled, if explicit global usings for xUnit are intended, please confirm that this aligns with your coding conventions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.editorconfig(1 hunks).gitignore(1 hunks)Gotenberg.Sharp.Api.Client.Tests/Domain/Builders/HtmlRequestBuilderTests.ToApiRequestMessage_PaperDimensionWithDecimal_NumbersUsesDotAsDecimalSeparator_Test.verified.txt(1 hunks)Gotenberg.Sharp.Api.Client.Tests/Domain/Builders/HtmlRequestBuilderTests.cs(1 hunks)Gotenberg.Sharp.Api.Client.Tests/Domain/Dimensions/DimensionTests.cs(1 hunks)Gotenberg.Sharp.Api.Client.Tests/Gotenberg.Sharp.Api.Client.Tests.csproj(1 hunks)GotenbergSharpApiClient.sln(2 hunks)lib/Domain/Dimensions/Dimension.cs(2 hunks)
🔇 Additional comments (15)
Gotenberg.Sharp.Api.Client.Tests/Domain/Builders/HtmlRequestBuilderTests.cs (1)
20-38: LGTM! Comprehensive integration test.The test effectively verifies that decimal dimensions are correctly formatted in the API request content, with good use of the Verify framework for snapshot testing.
GotenbergSharpApiClient.sln (1)
13-48: LGTM! Test project properly configured.The test project is correctly integrated into the solution with all necessary build configurations.
lib/Domain/Dimensions/Dimension.cs (1)
17-17: LGTM! Culture-invariant string formatting implemented correctly.The use of
CultureInfo.InvariantCultureensures consistent decimal formatting regardless of system culture.Also applies to: 149-152
Gotenberg.Sharp.Api.Client.Tests/Gotenberg.Sharp.Api.Client.Tests.csproj (4)
1-8: Project Configuration is Correct and Modern.
The<PropertyGroup>properly targets .NET 9.0 and enables implicit usings and nullable reference types. Marking the project as not packable is appropriate for a test project.
10-14: Coverlet Package Reference is Set Up Appropriately.
The reference tocoverlet.collectorhas correctly configured<PrivateAssets>and<IncludeAssets>, ensuring it does not leak to downstream projects.
15-21: Unit Testing Packages Configuration Looks Solid.
The additional package references forMicrosoft.NET.Test.Sdk,Verify.Xunit,xunit, andxunit.runner.visualstudioare correctly specified with appropriate versions and asset protection where necessary.
28-30: Project Reference is Correctly Configured.
The new test project references the main library via<ProjectReference>, ensuring that tests will run against the current implementation ofGotenberg.Sharp.Api.Client.Gotenberg.Sharp.Api.Client.Tests/Domain/Builders/HtmlRequestBuilderTests.ToApiRequestMessage_PaperDimensionWithDecimal_NumbersUsesDotAsDecimalSeparator_Test.verified.txt (6)
1-4: Scale Parameter is Correctly Specified.
The test file begins with the expected headers and a value of1for thescaleparameter. This is appropriate and in line with test expectations.
5-8: Paper Width Uses Dot as Decimal Separator.
ThepaperWidthvalue of8.5inclearly uses a dot as the decimal separator, which meets the requirements imposed by the Gotenberg service.
9-12: Paper Height Uses Dot as Decimal Separator.
ThepaperHeightvalue of12.2inconfirms the correct formatting for decimal numbers. This ensures that the system’s culture settings do not lead to commas in the representation.
13-20: Margin Values are Uniform and Correct.
The margins (marginTop,marginBottom,marginLeft,marginRight) are consistently set to1in, which is appropriate for maintaining uniformity in the document layout.
21-28: Boolean Parameters are Explicitly Defined.
Parameters such aslandscape(False),preferCssPageSize(False),printBackground(False),omitBackground(False),generateDocumentOutline(False),failOnConsoleExceptions(False), andskipNetworkIdleEvent(False) are clearly specified. This explicitness ensures that the API request is built with the expected configuration.
29-61: HTML File Content is Minimal Yet Sufficient.
The inclusion of a simple HTML snippet (<h1>Hello</h1>) for the file upload portion serves well as a test fixture. It confirms that the builder can handle multipart form-data with both text and HTML content appropriately..editorconfig (1)
79-89: New Formatting Rules for Compound ExtensionsThe newly added section
# Verify settings [*.{received,verified}.{json,txt,xml}] charset = "utf-8-bom" end_of_line = lf indent_size = unset indent_style = unset insert_final_newline = false tab_width = unset trim_trailing_whitespace = falseclearly defines formatting rules for files that follow a compound extension pattern (for example,
file.received.jsonorfile.verified.xml). These rules are set to enforce consistency and help avoid culture-dependent issues such as the decimal separator problem, which is especially relevant when interfacing with services like Gotenberg.One thing to verify is whether the selected glob pattern (
[*.{received,verified}.{json,txt,xml}]) matches exactly the intended files. For example, if you also need to cover files with a single extension like.json(without a preceding identifier like "received" or "verified"), you might need to add extra sections or adjust the pattern. If this specificity is by design, the configuration is already well-structured..gitignore (1)
291-293: New Ignore Patterns for Received FilesThe added ignore entries:
# Verify *.received.* *.received/ensure that files (and directories) with a
".received"marker are not tracked by version control. This matches the intended approach to ignore files that are likely generated by external processes (such as Gotenberg outputs).Please verify that these patterns cover all cases you intend to ignore—for instance, files with compound extensions like
document.received.jsonand directories ending with.received. Also, check if case-sensitivity or any further variations need to be accounted for.
chrisbbe
changed the title
Gotenberg do not support decimal numbers with comma decimal separator.
Summary by CodeRabbit
Chores
Refactor
Tests
These updates enhance overall consistency and reliability while ensuring uniform behavior in document processing.