WIP Create clang-format workflow

[OmniBlade]

Create an agreeable clang-format file and the workflow/instructions for using it appropriately

Fixes #485

[xezon]

Is this correct?

[OmniBlade]

Needed to get the correct spacing in nested templates to compile correctly with VC6.

[xezon]

Indentation is meant to be width 2. So either a tab or 2 spaces.

I suggest we keep leading tabs for starters, because it will make the diff smaller.

[OmniBlade]

I've changed it to 2 spaces. I think if we are going to standardise on a size we should use spaces to ensure it. The fact the code looks incorrectly indented in a lot of places when viewed with tab sizes other than 2 reinforces this in my opinion.

[xezon]

The problem with the 2 spaces is that it now creates this huge diff. I expect the diff will be just half or less big if using leading tabs. Perhaps do the 2 spaces another time?

[OmniBlade]

Okay, I've changed this to tabs and set the indent to 2 to match the original as much as possible and try and reduce the diff.

[xezon]

Can we please use Struct* instead of Struct *?

[OmniBlade]

A somewhat random sampling of files suggests that the prevelant existing style is Type *var and Type &var rather than Type* var and Type& var.

[Caball009]

The current style does favor Type *var and Type &var, but I also prefer Type* var and Type& var.

[xezon]

It does indeed. I ran python script

import glob
import os

def main():
    current_dir = os.path.dirname(os.path.abspath(__file__))
    root_dir = os.path.join(current_dir, "..", "..")
    root_dir = os.path.normpath(root_dir)
    core_dir = os.path.join(root_dir, "Core")
    generals_dir = os.path.join(root_dir, "Generals")
    generalsmd_dir = os.path.join(root_dir, "GeneralsMD")
    fileNames = []
    fileNames.extend(glob.glob(os.path.join(core_dir, '**', '*.h'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(core_dir, '**', '*.cpp'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(core_dir, '**', '*.inl'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(generals_dir, '**', '*.h'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(generals_dir, '**', '*.cpp'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(generals_dir, '**', '*.inl'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(generalsmd_dir, '**', '*.h'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(generalsmd_dir, '**', '*.cpp'), recursive=True))
    fileNames.extend(glob.glob(os.path.join(generalsmd_dir, '**', '*.inl'), recursive=True))

    countLeftPointer = 0
    countRightPointer = 0

    for fileName in fileNames:
        with open(fileName, 'r', encoding="cp1252") as file:
            try:
                lines = file.readlines()
            except UnicodeDecodeError:
                continue # Not good.

        for line in lines:
            for i in range(0, len(line)):
                if i > len(line) - 3:
                    break
                if i < 1:
                    continue
                if line[i-1] == '&' or line[i+1] == '&' or line[i-1] == '*' or line[i+1] == '*' or line[i-1] == '/' or line[i+1] == '/':
                    continue
                if line[i] == '&' or line[i] == '*':
                    if line[i-1].isspace() and not line[i+1].isspace():
                        countRightPointer += 1
                    if line[i+1].isspace() and not line[i-1].isspace():
                        countLeftPointer += 1

    return

if __name__ == "__main__":
    main()

And it returned

countLeftPointer = 25260
countRightPointer = 111217

I also prefer left side pointer because right side pointer makes no logical sense. The pointer is part of the type, not the variable, aka int* is a type, whereas *var is a dereference.

int* var looks like a type
int *var looks like a type/dereference abomination

[OmniBlade]

I agree with the argument that pointer is part of the type, however for the sake of the diff right aligned is a better match for the original style of the code.

[xezon]

To keep the diff small, right pointer is ok for now. In the future we can change to left pointer.

[xezon]

I very much preferred the original formatting of the array lists. Can we preserve this?

[OmniBlade]

I think a trailing , needs to be added to make it keep the list as it originally was but I'll play around with a couple of settings and see what happens.

[xezon]

Here is another example where multiline formatting does not help readability.

[OmniBlade]

Not sure what controls this just for macros to be honest.

[xezon]

This enum has very strange formatting in regards to the curly brackets.

[OmniBlade]

After some testing, it appears that the CPP_11 macro used on a lot of enums breaks clang-format's ability to handle this correctly. until we are VC6 free and the macros are removed, clang format won't get enums correct 😞

[xezon]

The way this is auto formatted is not great.

[xezon]

I think it would be better if this was not cramped into one line.

[xezon]

This is not a good formatting.

[ViTeXFTW]

Those are the annoying cases where the comment eceeds the line limit.
But this is a function definition right? Shouldn't they have doc strings above them instead of inline?

[ViTeXFTW]

Updating clang-format with these values:

PenaltyBreakComment: 0  # modified; was 300
PenaltyBreakOpenParenthesis: 1  # modified; was 0

will give this output

  virtual void loadIntoDirectoryTree(
      const ArchiveFile *archiveFile,
      const AsciiString &archiveFilename,
      Bool overwrite = FALSE); ///< load the archive file's header information and apply it to the global archive directory
                               ///< tree.

Is this more in line with what you expect?

[OmniBlade]

I applied this suggestion.

[OmniBlade]

Made a few changes, but I'm not sure its actually improved anything. I think the issue with the enums might be the CPP_11 macro messing up detection of it being an enum but there isn't much we can do about that for now I guess. Most of the other list formatting changes can be prevented by a trailing , but that involves a bit of manual fixing once an otherwise agreeable format has been accepted.

[xezon]

I was experimenting with uncrustify and was barely unable to isolate minimal formatting. It has huge issues with tab spacings, but gives finer controls than clang format.

[Skyaero42]

Closing this in favor of #1807