[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

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

HazardyKnusperkeks added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
     ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
     backward compatibility.
 
----------------
crayroud wrote:
> MyDeveloperDay wrote:
> > Now I look back here, why where these Macro considered the same as for loops? why would we want
> > 
> > ```
> > for (....)
> > Q_FOREACH(...)
> > ```
> > 
> > So this really does need a struct or we'll be eventually be adding
> > 
> > `SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacrosButNotIfAndDefinatelyWhilesAndSometimesSwitchButOnlyOnTheSecondThursdayOfTheMonth`
> > 
> > ```
> > SpaceBeforeParen:
> >      AfterFunctionDefinitionName: false
> >      AfterFunctionDeclarationName: true
> >      AfterSwitch: true
> >      AfterForeachMacros: false
> >      .... (there are likely others)
> > ```
> >      
> > `
> > 
> > 
> > 
> > 
> > 
> Indeed replacing the enum with a struct as suggested is better to support the different possible combinations, compare to the current version of SpaceBeforeParens that results in very long names.
> 
> To support existing configuration files, I propose to keep the enum and to add a struct to handle the custom use cases and to cleanup the code. What do you think ?
> 
> ```
> SpaceBeforeParens: Custom
> SpaceBeforeParensCustom:
>      AfterFunctionDefinitionName: false
>      AfterFunctionDeclarationName: true
>      AfterSwitch: true
>      AfterForeachMacros: false
>     …
> ```
I haven't looked too deep into the parsing, but if we could try to parse it as a struct and if that fails as enum for compatibility I would be in favor of that. But a `custom` is also acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833