Add new line to be specified for JSON formatting#100890

martincostello · 2024-04-10T18:40:36Z

Allow the new line string to use for indented JSON to be specified through options.

Resolves #84117.

Allow the new line string to use for indented JSON to be specified through options. Resolves dotnet#84117.

ghost · 2024-04-10T18:40:43Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

martincostello · 2024-04-10T18:41:48Z

@eiriktsarpalis I've left this in draft for now before I dig around tomorrow and see if I've missed anything, but feel free to start reviewing at your convenience.

src/libraries/System.Text.Json/src/Resources/Strings.resx

src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonWriterOptionsTests.cs

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs

src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs

martincostello · 2024-04-11T08:52:08Z

Failing test is EnsureConsoleLoggerOptions_ConfigureOptions_SupportsAllProperties() - I need to update the JSON console logger options to account for the new property.

- Cater for `_newLine` in JsonSerializerOptions caching. - Lazily initialize field. - Allow null to reset to default. - Add assertions. - Add/update comments. - Use `nameof()`. - Remove redundant field. - Extend tests.

- Update property count to fix assertion. - Update test to validate `NewLine` can be set/bound.

Only normalize the line endings if the `JsonWriterOptions` are not using the defaults.

src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs

...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs

martincostello · 2024-04-11T12:23:19Z

I'm not sure why the Mono tests are failing - there's something in the logs about how some of the tests take a long time to run under the interpreter, so I wonder if the extra test cases I've added to the combinations for NewLine have tipped it over the edge for the 1 hour timeout?

...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs

- Access lazily initialized field through property. - Update hash code assertion.

Use similar format string to `Format_InvalidGuidFormatSpecification`/

...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs

src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterOptions.cs

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OptionsTests.cs

martincostello · 2024-04-11T15:58:33Z

I'm not sure why the Mono tests are failing

Looks like these are known issues.

Reword comment as suggested.

- Simplify condition. - Disallow null for `string NewLine` properties.

eiriktsarpalis · 2024-04-12T13:59:48Z

I'm not sure why the Mono tests are failing

Looks like these are known issues.

We generally use the "Build Analysis" leg to determine if there any test failures that haven't been flagged as known issues.

eiriktsarpalis

Thanks!

* Add new line to be specified for JSON formatting Allow the new line string to use for indented JSON to be specified through options. Resolves dotnet#84117. * Address review feedback - Cater for `_newLine` in JsonSerializerOptions caching. - Lazily initialize field. - Allow null to reset to default. - Add assertions. - Add/update comments. - Use `nameof()`. - Remove redundant field. - Extend tests. * Update Logging.Console tests - Update property count to fix assertion. - Update test to validate `NewLine` can be set/bound. * Only normalize line endings if needed Only normalize the line endings if the `JsonWriterOptions` are not using the defaults. * Address feedback - Access lazily initialized field through property. - Update hash code assertion. * Update exception message Use similar format string to `Format_InvalidGuidFormatSpecification`/ * Address feedback Reword comment as suggested. * Address feedback - Simplify condition. - Disallow null for `string NewLine` properties.

Add new line to be specified for JSON formatting

839c642

Allow the new line string to use for indented JSON to be specified through options. Resolves dotnet#84117.

ghost added area-System.Text.Json new-api-needs-documentation labels Apr 10, 2024

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 10, 2024