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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to