[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb40e9dce0a67: [clang-format] Avoid breaking )( with BlockIndent (authored by gedare, committed by owenpan). Repository: rG LLVM Github Monorepo C

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added a comment. In D137762#3969770 , @owenpan wrote: > @gedare do you need us to commit this patch on your behalf (using "Gedare > Bloom ")? Yes, please. I don't have any llvm permissions, this is my first patch set for llvm actually. Anything

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

owenpan added a comment. @gedare do you need us to commit this patch on your behalf (using "Gedare Bloom ")? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 __

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

owenpan accepted this revision. owenpan added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added a comment. I have changed the code as recommended by @owenpan with new unit tests passing, and reverting the modifications to existing unit tests. Someone else may janitor the EXPECT_EQ blocks separately. I will not be able to look at this again for ~2 weeks. with luck it is ready

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 476196. gedare added a comment. - fixup: use simpler logic for handling rparens - Revert "fixup: port other EXPECT_EQ to verifyFormat" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.ll

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5007 return false; } const FormatToken *Previous = Right.MatchingParen->Previous; owenpan wrote: > Can we simply add the above and leave the original `return` statement

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

owenpan added a comment. @gedare thanks for changing `EXPECT_EQ` to `verifyFormat`, but IMO we should do that in another patch so that it would be easier (at least for me) to review the new tests and to make sure no existing tests have been changed by accident. Comment at: cl

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Thanks for your patience. Please wait for @MyDeveloperDay . Comment at: clang/unittests/Format/FormatTest.cpp:7215 Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign; - EXPECT_EQ(StringRef(

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7215 Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign; - EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n" - "paramC);\n" - "void functionD

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 475313. gedare added a comment. - FormatTest: restore Input variable to one test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/Continuation

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7215 Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign; - EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n" - "paramC);\n" - "vo

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 475165. gedare added a comment. - fixup: unit test formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/ContinuationIndenter.cpp clan

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 475164. gedare added a comment. - fixup: replace EXPECT_EQ with verifyFormat - fixup: port other EXPECT_EQ to verifyFormat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D1377

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5006-5010 +return !((Previous && (Previous->is(tok::kw_for) || Previous->isIf())) || + (Right.Next && + (Right.Next->is(tok::l_paren) || + (Right.N

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare marked an inline comment as done. gedare added a comment. I've added unit tests and confirmed (1) the new tests fail on current `main` and (2) all unit tests pass with this revision applied. [100%] Running lit suite /llvm-project/clang/test/Unit Testing Time: 250.49s Skipped:

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 474901. gedare added a comment. - fixup typo in unit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/ContinuationIndenter.cpp clang/li

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 474881. gedare added a comment. Refactor horrible to read logic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/ContinuationIndenter.cpp cl

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 474876. gedare added a comment. add unit tests for BlockIndent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/ContinuationIndenter.cpp cla

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare marked 2 inline comments as done. gedare added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5006-5010 +return !((Previous && (Previous->is(tok::kw_for) || Previous->isIf())) || + (Right.Next && + (Right.Next->is(tok::l_paren

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

HazardyKnusperkeks added a comment. In D137762#3919092 , @MyDeveloperDay wrote: > Definitely needs tests Could you please add tests in the `clang/unittests/Format/FormatTest.cpp` file? Comment at: clang/lib/Format/TokenAnnotator.cpp:

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 474814. gedare added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/ContinuationIndenter.cpp clang/lib/

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 474812. gedare added a comment. Run tests and fix bad breaks. Fix comment style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/TokenAnnotat

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:720 (!Previous.Previous || !Previous.Previous->isOneOf( - tok::kw_for, tok::kw_while, tok::kw_switch)) && + tok::r_paren, tok::kw_for, tok

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

MyDeveloperDay added a comment. Definitely needs tests Comment at: clang/lib/Format/ContinuationIndenter.cpp:720 (!Previous.Previous || !Previous.Previous->isOneOf( - tok::kw_for, tok::kw_while, tok::kw_switch)) && + tok:

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

rymiel added a comment. This patch will probably need unit tests Comment at: clang/lib/Format/TokenAnnotator.cpp:5004-5005 const FormatToken *Previous = Right.MatchingParen->Previous; -return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf())); +// avoid

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added a comment. Another test case for function pointers is here: F25259090: tmp.c Before applying these changes, it is: clang-format -style="{BasedOnStyle: LLVM, AlignAfterOpenBracket: BlockIndent}" tmp.c int foo(long l) { return l; } int m

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added a comment. A test case from the GitHub Issue https://github.com/llvm/llvm-project/issues/57250 is attached here: F25258989: test.c When run without these changes, it yields: clang-format -style="{BasedOnStyle: LLVM, AlignAfterOpenBracket: BlockI

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 474430. gedare added a comment. Add a fix to enable breaking for continuation after a )( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format/Cont

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare updated this revision to Diff 474429. gedare added a comment. Allow line breaks in a continuation with BlockIndent after )( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 Files: clang/lib/Format

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare added a comment. Something about this is still a bit off though. This change seems to prevent any more breaking on the line. For example, the test example given in GitHub Issue https://github.com/llvm/llvm-project/issues/57250 when formatted with this change becomes: `clang-format -styl

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

gedare created this revision. Herald added a project: All. gedare requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The BracketAlignmentStyle BAS_BlockIndent was forcing breaks before a closing right parenthesis yielding strange-looking resul