[PATCH] D33029: [clang-format] add option for dangling parenthesis

[Ryan Stringham via Phabricator via cfe-commits]

stringham added inline comments.


================
Comment at: include/clang/Format/Format.h:793
+  /// \endcode
+  bool DanglingParenthesis;
+
----------------
djasper wrote:
> stringham wrote:
> > djasper wrote:
> > > I don't think this is a name that anyone will intuitively understand. I understand that the naming is hard here. One thing I am wondering is whether this might ever make sense unless AlignAfterOpenBracket is set to AlwaysBreak?
> > > 
> > > Unless that option is set, we could have both in the same file:
> > > 
> > >   someFunction(aaaa,
> > >                bbbb);
> > > 
> > > and
> > > 
> > >   someFunction(
> > >       aaaa, bbbb
> > >   );
> > > 
> > > Is that intended, i.e. are you actively using that? Answering this is important, because it influence whether or not we actually need to add another style option and even how to implement this.
> > The name was based on the configuration option that scalafmt has for their automatic scala formatter, they also have an option to have the closing paren on its own line and they call it `danglingParentheses`. I don't love the name and am open to other options.
> > 
> > That's a good point about AlignAfterOpenBracket being set to AlwaysBreak. In our usage we have that option set, and I'm also unsure if it makes sense without AlwaysBreak.
> Hm. I am not sure either. What do you think of extending the BracketAlignmentStyle enum with an AlwaysBreakWithDanglingParenthesis? Or AlwaysBreakAndWrapRParen?
Sorry for the delay in the reply!

That seems like a reasonable solution to me. I believe the structure of the patch would be the same, just changing out the configuration option.

Can you point me to where I should look to figure out how to run the unit tests and let me know if there are other changes you would recommend besides updating configuration options?


https://reviews.llvm.org/D33029