Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format source code with clang-format. #17

Open
malespiaut opened this issue Jul 18, 2023 · 6 comments
Open

Format source code with clang-format. #17

malespiaut opened this issue Jul 18, 2023 · 6 comments

Comments

@malespiaut
Copy link
Contributor

No description provided.

@fabiangreffrath
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

Do you have a reasonable configuration at hand?

@malespiaut
Copy link
Contributor Author

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
Copy link
Owner

@malespiaut
Copy link
Contributor Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants