fabiangreffrath / taradino

SDL2 port of Rise of the Triad
GNU General Public License v2.0
35 stars 9 forks source link

Format source code with clang-format. #17

Open malespiaut opened 1 year ago

fabiangreffrath commented 1 year ago

This will be a brutal measure, not even Tom Hall will recognize his own code anymore. But given the degree of spaghetti in the current code base, I might even consider it. However some of the bigger overhauls need to be finished first.

malespiaut commented 1 year ago

I do agree that this won't be a light move. And I wanted to add that I would suggest to do this both “at the end” of every other tasks, and progressively. The problem is that some functions gets reformated by hand anyway when working on them. I do understand that one might want to do changes with surgical precision on this source code; but I also think that the state of the source code is a reflection of the mess that it was back in the DIP's offices.

fabiangreffrath commented 1 year ago

Do you have a reasonable configuration at hand?

malespiaut commented 1 year ago

I have a .clang-format file that I use for all of my C projects, which is based on one of the predefined style with a few tweaks.

---
Language: Cpp
BasedOnStyle: GNU
AllowShortBlocksOnASingleLine: Empty
AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: None
ColumnLimit: 0
ConstructorInitializerIndentWidth: 2
ContinuationIndentWidth: 2
Cpp11BracedListStyle: true
IndentCaseBlocks: true
IndentGotoLabels: false
PointerAlignment: Left
SpaceBeforeParens: ControlStatements
TabWidth: 2

As you can see, I'm using the GNU formating style, with a few cosmetic changes. Here's a formatted code snippet to give you an idea:

void
Z_Realloc(void** ptr, size_t newsize)
{
  memblock_t* block;
  void* newptr;
  size_t oldsize;

  block = (memblock_t*)((char*)(*ptr) - HEADER_SIZE);
  oldsize = block->size;
  newptr = SafeMalloc(newsize);
  if (oldsize > newsize)
    {
      oldsize = newsize;
    }
  memcpy(newptr, *ptr, oldsize);
  SafeFree(*ptr);
  *ptr = newptr;
}

Of course, style is a matter of personnal preference, and I would encourage you to give a try to all the predefined styles, to see which one suits you best. Any style is alright, as long as it is consistent.

There are tools that format the code with all possible parameters to find out with set of rules is more likely to make the less changes to the source code. But considering how large the codebase is, and how messy and inconsistent the “formatting style” is, using such tools won't help making the commits smaller.

fabiangreffrath commented 1 year ago

We could use this file for a start:

https://github.com/chocolate-doom/chocolate-doom/blob/master/.clang-format

malespiaut commented 1 year ago

Good idea. I'm OK with absolutely any coding style, as long as it is consistent.

However, I must point out one drawback of this configuration file is that it is long. That means we'll have, at least, to check out deprecated flags over time.

clang-format comes with 7 predefined coding styles: LLVM, GNU, Google, Chromium, Microsoft, Mozilla, and WebKit. Using an unmodified predefined coding style reduces the size of the .clang-format file to only 3 lines:

---
Language:        Cpp
BasedOnStyle:  LLVM

The final decision is yours!