benhamilton marked an inline comment as done.
benhamilton added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:904
+               : State.Stack.back().Indent);
       if (NextNonComment->LongestObjCSelectorName == 0)
+        return MinIndent;
----------------
djasper wrote:
> benhamilton wrote:
> > djasper wrote:
> > > Does this if actually change the behavior in any way? With 
> > > LongestObjCSelectorName being 0, isn't that:
> > > 
> > >   return MinIndent +
> > >          std::max(0, ColumnWidth) - ColumnWidth;
> > > 
> > > (and with ColumnWidth being >= 0, this should be just MinIndent)
> > The `- ColumnWidth` part is only for the case where 
> > `LongestObjCSelectorName` is *not* 0. If it's 0, we return `MinIndent` 
> > which ensures we obey `Style.IndentWrappedFunctionNames`.
> > 
> > The problem with the code before this diff is when 
> > `LongestObjCSelectorName` was 0, we ignored 
> > `Style.IndentWrappedFunctionNames` and always returned 
> > `State.Stack.back().Indent` regardless of that setting.
> > 
> > After this diff, when `LongestObjCSelectorName` is 0 (i.e., this is either 
> > the first part of the selector or a selector which is not colon-aligned due 
> > to block formatting), we change the behavior to indent to at least 
> > `State.FirstIndent + Style.ContinuationIndentWidth`, like all other 
> > indentation logic in this file.
> > 
> > I've added some comments explaining what's going on, since this code is 
> > quite complex.
> I am not saying your change is wrong.  And I might be getting out of practice 
> with coding. My question is, what changes if you remove lines 906 and 907 
> (the "if (...) return MinIndent")?
Oh, I see what you're saying now! Thanks for clarifying.

Yes, we can remove these lines now. Done.


Repository:
  rC Clang

https://reviews.llvm.org/D44994



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

Reply via email to