[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add flags

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.


================
Comment at: clang/include/clang/Format/Format.h:3341
+    /// \endcode
+    bool AfterFunctionDeclarationName;
+    /// If ``true``, put a space between function definition name and opening
----------------
Could you sort these? AfterControlStatements e.g. would be in front of this.


================
Comment at: clang/include/clang/Format/Format.h:3361
+    /// \endcode
+    bool AfterOperators;
+    /// If ``true``, put space between if macros and opening parentheses.
----------------
Should `new` be handled differently than the other operators? But at least add an operator example which is not `new`, and keep this one.


================
Comment at: clang/include/clang/Format/Format.h:3405
+  ///
+  /// If ``SpaceBeforeParens`` is set to ``SBPO_Custom``, use this to specify
+  /// how each individual space before parentheses case should be handled.
----------------
Custom


================
Comment at: clang/include/clang/Format/Format.h:3416
+  /// \version 14
+  SpaceBeforeParensCustom SpaceBeforeParensFlags;
+
----------------
MyDeveloperDay wrote:
> I'm not a massive fan of the word `Flags` here and thoughts?
Yeah, neither, but Options is taken.

But Flags indicate that these will always be booleans, and we know that may change in the future.

Is it possible the reuse SpaceBeforeParensOptions as struct and still parse the old enum? (Yes of course, but is it feasible in terms of needed work?)


================
Comment at: clang/include/clang/Format/Format.h:3422
   ///    true:                                  false:
-  ///    for (auto v : values) {}       vs.     for(auto v: values) {}
+  ///    for (auto v : values) {}       vs.     for (auto v: values) {}
   /// \endcode
----------------
Please remove again. :)


================
Comment at: clang/lib/Format/Format.cpp:1221
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  LLVMStyle.SpaceBeforeParensFlags.AfterControlStatements = true;
+  LLVMStyle.SpaceBeforeParensFlags.AfterOperators = true;
----------------
Is this needed? Shouldn't the expand take care of that?


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

https://reviews.llvm.org/D110833