[Development] White space / coding style patches welcome?

[Axel Waggershauser]

On Wed, Mar 13, 2013 at 10:30 AM, Oswald Buddenhagen
<oswald.buddenhagen at digia.com> wrote:
> On Wed, Mar 13, 2013 at 01:38:31AM +0100, Axel Waggershauser wrote:
>> I'd say in terms of Lars's worries about having automatic checks in
>> place before attacking whitespace issues, there should not be any
>> objection against fixing at least those 4 issues immediately.
>
>> One might ask, though, if it would be a good idea to actually enforce
>> the checks instead of merely letting the Sanity Bot warn about them?
>>
> actual enforcement was lars' request.
> but this clearly requires a very low false report rate to be accepted.

I see. Regarding false report rate, I think that checking for trailing
ws as well as checking for leading tabs should be secure enough to
actually strictly enforce them. See also 'git diff --check'.

A word about the auto-generated code situation: Having had a look at
files in corelib/codecs/ that contain presumably auto-generated
tables, I don't see why they should not be strictly checked as well.
As far as I can tell, the files have been assembled/edited manually
and if they should be updated some time, then a simple CTRL+I in
QtCreator and saving with the "Clean Whitespace" option set should
take care of that easily. Also, they are by no means consistent as
they are right now, anyway.

The 'mixed whitespace only changes' test is definitely flaky, so not
good for enforcement. E.g I just ran across the following false
positive:

 //! [0]
     int numRead = 0, numReadTotal = 0;
     char buffer[50];

     forever {
-       numRead  = socket.read(buffer, 50);
+        numRead  = socket.read(buffer, 50);
+
+        // do whatever with array

-       // do whatever with array
-
        numReadTotal += numRead;
        if (numRead == 0 && !socket.waitForReadyRead())
            break;
     }
 //! [0]

This is a ws-only change but sanitize-commit processes that text in
"chunks" of connected '-'/'+' blocks, so the last two '-' lines are
not taken into consideration. But I think the whole "mixed commit'
issue will become more or less irrelevant, if all issues in the
current source base are solved, as there would be simply no
'collateral' ws-fixes in the future.

Regarding the 'check for space after keyword' check and others that I
might be able to add, I have no feeling about their robustness, yet.


>> Given that the trailing ws issue is very easily fixed by a one-liner
>> as Corentin has used and that the overlap with the tab issue is so
>> small, I suggest to fix all those 2k lines of trailing whitespace
>> adding the couple tab-fixes manually, so the sanity bot is happy, and
>> commit that in one go.
>>
>> Then in a second step deal with tabs and maybe other ws issues, that
>> require manual intervention in smaller chunks.
>>
> ok, sounds like a plan. go ahead.

First step see https://codereview.qt-project.org/#change,50875

 - Axel