search

rcthomas / resist

BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

We should conform to a coding style. #7

Closed bcfriesen closed 7 years ago

bcfriesen commented 7 years ago

Since this will be pure C, we have the great luxury of being able to use GNU indent to do this for us.

rcthomas commented 7 years ago

I need to re-read the indent docs, but I think this will be fine. I am not a huge fan of automating source reformatting on check-in, but we should require ourselves to run indent on code before check-in. Is there a way to have indent run in a check mode, where if it doesn't meet the coding style it causes an error? Passing the format check could be required before a merge, say.

bcfriesen commented 7 years ago

Agreed, nothing should be automated. As far as I can tell, indent has no "check" mode. So the decision to make is how much we care about this, viz. policing ourselves vs the RESIST arbiter checking each request. One could emulate a "check mode" with a script like the following:

bash-4.4$ cat checkpatch.sh 
#!/bin/bash

INDENT="gindent"
INDENT_OPTS="-linux"

INPUT="$1"
FORMATTED_INPUT="${INPUT}.fmt"

${INDENT} ${INDENT_OPTS} ${INPUT} -o ${FORMATTED_INPUT}

if [ ! $(cmp -s ${INPUT} ${FORMATTED_INPUT}) ]; then
    printf "%s\n" "ERROR: file ${INPUT} violates code style!"
fi

rm -f ${FORMATTED_INPUT}
rcthomas commented 7 years ago

That sounds viable, let us have that.

bcfriesen commented 7 years ago

OK. I shall make a pull request momentarily with this script. Do you prefer a subdir called utils or scripts or misc or some such?

bcfriesen commented 7 years ago

Also which style do you prefer? I don't have a preference, only that we enforce the one we pick. Note also that, in addition to the myriad formatting flags to choose from, indent also supports 3 "shortcut" styles: -gnu (the GNU style), -linux (the Linux kernel style), and -kr (Kernighan & Ritchie).

rcthomas commented 7 years ago

The downside to doing it this way is that we don't actually get output like pep8 gives you: a list of what's wrong. What about pep7

https://github.com/mike-perdide/pep7

bcfriesen commented 7 years ago

Huh, I did not know pep7 was a thing. Yeah we could definitely do that. What style does it enforce?

rcthomas commented 7 years ago

Why it enforces PEP 7 style of course, which seems okay to me...

https://www.python.org/dev/peps/pep-0007/

bcfriesen commented 7 years ago

Oh. Ok then let's do that!

bcfriesen commented 7 years ago

To bring our offline discussion back online, PEP 7 seems to be woefully out of sync with PEP 8 and depends on it unnecessarily. So we should use GNU indent instead.

rcthomas commented 7 years ago

Further research turned up uncrustify which is quite configurable. In commit 510ab18e4b9fc642cbe01c4d31032416046afece I've added a make target make format for checking all the source, along with a configuration file that we can use to enforce consistent style. One simply needs to install uncrustify for it to work.

In future, make sure to do make format before committing to identify any noncompliant files. If you just want to run the formatter, you can do uncrustify -c uncrustify.cfg --replace --no-backup file.c and it will format the source.