[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:2290
 
+**SpacesAroundConditions** (``bool``)
+  If ``true``, spaces will be inserted around if/for/while (and similar) conditions.
----------------
`SpacesInConditionalStatement` is probably a better name, remaining consistent with existing `SpacesIn*` options, as well as indicating it only affects the entire conditional statement, and not individual conditions within the statement.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2511
+      return t->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
+                        tok::kw_switch, tok::kw_constexpr, TT_ForEachMacro);
+    };
----------------
It seems that you are mixing statement tokens `if`, `while`, etc. with preprocessor and macro tokens. Going by the documentation, this is unexpected.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2919
+  if (Right.is(tok::coloncolon) &&
+      !Left.isOneOf(tok::l_brace, tok::comment, tok::l_paren))
     return (Left.is(TT_TemplateOpener) &&
----------------
MyDeveloperDay wrote:
> I'm not sure I understand this change so you added a `tok::l_paren` here? but its not just for your style, so something else must have changed. Did you run all FormatTests?
> 
> one of the tests you add need to test why you added this here
This is not covered by your tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346