Should I edit a codebase's whitespace to conform to a coding style?

Question

We have a large Java project (1m+ SLOC) with mixed whitespace - some files have tabs and some have spaces. It's tricky to make my editor work with whichever file I happen to be editing.

We are going to choose a convention and enforce it in future. The question is whether I should make one commit to correct the whitespace in the whole project after the decision is made.

Git can ignore changes in whitespace, so in future to compare with older revisions we would have to use git diff -w. However, if and when we update a file in a piecemeal way we would still have to use git diff -w.

For the record, this is an Eclipse RCP project, so using an IDE other than Eclipse is not really sensible.

Edit: There are some good answers here, but they tend to discuss how I should go about this rather than whether this is a good idea or whether I should just leave well alone, which is what I'm really interested in.

Answer 1

_{What do you mean by "my editor"? And what do you mean by "git -w"? Are you using an editor and command line tools instead of an IDE? May I recommend IntelliJ IDEA? It is the best java IDE ever, and it has no problem with either kind of whitespace, or even with mixed whitespace within the same file.}

Generally, if a massive change has to be made to the code base, like changing the formatting, it should be done all at once and as early as possible.

If you don't do that, then files will keep being committed in the future for no reason other than whitespace changes, unnecessarily bloating the history lists, and forcing you to often request diffs of files only to be told "files differ only in whitespace".

Also, it will never be obvious whether a file was committed due to actual changes or only due to whitespace changes, so if two or more developers happen to have different whitespace and/or formatting settings by mistake, it will take you some time and several commits where one developer is undoing the whitespace changes of another until you realize that this discrepancy exists.

By reformatting and committing everything at once, the revision number of that commit (which will from that day on be known as "The Great Big Reformatting") will be memorized by everyone, so whenever you see that revision number you will know to not even request a diff.

Plus, from that moment on, everyone will know that subsequent commits due to whitespace changes only should not be made, because they are obviously the result of a configuration mismatch between developers.

Amendment

Now, on the question of whether you should convert the entire code base to conform to a particular coding style, this is not an easy thing to answer without knowing the particulars of your situation. The obvious answer, which anyone out there will tell you, is that a consistent coding style is important, and that even a bad (by whatever standard) but consistent style is better than an inconsistent style. However, there are some practical questions to be asked first:

• Do most of the important contributors at your workplace agree?

• Are there any contributors who, despite being a minority, might rage-quit if you proceed with this? And how important are they?

• How big of a variety of styles do you have? Is it only tabs vs. spaces, or does it include other major aspects of coding style like Allman vs. Egyptian braces? People should be flexible enough to not mind a small variety.

• But mostly: Is it really necessary?

I mean, in my current job, each developer is working on a specific, clearly delineated subset of the code base, so I don't delve (much) into other people's code, and nobody delves into my code, so it does not really matter that we have vastly different styles. It neither picks my pocket nor breaks my leg that a colleague wraps his lines at column 80. (Ssssh, he probably does not know how to change the relevant setting.) The situation would be quite different if we had developers whose job involved frequently dealing with other people's code. If you do have such a situation, and if the coding styles vary so much as to make it hard for them to do their job, then you probably should enforce a single coding style for everyone. Otherwise, perhaps not.

One more final note:

In theory, it should be possible to resolve this issue with technical means so that a) different developers can work on the same code, and yet b) each developer gets to enjoy whatever coding style he or she prefers. The way this would (in theory) be accomplished would be by having code formatted to your preferred style when updating from the version control system, and re-formatted to the "project style" right before committing. Unfortunately, as of today, there are no tools that will do this as far as I know. IntelliJ IDEA gets close by supporting multiple styles, including a "personal style" and a "project style", but it is not fully automated: you still get to browse unmodified code in "project style", if you re-format any files to your "personal style" they will unfortunately appear as modified with respect to the pristine copies, (which is probably a shortcoming of the version control system and not of IDEA,) and the (optional of course) step which re-formats files back to "project style" when committing leaves all of your local copies in "project style" again. If anyone knows of anything that achieves more than that, please do say.

Answer 2

1. Before you even start the Big Format, put the proper processes, procedures, and tools in place to ensure consistent formatting in the future. There's no point going through that pain, if the same mess is going to pop up a year from now anyway.
2. In order to minimize merge hell, coordinate with everybody using the source tree, and set a specific date when the Big Format is taking place, giving everybody a chance to merge their changes back into the mainline. Record and publicize that date, as well as the name/number/hash/whatever of the commit. It would probably be a good idea to tag is as well.
3. Do not, under any circumstance mix non-formatting and formatting changes.
4. Use a well-known, well-reviewed, well-tested, well-trusted automated tool (a small sed script, indent, Eclipse's formatting engine, …) to perform the Big Format, and do not do any manual changes to the output of that tool; the output of the tool should be committed as-is, and the commit message should specify the exact name, version, configuration, and command-line (if applicable) for the tool, so that everybody can reproduce the results.

That last point has two advantages:

• Being able to reproduce the diff means that you can shift having to trust the diff that nobody was trying to sneak a change into that 1m line diff to having to trust the Eclipse Foundation (for example) that they are not trying to sneak a change into your code via their formatting engine (which is extremely unlikely, and could further be mitigated by using multiple different tools to end up with the same diff, if possible).
• Being able to reproduce the diff means that developers on non-mainline branches can perform the "merge" by reproducing the effects of the commit mechanically rather than having to actually merge their changes into the newly formatted mainline code.

Answer 3

Yes, fixing all the existing files immediately before starting to enforce whitespace rules for new commits is a good idea. If you have mixed whitespace, there is no such thing as "leaving well enough alone". Your codebase is not well.

If you do not fix it, it becomes nearly impossible to enforce a 'whitespace only' vs 'code change only' policy. And that means

• Small extraneous diffs every time a minor edit is made to an old file.
• Large whitespace changes in random feature commits, whenever someone gets frustrated enough to fix the file they are working in.

Just do it.

Answer 4

No, wait until a decision is made.

Before the reformat I suggest to create a Eclipse formatting profile and distribute it to all developers and start using it. Make Eclipse use the save action to format automatically with every file save.

Then do a complete reformat and commit it as a single refactoring.

The code is now consistent and will remain that way.

But add some analysis tool to the build to check for correct formatting. This to catch mistakes.