OrchardCMS
Add CSharpier config
Ran dotnet csharpier . on the root folder of Orchard Core to reformat everything.
Adds Csharpier configs and VS Code settings for CSharpier.
Should it be part of the project? Does it mean someone will run the tool locally? I don't think I would like the CI to block my PR because I missed a space. (I know I don't like it when it happens in other repos, example ImageSharp).
Yes, we will need to run the tool locally to reformat everything at some point. We can also add a VS Code setting so that it be applied on saving the file automatically. I wanted to see if we would adopt it or not first.
Formatting with CSharpier will not be reformatted by Visual Studio afterward from what I've seen so far.
This pull request has merge conflicts. Please resolve those before requesting a review.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Yes I support this.
means no more opinions about what code style to be using. And less noise on prs, saying please format it like this.
I believe it can also be installed on the CI to autoformat and push back, which would be a good future improvement.
👍
I think it would be nice to create an action that runs it once a week to create a PR automatically.
Side question, why not dotnet-format. It's part of the SDK and it support the editorconfig file which already have our formatting preferences.
Would need to compare it with CSharpier. I believe it supports editorConfig too but maybe not the same way. Need to read documentation.
I have nothing against dotnet-format I did not even know it existed! Let's open a second draft PR to compare.
This pull request has merge conflicts. Please resolve those before requesting a review.
Ok, took a look and tested dotnet-format. The difference is that CSharpier allows to configure a max-length per line which is what we want to achieve (to be able to see code without scrolling left and right all the time).
See https://github.com/dotnet/format/blob/main/docs/Supported-.editorconfig-options.md#core-options
Though, dotnet-format will reorder the usings just like VS do when CTRL + K + D or CTRL + R + G which CSharpier doesn't do.
Also dotnet format was a lot more slower than CSharpier when doing a bulk update.
This will break every PR we have pending. It's the same reason why we have not yet converted all of our classes to use file-scope namespaces.
Also, CSharpier does not yet support c#12. Not sure if this is a problem or not for us.
Yes, seems like it has issues with Primary constructors. Does'nt remove the private readonly vars properly. Though we would need to start using it.
Also, dotnet-format on the counterpart doesn't do much line-length wrapping.
https://github.com/dotnet/format/issues/246
I am do not mind the values used. But, I do not want to break every PR as result. But, at some point we would have to decide when to close these old PR so we are able to make this kind of change easily.
you're not breaking every pr, just creating merge conflicts.
resolve the merge conflict from the other pr side, and save it, and it will reformat.
Maybe ... running the same tool/options only of the modified files in the PRs would fix the merge conflicts
This pull request has merge conflicts. Please resolve those before requesting a review.
Is there an option to remove the trailing comas when multi-lines is converted to single lines? ", }
I guess we would need to combine dotnet-format and CSharpier to get to a complete result with all these options.
yeah the point with CSharpier is that it's opiniated, so has very few options
I like this, although I got PTSD from the amount of files changed just by getting a glimpse of the number in the top-right tab :D. But we need it automated, otherwise, we'll need to redo it manually all the time. So, either reformat automatically on push from GitHub Actions as others mentioned, or run it with --check and fail the workflow if formatting is missing and require people to do that manually.
It seems that this pull request didn't really move for quite a while. Is this something you'd like to revisit any time soon or should we close? Please comment if you'd like to pick it up.
Closing this pull request because it has been stale for very long. If you think this is still relevant, feel free to reopen it.