[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

2021-06-10 Thread Darwin Xu via Phabricator via cfe-commits
darwin created this revision. darwin added a project: clang-format. darwin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This a bug fix of https://bugs.llvm.org/show_bug.cgi?id=50116 First revision only contains the change of the test co

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

2021-06-10 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. Sorry, I may need some help here. It shows "Context not available.", how do I correct it? Repository: rZORG LLVM Github Zorg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104044/new/ https://reviews.llvm.org/D104044 __

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

2021-06-10 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D104044#2811268 , @HazardyKnusperkeks wrote: > In D104044#2810909 , @darwin wrote: > >> Sorry, I may need some help here. It shows "Context not available.", how do >> I correct it? > >

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

2021-06-11 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D104044#2812612 , @MyDeveloperDay wrote: > Just so I'm clear are you proposing that a newlines should always be added or > that single blank lines should be ignored? I can't tell if the bug is that > the line isn't being remo

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

2021-06-11 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D104044#2813491 , @MyDeveloperDay wrote: > Devils advocate how is this any different from > > class Foo { > > class Bar {} ; > } > > }; > > This would become > > class Foo { > class Bar {}; > }; > > i.e.

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

2021-06-13 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D104044#2813515 , @MyDeveloperDay wrote: > Do we need a set options for when we want to insert/retain/add a newline > after various constructs? frankly I've been mulling over the concept of > adding a > > NewLinesBetweenFun

[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

2021-06-13 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D104044#2814016 , @HazardyKnusperkeks wrote: > Going the full way, to fix the number of empty lines after/before/between > elements would be real nice. But even nicer would be if you can state a range. > > But I think all thos

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-15 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D104044#2816397 , @MyDeveloperDay wrote: > I think we can agree its complicated, how about you take your unit tests and > show us what the "code change" looks like to fix the bug > > If that gets overly convoluted then perhaps

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-19 Thread Darwin Xu via Phabricator via cfe-commits
darwin updated this revision to Diff 353200. darwin added a comment. Updated with the fix code. Look into the code, I am pretty sure this is a bug, it is about the logic of the parameter `KeepEmptyLinesAtTheStartOfBlocks`. When `KeepEmptyLinesAtTheStartOfBlocks` is false, it will remove the emp

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-21 Thread Darwin Xu via Phabricator via cfe-commits
darwin marked an inline comment as done. darwin added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:281 + CustomStyle)); + EXPECT_EQ("/* something */ namespace N\n" +"{\n" MyDeveloperDay wrote: > What does > >

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-21 Thread Darwin Xu via Phabricator via cfe-commits
darwin marked 2 inline comments as done. darwin added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:280 + "}", + CustomStyle)); + EXPECT_EQ("/* something */ namespace N\n" MyDeveloperDay wrote: > I'm not s

[PATCH] D97137: Bug fix of https://bugs.llvm.org/show_bug.cgi?id=49175

2021-02-20 Thread Darwin Xu via Phabricator via cfe-commits
darwin created this revision. darwin added a reviewer: clang-format. darwin added a project: clang-format. darwin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The AlignConsecutiveDeclarations option doesn't handle pointer properly: The

[PATCH] D97137: Bug fix of https://bugs.llvm.org/show_bug.cgi?id=49175

2021-02-21 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D97137#2577794 , @HazardyKnusperkeks wrote: > Just out of curiosity, in which language is `Const` used? For example it is C's macro. I found this issue in our C code. It has many macros and one function was defined like this:

[PATCH] D97137: Bug fix of clang-format, the AlignConsecutiveDeclarations option doesn't handle pointer properly

2021-02-21 Thread Darwin Xu via Phabricator via cfe-commits
darwin added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:716-717 break; if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName, tok::kw_operator)) return false; Maybe

[PATCH] D97137: Bug fix of clang-format, the AlignConsecutiveDeclarations option doesn't handle pointer properly

2021-02-22 Thread Darwin Xu via Phabricator via cfe-commits
darwin updated this revision to Diff 325455. darwin added a comment. Add more test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97137/new/ https://reviews.llvm.org/D97137 Files: clang/lib/Format/WhitespaceManager.cpp clang/unittests/Fo

[PATCH] D97137: Bug fix of clang-format, the AlignConsecutiveDeclarations option doesn't handle pointer properly

2021-02-22 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D97137#2579669 , @HazardyKnusperkeks wrote: > You should mark comments as done, if they are. > > Does your modification maybe add something to the alignment which is not a > declaration? > > int a; > double b; > a * b; >

[PATCH] D97137: Bug fix of clang-format, the AlignConsecutiveDeclarations option doesn't handle pointer properly

2021-02-23 Thread Darwin Xu via Phabricator via cfe-commits
darwin updated this revision to Diff 325766. darwin marked 3 inline comments as done. darwin added a comment. Fix the code's alignment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97137/new/ https://reviews.llvm.org/D97137 Files: clang/lib/For

[PATCH] D97137: Bug fix of clang-format, the AlignConsecutiveDeclarations option doesn't handle pointer properly

2021-02-24 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. Hi guys, thanks for accepting the change. But I don't have have commit access of LLVM. Can someone commit it for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97137/new/ https://reviews.llvm.org/D97137

[PATCH] D97137: [clang-format] Fix AlignConsecutiveDeclarations handling of pointers

2021-02-25 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D97137#2587075 , @HazardyKnusperkeks wrote: > In D97137#2584417 , @darwin wrote: > >> Hi guys, thanks for accepting the change. But I don't have commit access of >> LLVM. Can someone com

[PATCH] D104900: [clang-format] PR50525 doesn't handle AlignConsecutiveAssignments correctly in some situations

2021-06-26 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. Sorry I haven't had a chance to look at this bug before it has closed. But I do have question: I observed that this code are formatted incorrectly by the same config: llvm::Optional CurrentCode = None; autoEnv = std::make_unique(Code,

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-26 Thread Darwin Xu via Phabricator via cfe-commits
darwin updated this revision to Diff 354696. darwin marked an inline comment as done. darwin added a comment. Add new test cases according to the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104044/new/ https://reviews.llvm.org/D104044 Files: clang/lib/Format/UnwrappedLine

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-26 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. And can someone commit it for me? I don't have the right to push it yet. Or let me know how to apply? Thank you very much. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104044/new/ https://reviews.llvm.org/D104044 ___

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-26 Thread Darwin Xu via Phabricator via cfe-commits
darwin added a comment. In D104044#2842757 , @HazardyKnusperkeks wrote: > In D104044#2842726 , @darwin wrote: > >> And can someone commit it for me? I don't have the right to push it yet. Or >> let me know how t

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-28 Thread Darwin Xu via Phabricator via cfe-commits
darwin marked an inline comment as done. darwin added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1270 !startsExternCBlock(*PreviousLine)) Newlines = 1; MyDeveloperDay wrote: > Nit:I do think at some point we need to hav