spacetelescope / drizzle

A package for combining dithered images into a single image
https://spacetelescope-drizzle.readthedocs.io/en/latest/
Other
51 stars 26 forks source link

Reformat C code using clang-format #113

Closed mcara closed 1 month ago

mcara commented 1 year ago

Apply uniform style to C code similar to PEP8 rules for Python code. This uses modified Google style. "external" FCT headers have been moved to a separate folder in order to be excluded from style formatting.

mcara commented 1 year ago

There are many lines to review here but this is only style change. No functional changes have been made.

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.29%. Comparing base (f3fdc01) to head (a306465). Report is 62 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #113 +/- ## ======================================= Coverage 75.29% 75.29% ======================================= Files 7 7 Lines 344 344 ======================================= Hits 259 259 Misses 85 85 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mcara commented 1 year ago

@kmacdonald-stsci This style tries to follow https://peps.python.org/pep-0007/ vague "guide" but not everything is doable (or recommended - I think we have enough freedom to come up with a style that most agree with).

For example, I could not get this to work automatically:

When you break a long expression at a binary operator, the operator goes at the end of the previous line, and braces should be formatted as shown. E.g.:

if (type->tp_dictoffset != 0 && base->tp_dictoffset == 0 &&
type->tp_dictoffset == b_size &&
(size_t)t_size == b_size + sizeof(PyObject *))
{
return 0; /* "Forgive" adding a __dict__ only */
}

The following style:

Code structure: one space between keywords like if, for and the following left paren; no spaces inside the paren; braces are required everywhere, even where C permits them to be omitted, but do not add them to code you are not otherwise modifying. All new C code requires braces. Braces should be formatted as shown:

if (mro != NULL) {
...
}
else {
...
}

is the Stroustrup's style and it is taken care of in https://github.com/spacetelescope/drizzle/blob/98d6fa9e32599da6e1ac39adc5e2d124bbd82420/.clang-format#L6

But I do not know how to do switching from Stroustrup to Allman style only for very long conditionals. It is either one or another and I like Stroustrup much more than Allman. I like Linux style even more than Stroustrup but I Stroustrup is what PEP7 suggests.

I think we do not have to adhere to PEP 7 since we are not writing code for C implementation of Python.

mcara commented 1 year ago

Ah, the 4 spaces indent is taken care of in https://github.com/spacetelescope/drizzle/blob/98d6fa9e32599da6e1ac39adc5e2d124bbd82420/.clang-format#L11-L12