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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
     ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
     backward compatibility.
 
----------------
HazardyKnusperkeks wrote:
> 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.
If possible, we should deprecate the enum values and match them to the new struct values for backward compatibility without resorting to a `Custom` sub-option. See `PackConstructorInitializers` for an example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833