ioeric added a comment. In https://reviews.llvm.org/D28943#651536, @amaiorano wrote:
> In https://reviews.llvm.org/D28943#651489, @ioeric wrote: > > > In https://reviews.llvm.org/D28943#651488, @amaiorano wrote: > > > > > In https://reviews.llvm.org/D28943#651470, @ioeric wrote: > > > > > > > @amaiorano: The test itself is correct. It's just that this test failed > > > > in our internal test. We could've fixed it internally, but the fix > > > > would be ugly. Since the intended behavior is already covered in the > > > > case above it, and it's really just checking the default fallback style > > > > is LLVM, which is not related to the original change, I think it makes > > > > sense to get rid of the case. Hope you don't mind :) > > > > > > > > > Of course I don't mind :) Why did it fail your internal tests, btw? Just > > > curious. Was it something I could've detected myself? > > > > > > Probably not... it's just that our default fallback style is "Google" > > instead of "LLVM". > > > (I replied the following by email, but I'm not sure where it went... posting > it here instead) > > You mean you build a modified version of clang-format where Style is > initialized to getGoogleStyle()? > > I had wondered whether adding a "defaultStyle" argument might be useful, > specifically in the case where you want to pass in yaml that simply tweaks > the default style, but I figured it's not much harder to pass in > "BasedOnStyle" in the yaml. No.. we set both default style and default fallback style (tool options) to "google" in our build. I don't see why "defaultStyle" would be useful. I think having the existing "style" and "fallback-style" options is sufficient (and already a bit confusing). Repository: rL LLVM https://reviews.llvm.org/D28943 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
