[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2020-06-07 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment. @MyDeveloperDay I don't have commit rights, could you land this change for me please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71512/new/ https://reviews.llvm.org/D71512 ___ cfe-commits mailing list cfe-commits

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2020-03-28 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska updated this revision to Diff 253366. Bouska added a comment. Do not touch current tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71512/new/ https://reviews.llvm.org/D71512 Files: clang/lib/Format/UnwrappedLineFormatter.cpp clang/unittests/Format/FormatTest.cpp Index

[PATCH] D73354: clang-format: insert trailing commas into containers.

2020-01-24 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment. I have an issue with this change. Currently (at least for C++), the presence of a trailing comma is used as a formatting hint to put all the element in one line or one per line as below: enum test1 = {ONE, TWO, THREE}; enum test2 = { ONE, TWO, TH

[PATCH] D73354: clang-format: insert trailing commas into containers.

2020-01-24 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73354/new/ https://reviews.llvm.org/D73354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mai

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2020-01-20 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska marked an inline comment as done. Bouska added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:662 "{\n" - " ff();\n" "}", MyDeveloperDay

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2020-01-20 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska marked an inline comment as done. Bouska added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:344 - return MergedLines; -} // Don't merge block with left brace wrapped after ObjC special blocks MyDeveloperDay wrote:

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2020-01-11 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment. Ping @MyDeveloperDay @klimek CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71512/new/ https://reviews.llvm.org/D71512 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2020-01-04 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska updated this revision to Diff 236188. Bouska added a comment. Updated unittest to check under the column limit and over the column limit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71512/new/ https://reviews.llvm.org/D71512 Files: clang/lib/Format/UnwrappedLineFormatter.cpp

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-28 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment. I'm not sure creating a special case for else/catch is the best idea. I would seem better to change the control statement @ line 309 to add it the case where we break before else or catch (add a new || test in the middle condition). I'm really new to this code too, so I

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment. Is there a reason why this option adds space before and after the tok::arrow token but not the tok::arrowstar token? That seems inconsistent to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71659/new/ https://reviews.ll

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska requested changes to this revision. Bouska added a comment. This revision now requires changes to proceed. So, you are trying to fix the issue at the wrong place. Contrary from what we should expect from a UnwrappedLine, BWACS_Multiline works by always wrapping the brace in UnwrappedLineP

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added a comment. Your code still breaks MultiLine [ RUN ] FormatTest.MultiLineControlStatements /home/pablomg/dev/llvm-project/clang/unittests/Format/FormatTest.cpp:1562: Failure Expected: "try {\n" " foo();\n" "} catch (\n" "Exception &bar)\n" "{\n" " baz();\n" "}

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska updated this revision to Diff 235435. Bouska added a comment. Set line length to column limit + 1 (41) in tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71512/new/ https://reviews.llvm.org/D71512 Files: clang/lib/Format/UnwrappedLineFormatter.cpp clang/unittests/Format/

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2019-12-21 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:586 verifyFormat("if (true) {\n" - " ff();\n" "}", MyDeveloperDay wrote: > any reason you are cha

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2019-12-14 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska created this revision. Bouska added reviewers: djasper, klimek, krasimir. Bouska added a project: clang-format. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch fixes bug #44192 When clang-format is run with option AllowShortBlocksOnASingleLine, it is expe