search

GPUOpen-Effects / TressFX

DirectX 12 and Vulkan libraries that provide convenient access to realistically rendered and simulated hair and fur
MIT License
766 stars 129 forks source link

Add a .clang-format file? #20

Open vlj opened 8 years ago

vlj commented 8 years ago

Clang format is a tool shipped with clang that can reformat file to match a given code style. It's often difficult to strictly follow a code style in a project especially if you work on others projects with different code style ; clang format just check and fixed that automatically. It can even be run as a git script so that no commit "break" style.

I tried to write a config file that defines the code style in tressFx. However it looks like tressFx is using an extra space inside parenthesis in function body (in if/for/...) , but not in function declaration. Unfortunately clang format has a single spaceInParenthesis that is used for every occurrence of parenthesis, whether it's in code or in declaration. Is it important to have space inside parenthesis? Using clang format would avoid contributor and reviewer to spend time on checking indent, space, alignment of parenthesis, * location and so on.

khillesl-AMD commented 8 years ago

Internally, we've been using vera++ with some custom rules. Spaces inside parenthesis - I don't know that we have a standard there. We do have a rule that checks for a space after certain keywords (after if, for, ...) but that's not inside the parenthesis.

We'll take a look at clang format as an alternative or addition, though.

danilaml commented 8 years ago

AFAIK you can setup clang-format to only run on new code, leaving already written as it is to preserve history. Usage of space inside of parenthesis doesn't seem to be consistent at least after a quick glance at https://github.com/GPUOpen-Effects/TressFX/blob/master/amd_tressfx/src/TressFX.cpp

jstewart-amd commented 8 years ago

I've just written a first attempt at a clang-format file for GPUOpen-Effects. It still needs more testing. I'll start using it to clean up existing code and see if it does what I want in all cases. If so, I can add it to the repos for others to use.

jstewart-amd commented 8 years ago

P.S. This would be in addition to our vera++ rules, not a replacement, as clang-format can't do everything (e.g. it can't check for the presence of the copyright/license comment header, but vera++ can). I can also post our vera++ package (eventually) for others to use. The main holdup there is not all existing code passes the check yet. (d'oh)

danilaml commented 8 years ago

Btw, can't you setup CI (like travis or appveyor) for your repo? You can include all such codestyle checks there.