klimek added inline comments.
================ Comment at: lib/Format/WhitespaceManager.cpp:678 // Indent with tabs only when there's at least one full tab. - if (FirstTabWidth + Style.TabWidth <= Spaces) { + if (Style.TabWidth <= Spaces) { Spaces -= FirstTabWidth; ---------------- guiguiiiiiiii wrote: > klimek wrote: > > Why is this not just if (FirstTabWidth <= Spaces) then? > Actually, that was my first way of fixing it. > > ``` > if (FirstTabWidth <= Spaces && Spaces > 1) // to avoid converting a single > space to a tab > ``` > > Doing so will produce a correct output but introduce what I thought could be > an unwanted change : whitespace less than **TabWidth** might get converted to > a tab. > The comment above the test seems to indicate that this behavior was not > originally wanted, that's why I changed my mind. > > But after thinking it over, I think my fix attempt also introduce an unwanted > change. I misinterpreted the original goal of this option. > I went back to the official documentation and it says > > > Use tabs whenever we need to fill whitespace that spans at least from one > > tab stop to the next one > > This is clearly not what this code is doing. I can land another patch for a > more appropriate fix but I wonder if this option should stay like this. > Aren't people expecting the formatting to use as much tab as possible with > UT_Always ? > > Unless I'm mistaken, the following example (**TabWidth** of 4) > > > ``` > int a = 42; > int aa = 42; > int aaa = 42; > int aaaa = 42; > ``` > would become (following what's written in the documentation) > > ``` > int a = 42; > int aa = 42; > int aaa = 42; > int aaaa = 42; > // only spaces since there's never enough whitespace to span a full tab stop > ``` > And this is what I would expect: > > ``` > int a = 42; // int a\t = 42; > int aa = 42; // int aa\t = 42; > int aaa = 42; // int aaa\t = 42; > int aaaa = 42; // int aaaa = 42; > ``` The intent of the Always setting was to mirror what the Linux kernel wants, so whatever that is, we should probably follow it :) Repository: rC Clang https://reviews.llvm.org/D48259 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits