Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loops without braces #202

Closed
shocklateboy92 opened this issue May 16, 2021 · 2 comments · Fixed by #213
Closed

Loops without braces #202

shocklateboy92 opened this issue May 16, 2021 · 2 comments · Fixed by #213
Labels
area:formatting type:bug Something isn't working
Milestone

Comments

@shocklateboy92
Copy link
Collaborator

I had foreach loop in my code without braces

foreach (var c in word.Original)
    switch (c)
    {
        case '(':
        case '(':
            bracketCount++;
            break;
        case ')':
        case ')':
            bracketCount--;
            break;
    }

and csharpier did this to it:

foreach (var c in word.Original)switch (c)
{
    case '(':
    case '(':
        bracketCount++;
        break;
    case ')':
    case ')':
        bracketCount--;
        break;
}

Should we make csharpier deal with conditions/loops without braces? Or should we just have it add braces automatically?

@belav
Copy link
Owner

belav commented May 17, 2021

I see the options as

  1. Add braces automatically
  2. Add an option to add braces automatically - which kind of goes against the no options philosophy
  3. Don't add braces - for people that do want to enforce braces they could use style cop and it looks like this is what prettier does

Prettier has had requests to add an option prettier/prettier#7659
Which lead me to https://prettier.io/docs/en/rationale.html#what-prettier-is-not-concerned-about

Prettier only prints code. It does not transform it. This is to limit the scope of Prettier. Let’s focus on the printing and do it really well!
Here are a few examples of things that are out of scope for Prettier:

  • Adding/removing {} and return where they are optional.

Which has convinced me that not adding braces is probably the way to go. Also if we start adding braces then the CSharpier validation process needs to take that into account or it will report the new braces as a failure.

@shocklateboy92
Copy link
Collaborator Author

shocklateboy92 commented May 17, 2021 via email

@belav belav added this to the 0.9.4 milestone May 18, 2021
@belav belav added type:bug Something isn't working area:formatting labels May 18, 2021
belav added a commit that referenced this issue May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:formatting type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants