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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/include/clang/Format/Format.h:1950
+  /// similar) conditions.
+  bool SpacesAroundConditions;
+
----------------
its position in the file and the documentation is not quite alphabetic, given you are close i'd make it so


================
Comment at: clang/include/clang/Format/Format.h:2088
            SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
+           SpacesAroundConditions == R.SpacesAroundConditions &&
            Standard == R.Standard && TabWidth == R.TabWidth &&
----------------
move upto just before SpacesBeforeTrailingComments 


================
Comment at: clang/lib/Format/Format.cpp:520
     IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
+    IO.mapOptional("SpacesAroundConditions", Style.SpacesAroundConditions);
     IO.mapOptional("Standard", Style.Standard);
----------------
Same here


================
Comment at: clang/lib/Format/Format.cpp:779
   LLVMStyle.SpacesInAngles = false;
+  LLVMStyle.SpacesAroundConditions = false;
 
----------------
Sorry move to line 771-772


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2509
+  if (Style.SpacesAroundConditions) {
+    const auto is_cond_kw = [&](const FormatToken *t) {
+      return t->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
----------------
this isn't really the normal naming convention for variables, I think it would be `IsCondKw`  I think its clearer without using abbreviations,

why not just make this a function? does the Lambda really give us anything here in such a big function other than clutter?

`static bool isConditionKeyword(....)`


================
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) &&
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68346