microsoft / winget-create

The Windows Package Manager Manifest Creator command-line tool (aka wingetcreate)
MIT License
503 stars 85 forks source link

Add custom YamlDotNet emitter that makes all fields with newlines use literal style. #281

Closed jedieaston closed 2 years ago

jedieaston commented 2 years ago

Resolves #280.

This PR adds a custom emitter for YamlDotNet where when it is serializing to YAML, it will make sure all fields with a newline character are set to use ScalarStyle.Literal, which doesn't add newlines to our newlines. Usually, you would just add this to the YamlMember for the property, but since we have a ton of properties that can have newlines, adding partials for all of them seemed to be way harder to support than this. It turns out someone on Stack Overflow had the exact same problem, so I stole borrowed their solution :)

(It also appears that Rider has decided to reformat this file a lot. I can put it back if needed, but since it's just interpreting the existing .editorconfig, maybe you want to keep it?)

Tested: manually, although I'm happy to write tests if needed.

Microsoft Reviewers: Open in CodeFlow
ryfu-msft commented 2 years ago

/azp run

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 1 pipeline(s).
ryfu-msft commented 2 years ago

@jedieaston, thank you so much for this contribution!

I think a unit test to verify this behavior is needed. I think a good place to do that would be here in CharacterValidation.cs. You can follow a similar approach to the unit test that's already there and then use File.ReadAllText to acquire the text of the generated test manifest and compare the text to verify that the output is as expected.

jedieaston commented 2 years ago

I added a unit test (sorry for the delay, was busy this weekend). It's a bit weird though, as \n, \r\n, and \x85 are all replaced by \n by YamlDotNet but \x2028 and \x2029 aren't.

ryfu-msft commented 2 years ago

/azp run

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 1 pipeline(s).