oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1268706, @djasper wrote:
> Ok, I think I agree with both of you to a certain extent, but I also think > this change as is, is not yet right. > > First, it does too much. The original very first example in this CL is > actually not produced by clang-format (or if it is, I don't know with which > flag combination). It is a case where the lambda is the last parameter. Right, I cheated and created that example by hand. My apologies for the confusion. I've just pasted real code that I pumped through `clang-format`. Please take a look at the updated summary. > Second, I agree that the original behavior is inconsistent in a way that we > have a special cases for when the lambda is the very first parameter, but > somehow seem be forgetting about that when it's not the first parameter. I'd > be ok with "fixing" that (it's not a clear-cut bug, but I think the > alternative behavior would be cleaner). However, I don't think your patch is > doing enough there. I think this should be irrespective of bin-packing (it's > related but not quite the same thing), Also there is a special case for multiple lambdas. It forces line breaks. That aside, for the single-lambda case, are you suggesting that it is always "pulled up", irrespective of its place? That would contradict the direction I am trying to take as I like `BinPackArguments: false` and so long lamba args go to their own lines. This looks very much in line with what bin packing is, but not exactly the same. Obviously, I can add a new option `favor line breaks around multi-line lambda`. Could you look at the updated summary (high level) and the new tests I added (low level) please? Every other test passes, so we have almost the entire spec. I can add a few cases where an existing snippet is reformatted with `BinPackArguments: false` too. > ...and it should also apply to multiline strings if > AlwaysBreakBeforeMultilineStrings is true. It doesn't all have to be done in > the same patch, but we should have a plan of what we want the end result to > look like and should be willing to get there. > > Maybe to start with, we need the complete test matrix so that we are > definitely on the same page as to what we are trying to do. I imagine, we > would need tests for a function with three parameters, where two of the > parameters are short and one is a multiline lambda or a multiline string (and > have the lambda be the first, second and third parameter). Then we might need > those for both bin-packing and no-bin-packing configurations. Repository: rC Clang https://reviews.llvm.org/D52676 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
