25

Code formatting matters. Even indentation matters. And consistency is more important than minor improvements. But projects usually don't have a clear, complete, verifiable and enforced style guide from day 1, and major improvements may arrive any day. Maybe you find that

SELECT id, name, address
FROM persons JOIN addresses ON persons.id = addresses.person_id;

could be better written as / is better written than

SELECT persons.id,
       persons.name,
       addresses.address
  FROM persons
  JOIN addresses ON persons.id = addresses.person_id;

while working on adding more columns to the query. Maybe this is the most complex of all four queries in your code, or a trivial query among thousands. No matter how difficult the transition, you decide it's worth it. But how do you track code changes across major formatting changes? You could just give up and say "this is the point where we start again", or you could reformat all queries in the entire repository history.

If you're using a distributed version control system like Git you can revert to the first commit ever, and reformat your way from there to the current state. But it's a lot of work, and everyone else would have to pause work (or be prepared for the mother of all merges) while it's going on. Is there a better way to change history which gives the best of all results:

  • Same style in all commits
  • Minimal merge work

?

To clarify, this is not about best practices when starting the project, but rather what should be done when a large refactoring has been deemed a Good Thing™ but you still want a traceable history? Never rewriting history is great if it's the only way to ensure that your versions always work the same, but what about the developer benefits of a clean rewrite? Especially if you have ways (tests, syntax definitions or an identical binary after compilation) to ensure that the rewritten version works exactly the same way as the original?

9
  • 25
    Why would you want to rewrite history? It defeats the purpose of version control. You want to make sure that the application you shipped 3 month ago matches the revision xxxxxx without the slightest doubt. Even trivial reformatting is unacceptable. Commented Jun 28, 2012 at 15:35
  • 5
    I like to comment commits that I do this with tagged with "Reformat. No functional change"
    – rlperez
    Commented Jun 28, 2012 at 18:05
  • 3
    On a unrelated topic, it sounds like you were suggesting to rewrite Git history by reformatting all the code. Don't give people idea, rewriting Git history is bad for 99.9% of the cases. Reformatting is not the .1% edge case. Commented Jun 28, 2012 at 21:06
  • 4
    In some languages (I'm looking at YOU, Python) reformatting can change the logical functioning of the code. You'd have to be able to parse all languages stored in your VCS to track and ignore reformats safely. Commented Jun 29, 2012 at 15:58
  • 3
    Reformats are code changes and should be committed as such. Commented Jun 29, 2012 at 21:11

6 Answers 6

30

Do the reformatting as separate commits. This will interfere minimally with the history, and you should be able to see at a glance which commits are just reformatting and which actually change code. It could skew git blame and similar, but if it points to a reformat-only commit, it's fairly straight forward to look for the previous change before that.

5
  • I've seen projects derailed for weeks because one of the developers thought this was a good idea. If you're going to do this, understand the risks beforehand, and decide exactly how far you're going to go with the formatting. I think mjfgates has the right answer.
    – Johntron
    Commented Jan 18, 2016 at 20:41
  • 1
    Sounds like the team in question has bigger problems than code formatting. But yeah, I don't recommend doing this unless you thing you have to. If you want to do reformatting changes, I'd still say better do them as separate commits than intermixed with functional changes.
    – harald
    Commented Jan 23, 2016 at 17:31
  • Yeah, lots of problems :P I just want to caution new developers that it's not a simple as it sounds. Bulk-reformatting tools are risky (especially if you build it yourself with regex - at least use AST), and if you care about code review and bug tracking, it can really mess with your process. Personally, I write my code to be consistent with the style of each file, though I don't mind reviewing code when a few functions are reformatted. Many developers get hung up on code style and neglect the bigger issues like architecture, process, tooling, etc.
    – Johntron
    Commented Feb 5, 2016 at 6:42
  • In programming, nothing is as simple as it sounds :)
    – harald
    Commented Feb 5, 2016 at 10:01
  • You might also choose a good point in time to do this, combined with automatic reformat on save in your IDE. Commented Dec 23, 2021 at 6:50
14

Don't rewrite VCS history: it;s against VCS principles.

Don't try to automate fixing the formatting: it is treating the symptoms, not the real problem (= developers not following coding standards).

Define the coding standard and formatting best practices in a common document and get all developers to agree.

You mention Git, which is great, because it's distributed. With a DVCS it's very easy to enforce best practices through the gatekeeper workflow. Gatekeepers reject merge proposals (= pull requests in Git) that don't conform to the common guidelines. And I do mean reject, in bold letters, otherwise the coder in violation will not bother to follow the rules and continue to repeat the same mistakes.

This technique works well for me. Coders want their work to be merged, so after a few mistakes in the beginning they start following the rules.

As per fixing the existing code base... I recommend doing that gradually, perhaps module by module, or as it makes sense for your project. Test carefully at each step. It may sound stupid, but mistakes do happen even with trivial changes like just the formatting, so be prepared for some minor bumps on the road.

2
  • 1
    Downvoted, because the author clearly states this is in the context of projects that didn't start with "... a clear, complete, verifiable and enforced style guide from day 1". He can't treat the real problem, because it's already happened. I do agree with you though :)
    – Johntron
    Commented Jan 18, 2016 at 20:35
  • 2
    reject means that there will be a fight between the humans and the robot. Been there. Sooner or later, the robot will require a really complex piece of code to be formatted in an unreadable way. Examples: a Java string is in fact a SQL statement, but the robot does not know this; whitespace before closing parens may carry information about the structure of the code for humans, but not for the robot; function parameters get split across multiple lines in the most meaningless way... Commented Feb 8, 2018 at 10:31
10

The answer to your actual question is, "You don't." I know of no current SCM tool that can trace changes in logic from code formatted in one way, through a major formatting change, and through further changes after the code is formatted in the new way. And, you know this, losing the history on a piece of code is Not Good.

Accordingly, I'm going to contradict your first sentence a bit. Code formatting doesn't matter that much. Pretty is nice, but it's not what we're here for. I understand as well as anybody that getting dumped into somebody's old hellish weird K&R variant code with the two-space indents sucks (1), but... the formatting isn't actually an obstacle to understanding what's going on, unless it's something exceptionally pathological. And in that case, you're going to have problems changing the code anyway, and shouldn't bother it.

Therefore, it's not worth it to make changes to established code STRICTLY to reformat it. Changing the variable names, breaking up long functions, all that good refactoring stuff that changes the content, yes, but not JUST reformatting.

1) - I once owned the Windows Clipboard Viewer for a while. The whole thing was one, 150k, C module. I found a spot where different people had used, I think, five different brace styles within thirty lines of each other. But that section of things WORKED. I carried around a printout of that chunk of code for ten years, but I didn't poke it because that history mattered, and that code was in at least three source trees (Windows 3.x, NT, future 95) which all lived in different buildings.

2
  • In the past, using hg, I have found that merge-by-parts is an invaluable tool in coping with tricky big re-factor merges. Typically what I would do is merge the commits before the big re-factor, then merge the big re-factor itself and then finally merge the commits since the re-factor. Each of these three merges on their own are much easier then trying to untangle the mess that results from doing all of the merges in one go.
    – Mark Booth
    Commented Jun 29, 2012 at 15:41
  • I totally agree! Additionally, I've seen many developers go overboard (a younger version of myself included) on reformatting and code style, and they end up introducing defects. A missing comma/semicolon here, variable declarations moved to top of functions, for-loops changed to for-each's - they can all introduce subtle bugs. It takes a deceptive amount of skill to make these changes safely.
    – Johntron
    Commented Jan 18, 2016 at 20:39
4

But how do you track code changes across major formatting changes?

Formatting changes are code changes; treat them as you would any other change to your code. Anyone who has worked on a significant project will probably have seen bugs and other issues that were created when someone decided to "just" reformat some code.

But it's a lot of work, and everyone else would have to pause work (or be prepared for the mother of all merges) while it's going on.

Why do you have to reformat everything all at the same time? Especially if the reformatting doesn't change the meaning of the code, you should be able to reformat files individually and check them in as you go along. Better, get everyone on your team to agree on a style (otherwise there's no point in reformatting anyway) and have them all take care of reformatting in the course of their other work. After a while, you'll have covered most of the code with out disrupting the rest of the project.

1

There are viable two approaches that I've seen for this.

1. Reformat code on commit-hook

While it's initially hair-raising to alter code after they've submitted it, if your reformatting procedure (e.g. astyle) doesn't hurt the code, then its a safe operation. Over time the whole team will appreciate that all code eventually looks the same. Clearly, having comprehensive unit/automated tests will ensure that nothing broke.

2. One-time reformatting of all the code

This is more dangerous in my experience, and makes tracking issues across the big-bang difficult, but it's possible. Running all tests afterwards is essential. For coding style, the majority of differences revolve around use of whitespace - indentation or newlines. A decent merge tool should be able to be told to ignore all whitespace differences, so this will help with merges.

4
  • 1
    Wouldn't option one when turned on ripple across the majority of the code base quickly resulting in the same big bang of having each file change?
    – Sign
    Commented Jun 28, 2012 at 18:23
  • @Sign: Exactly my point - When the commit hook changes, your history could deteriorate into something almost useless. Formatting which doesn't change functionality shouldn't be a commit, it should be transplanted throughout the code history.
    – l0b0
    Commented Jun 28, 2012 at 20:12
  • 1
    If the IDE's support it then there is also 3) have the IDE autoformat at save. Then just use the same settings everywhere - this is easiest if you use the default with the IDE.
    – user1249
    Commented Jun 28, 2012 at 20:25
  • I've done both of these approaches. The first approach is very obtrusive because there will be a ton of changes every time a new file is committed for the first time. The second approach is better for the team, like ripping off a bandaid quick.
    – Druska
    Commented Apr 8, 2019 at 17:28
1

Since the question was originally asked, a much better way of handling the same formatting across the whole team has been introduced, namely an .editorconfig file in the root of your source tree.

This file is either read directly by most modern IDE's or can easily be made to do so and tells exactly how the source should be treated. Tabs versus spaces is a thing of the past.

For new projects this should be added in one of the very first commits.

I would suggest for an existing project that the whole team converges the work to a single branch, and the .editorconfig file is introduced with a reformat of all the sources on that branch in a single commit. Then normal work commences.

See https://editorconfig.org/ for details.

2
  • .editorconfig is great, and I use it on all projects. The question was more about how to gracefully introduce auto-formatting into an existing project without completely screwing with the traceability of changes.
    – l0b0
    Commented Jan 23, 2022 at 4:45
  • You Can’t and keep existing history. You need a reformat commit. Commented Jan 23, 2022 at 11:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.