[PATCH] D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab

[Guillaume Reymond via Phabricator via cfe-commits]

guiguiiiiiiii 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;
----------------
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;
```


================
Comment at: unittests/Format/FormatTest.cpp:9372
+  FormatStyle::UseTabStyle OldTabStyle = Alignment.UseTab;
+  unsigned OldTabWidth = Alignment.TabWidth;
+  Alignment.UseTab = FormatStyle::UT_Always;
----------------
klimek wrote:
> Instead of doing the save/restore dance, just put it at the end of a test or create a new test (or alternatively create a new style as copy and then change the settings on that).
I guess a new test would be better since the unit tests are really lacking toward using tabs.


Repository:
  rC Clang

https://reviews.llvm.org/D48259