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

Manuel Klimek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 14 03:21:34 PDT 2019


klimek added a comment.

In D33029#1423949 <https://reviews.llvm.org/D33029#1423949>, @MyDeveloperDay wrote:

> In D33029#1423947 <https://reviews.llvm.org/D33029#1423947>, @lebedev.ri wrote:
>
> > In D33029#1423944 <https://reviews.llvm.org/D33029#1423944>, @MyDeveloperDay wrote:
> >
> > > Adding the unit tests lets us see how this option will work in various cases, it will let us understand that its not breaking anything else.
> > >
> > > I personally don't like to see revisions like this sit for 2 years with nothing happening, I don't see anything wrong with this that would prevent it going in so I don't understand whats blocking it?,
> > >
> > > if you had some tests and a release note I'd give it a LGTM (but as I've said before I'm not the code owner, but someone wanting to address defects and add capabilities. but I think we need to be able to move forward, people will object soon enough if they don't like it.)
> > >
> > > Generally I don't understand why clang-format is so reluctant to change anything, As such we don't have many people involved and getting anything done (even defects) is extremely hard.
> > >
> > > It looks like you met the criteria, and reviewers have been given ample opportunity to make an objection. the number of subscribers and like tokens would suggest this is wanted,
> > >
> > > Please also add a line the in the release notes to say what you are adding. In the absence of any other constructive input all we can do is follow the guidance on doing a review, for what its worth I notice in the rest of LLVM there seems to be a much larger amount of commits that go in even without a review, I'm not sure what makes this area so strict, so reluctant to change especailly when revisions do seem to be reviewed.
> >
> >
> > I don't have any stake here, but i just want to point out that no tool (including clang-format)
> >  will ever support all the things all the people out there will want it to support. But every
> >  new knob is not just a single knob, it needs to work well with all the other existing knobs
> >  (and all of the combination of knob params), and every new knob after that.
> >
> > It's a snowball effect. Things can (and likely will, unless there is at least a *very* strict testing/quality policy
> >  (which is https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options about))
> >  get out of hand real quickly..
> >
> > Just 2c.
>
>
> Correct we should always be adding tests and show we don't break existing tests... We need to apply the "Beyonce Rule".. "if you liked it you should have put a test on it."
>
> We shouldn't just give up making improvements or fixing defects because something is hard or complex.


The problem is that clang-format is already at a complexity level where most patches for new things we see are overly complex and will make adding new things *even harder*, reducing the ability to move at all.
As I've written before, I agree that we're in a bad state, and I'm truly sorry for it - we need to get active contributors into a state where they can make changes aligned with the architectural spirit of clang-format, which make clang-format at least not more complex :)

For this specific patch, yes, please change it as djasper suggested to be configurable in the BracketAlignmentStyle.


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

https://reviews.llvm.org/D33029





More information about the cfe-commits mailing list