1581890 - Remove tab characters from most NSPR files

Status: RESOLVED FIXED

(NSPR :: NSPR, task)

Description

[Kai Engert [:KaiE:]]

Bug 1375876 is more work, so I'd like to start with a simpler change:

Let's remove all tab characters from most NSPR source files.

Easier editing, without having to worry about tabs, is my primary goal when working with the code.

Comment 1

[Kai Engert [:KaiE:]]

Attachment: Bug 1581890 - Remove tab characters from most NSPR files. r?glandium

Comment 2

[Kai Engert [:KaiE:]]

Attachment: Bug 1581890 - remove whitespace from end of line in h/c/cpp. r?glandium

Comment 3

[Kai Engert [:KaiE:]]

Attachment: Bug 1581890 - move comment into definition. r?glandium

Comment 4

[Kai Engert [:KaiE:]]

I've experimented with the "artistic style" (astyle) utility, and I like the results better than clang-format.

The changes made by astyle are less invasive. It cleans up the indenting nicely.

It avoids most of the changes you had critized in https://phabricator.services.mozilla.com/D36729
It doesn't touch the * alignments, and it keeps the blocks unchanged, where you had asked for // clang off
Also, I've added the braces for one line if statements, as you had requested.

I haven't addressed your complaints about win16 headers or "error instead of #error", I think that's out of scope and should be done separately.

I've used the following parameters for the astyle utility, and applied them to all files with h/c/cpp extensions:

astyle --mode=c --indent=spaces=4 --convert-tabs --min-conditional-indent=0 --options=none --indent-switches --keep-one-line-statements --add-braces --align-reference=none

I'll attach an incremental patch in a minute.

Comment 5

[Kai Engert [:KaiE:]]

Attachment: Bug 1581890 - formatted code with artistic style utility. r?glandium

Comment 6

[Kai Engert [:KaiE:]]

BTW, astyle appears to be smarter than the version of clang-format that we're using for NSS.
clang-format replaced tabs inside two strings, while astyle kept them correctly.

Comment 7

[Kai Engert [:KaiE:]]

astyle also offers
} else {
but enabling that would change many additional lines, and move the opening bracket in many places.
(additional astyle parameter --style=mozilla)

I'll attach an optional, incremental patch, and leave it to you, if you want to review that, too.
I don't consider it necessary.

Comment 8

[Kai Engert [:KaiE:]]

Attachment: Bug 1581890 - Additional astyle changes to have "} else {" on single line. r?glandium

Comment 9

[Kai Engert [:KaiE:]]

Mike, my primary question for you is:

Do you agree these changes are an improvement? If yes, I'd prefer to apply these changes, without going into much more detail.

Personally, I'd only use patches 1, 2, 3, and ignore 4.

Comment 10

[Kai Engert [:KaiE:]]

Here is a NSS try build, which built NSPR including the currently attached NSPR patches.
https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=8fb13f219d4d4c0013058dccfdff50084daa1cff

Comment 11

[Kai Engert [:KaiE:]]

Attachment: Bug 1581890 - add .hg-annotate-ignore-revs to hide reformat revisions. r?glandium

Comment 12

[Kai Engert [:KaiE:]]

Thanks for the reviews Mike!

https://hg.mozilla.org/projects/nspr/rev/6856b110d025599a243f0b0128c7f39572a9ead0
https://hg.mozilla.org/projects/nspr/rev/753c7baeb58d34bb93f6bdcdd49f8e74bc32ee41
https://hg.mozilla.org/projects/nspr/rev/b2e285dc48be27713b3c648de3cf33f8637a3e94