elucidsoft / dotnet-stellar-sdk

Stellar API SDK for .NET 6.x
Apache License 2.0
116 stars 55 forks source link

Format code using official dotnet-format tool #302

Closed leighmcculloch closed 3 years ago

leighmcculloch commented 3 years ago

Today I was experimenting with this SDK and regenerating code using xdrgen and I discovered that the tools I have do not format the .cs files in this repo in the same way that they are currently formatted. I tried a few tools, VSCode, dotnet-format, Visual Studio for Mac, and they didn't format them identical.

This is not particularly problematic for code that is purely edited by hand, but since there are many files generated by XDR it is difficult to regenerate those files then format them according to the norms that I see the maintainers are using.

Would it be possible to standardize on using the official dotnet-format tool to format code? It supports configuration so it can be configured to format code in a way that is pleasing to you. It's default formater seems pretty close, but not identical, to what is already in this repo.

leighmcculloch commented 3 years ago

As an example of what it would look like to reformat the code using the official dotnet-format tool, I opened #303.

fracek commented 3 years ago

I merged that PR. Before we close this issue I think it would be helpful to add:

leighmcculloch commented 3 years ago

on my machine I get some whitespace issues

@fracek The .editorconfig has a options for configuring the carriage return and new line characters. That might be what is needed.

end_of_line = crlf
insert_final_newline = false

Ref: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/code-style-rule-options?view=vs-2019

leighmcculloch commented 3 years ago

Another issue I have noticed that seems a little similar to formatting, although not, is that when I run the xdrgen it adds using System; to all the files. I'm guessing your IDEs are removing them automatically as unnecessary imports. Is the using System; something that should be removed from xdrgen csharp files generated, or should they be kept?

elucidsoft commented 3 years ago

using System; is usually kept.

fracek commented 3 years ago

@fracek The .editorconfig has a options for configuring the carriage return and new line characters. That might be what is needed.

Looks like the problem is with the character set that is not always utf-8.

leighmcculloch commented 3 years ago

using System; is usually kept.

@elucidsoft Thanks for confirming. I regenerated the .cs files from the .x files currently in the repo and reformatted, and it results in a diff re-adding using System; to most of the generated files. I've opened a PR with that diff since you confirmed the using System; is usually kept. https://github.com/elucidsoft/dotnet-stellar-sdk/pull/305

leighmcculloch commented 3 years ago

Looks like the problem is with the character set that is not always utf-8.

@fracek Ah you're right. Good catch. When I regenerate using xdrgen and format many of the files are changed from UTF-8 to us-ascii. I see other files in the repo that aren't generated that are us-ascii too. I'm not sure what you prefer, but https://github.com/elucidsoft/dotnet-stellar-sdk/pull/305 changes the generated files to us-ascii matching what xdrgen outputs.

fracek commented 3 years ago

Closing since we now check for code formatting as part of the CI process.