[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2018-12-01 Thread Reuben Thomas via Phabricator via cfe-commits
reuk created this revision. reuk added reviewers: Typz, krasimir, cfe-commits. This patch aims to add support for the following rules from the JUCE coding standards: - Always put a space before an open parenthesis that contains text - e.g. foo (123); - Never put a space before an empty pair of

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2018-12-10 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a subscriber: klimek. reuk added a comment. Would someone review this please? I'm not sure who to add for review (sorry), maybe one of the following? @klimek @Typz @krasimir Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-08 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 189942. reuk added a comment. I've rebased onto master, and removed unrelated formatting changes. I've also tried to remove some of the duplicate parens-related expressions. I agree that the heavy nested boolean expressions are a bit painful to read, but I'm n

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-09 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment. Thanks for the review, and for approving this PR. It's very much appreciated! I don't have merge rights - could someone commit this for me please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 _

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-10 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment. Does it make sense to allow `AllowShortIfElseStatementsOnASingleLine` to be enabled when `AllowShortIfStatementsOnASingleLine` is not? Perhaps it would be better to have `AllowShortIfStatementsOnASingleLine` be represented by an enum with values `Never`, `WithoutElse`, `Al

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-11 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment. This looks much better now, thanks for taking another look! I've flagged some formatting/spelling nits, and I think the tests/docs could be a little more explicit about the behaviour of `WithoutElse`. Other than that, looks great! Comment at: clang/docs/

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-11 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment. The code looks good now. There's just a few places left where the formatting looks a bit suspect (sorry for missing those last time). I think once that's fixed this will be good to go. Comment at: clang/unittests/Format/FormatTest.cpp:553 - AllowSimpl

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-12 Thread Reuben Thomas via Phabricator via cfe-commits
reuk accepted this revision. reuk added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59087/new/ https://reviews.llvm.org/D59087 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-15 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment. @MyDeveloperDay I'm not sure you built this branch. Perhaps you applied this patch to an older version of the repo. For me, SpaceBeforeCpp11BracedListOptions is defined at Format.h:1570. This builds fine for me, on macOS 10.14 with Xcode 10.1's clang. I just rebased onto

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-17 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 191015. reuk added a comment. @MyDeveloperDay I'm sorry, you're absolutely correct. I'd got some other JUCE-related changes mixed up in this PR. Should be fixed now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-17 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 191050. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp lib/Format/TokenAnnotator.h unittests/Format/FormatTest.c

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-18 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment. @klimek I agree that the rule is somewhat arbitrary. However, it's the style rule of an established codebase with many users (I don't have a concrete number, but the project has 1400 stars on github ). I've found this patch useful when c

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-20 Thread Reuben Thomas via Phabricator via cfe-commits
reuk marked an inline comment as done. reuk added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2546-2560 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) || (Style.SpaceBeforeParens != FormatStyle::SBPO_Never && -(Left.isOneOf(tok::

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-20 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 191560. reuk added a comment. Removed unnecessary parens. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp lib/For

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-30 Thread Reuben Thomas via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL357344: [clang-format]: Add NonEmptyParentheses spacing option (authored by reuk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-05 Thread Reuben Thomas via Phabricator via cfe-commits
reuk created this revision. reuk added reviewers: MyDeveloperDay, klimek. reuk added a project: clang-tools-extra. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch aims to support the following rule from the Juce coding standards

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-05 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 193889. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 Files: clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp I

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-07 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 194055. reuk added a comment. Updated with fixes. Thanks for pointing out that the options are ordered alphabetically, I hadn't noticed that! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 Files: clang/docs

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-07 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added a comment. I updated `ClangFormatStyleOptions.rst` by hand, is there a way to do that automatically instead (for future patches)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 ___ cfe-c

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-07 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 194057. reuk added a comment. Fixed formatting nit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/Forma

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

2019-04-08 Thread Reuben Thomas via Phabricator via cfe-commits
reuk closed this revision. reuk added a comment. Closed by https://reviews.llvm.org/rG91f60b44958f, https://reviews.llvm.org/rL357908, https://reviews.llvm.org/rC357908 (sorry, I forgot to update the body of the commit message to close this automatically) CHANGES SINCE LAST ACTION https://re

[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

2019-04-09 Thread Reuben Thomas via Phabricator via cfe-commits
reuk added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009 + // classes case + if (Style.AccessModifierIndentation && Line->Level % 2 == 0) +--Line->Level; klimek wrote: > What if the class starts at level 1? (for example, inside

[PATCH] D60726: Fixed -Wconversion-null warning in GCC.

2019-04-15 Thread Reuben Thomas via Phabricator via cfe-commits
reuk accepted this revision. reuk added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60726/new/ https://reviews.llvm.org/D60726 ___ cfe-commits mailin

[PATCH] D60726: Fixed -Wconversion-null warning in GCC.

2019-04-15 Thread Reuben Thomas via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL358441: [clang-format] Fix -Wconversion-null warning in GCC (authored by reuk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev