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

[clang-format] New AfterEnum brace wrapping changes have cause C# behaviour to change #50982

Closed
mydeveloperday opened this issue Aug 27, 2021 · 9 comments
Labels
bugzilla Issues migrated from bugzilla clang-format

Comments

@mydeveloperday
Copy link
Contributor

Bugzilla Link 51640
Resolution FIXED
Resolved on Oct 20, 2021 20:51
Version trunk
OS Windows NT
Blocks #50580 #51489
CC @mydeveloperday,@tstellar
Fixed by commit(s) ed367b9 a797306

Extended Description

in 13.0 and 14.0 AfterEnum brace wrapping is now different in how Enums are modified (when they have an access modifier)

Enumerators with access modifiers don't follow the BraceWrapping rules


Language: CSharp
AllowShortBlocksOnASingleLine: false
AllowShortEnumsOnASingleLine: false
BreakBeforeBraces: Custom
BraceWrapping:
AfterEnum: true

enum A
{
A,
B
}

internal enum A {
A,
B
}

public enum A {
A,
B
}

@mydeveloperday
Copy link
Contributor Author

@tstellar
Copy link
Collaborator

tstellar commented Sep 2, 2021

This doesn't apply cleanly to the release branch, can you provide a patch or a git branch with the patch applied.

@tstellar
Copy link
Collaborator

We will need an updated patch by Monday Sep. 13 or else this fix will risk missing the release.

@mydeveloperday
Copy link
Contributor Author

@​Tom (as of Oct 12th post 13.0.0 release) if I want to get this into 13.0.1 which branch should I be committing to?

I assume release/13.x, is that correct?

$ git branch -a
main

  • release/13.x

@tstellar
Copy link
Collaborator

Can you attach a patch to this bug? I'll do the backporting.

@mydeveloperday
Copy link
Contributor Author

Patch against 13.0.0 branch
I've merged from main to releases/13.x what is needed to fix this regression and ensure the unit tests pass without issue (FormatTests)

This should merge cleanly.

This is a full context diff (as I would use for a review) if you need one with a little less context let me know.

@mydeveloperday
Copy link
Contributor Author

Smaller patch with -U4 for completeness
Adding smaller patch for completeness

@tstellar
Copy link
Collaborator

Merged: a797306

@tstellar
Copy link
Collaborator

mentioned in issue #51489

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang-format
Projects
None yet
Development

No branches or pull requests

2 participants