Interrupt

Automatically format and lint code with pre-commit | Interrupt

I love clean and tidy codebases. I love them even more if I don’t have to manually spend hours doing it myself. The problem is not everyone shares the same love and passion for tidy codebases, especially when we’re pressed for time trying to get a new firmware build released.


This is a companion discussion topic for the original entry at https://interrupt.memfault.com/blog/pre-commit

Great post. Very clear and a great sample of starting points to up your CI game! Great to see attention paid to the idiosyncrasies of firmware.

I realise that auto-format is so prevalent in the web-dev space as to be largely beyond why and largely now concerned with how. But it’s always seemed so fundamentally concerning and I’ve never got any traction when I raise those concerns. This seems like a great forum to discuss. I figure web-dev is different enough from embedded-dev that the concerns are outweighed by the “stop the PR style-war pain” north star that seems to drive much of web-dev, and for that matter, distributed, open dev in general.

But posts like these show that we’re going to see these ideas merge with embedded-dev eventually, and I wonder if we can be judicious in how that merge occurs? Generally, I’m extremely excited about bring web-dev innovation to embedded-dev, but much more excited about cherry-picking and doing it empathetically, lest we bog things down instead of accelerating them.

With that framing out of the way, my concern is that formatting (by which I mean use of whitespace) is a significant factor in the maintainability of code. We really only have three levers when it comes to writing readable code: structure, naming and formatting. Are we really prepared to give up one of those?

In firmware, perhaps uniquely, whitespace is an extremely powerful tool. Let me demonstrate with some random samples from code I have handy.

Screen Shot 2021-10-28 at 3.29.03 pm

Now imagine those code snippets after auto-formatting, with all the programmer-supplied whitespace removed.

The point of including so many examples is to demonstrate that this is very much not an auto-thing, it’s a programmer-thinking thing. Many of these examples highlight cases where the “usual” convention was broken to aid readability instead. Using robots to format code seems to make as much sense as asking humans to pick passwords with lots of symbols and mixed case. Great for robots, not so good for humans.

And just like passwords, the problem to be solved remains (encouraging hard to hack passwords), but the right solution might not be the obvious one (use a memorable phrase instead).

So you never see final state of your code with changes your pre-commit tool made before it commited into repo?

No, by default if pre-commit makes changes it will reject the commit so you can review the changes. The well-trained monkey at the keyboard then learns to commit again.

The idea of pre-commit is to ensure you’ve formatted and linted as per the project’s requirements before committing. It doesn’t prevent you running those tools as often as you like before committing, but it prevents those “forgetful” developers from committing non-compliant code.

See this for a discussion on how humans tend to learn to work around these obstacles.

While you are right that there is some formatting that is best done by humans, the vast majority of it is handled perfectly well by computers.

For example, making sure there’s the right amount of spaces to indent a block.

So how do you apply it selectively? The flags that turns these things off and on work on a line by line basis, which is not the level of resolution you’re referring to. The article mentions clang-format. It doesn’t allow mixed formatting, does it?

Indenting a block correctly seems like a trivial reason to auto-format a source file. Between text editors and the person at the keyboard responsible for creating logical things, committing incorrect indent would seem easy to avoid.

Excellent!
I suppose GUI git clients do the same (commit fails and changes shown?)

Excellent article.

  1. get your C/C++ sources compiling with Clang

Easier said than done! :slight_smile:
That said, this is another good argument for a command-line-centric build.

1 Like

This is a good point! What I’ve done in the past is insert the // clang-format off and on fences around blocks that I want to preserve from clang-format (your examples are great BTW!):

https://clang.llvm.org/docs/ClangFormatStyleOptions.html#disabling-formatting-on-a-piece-of-code

Bit ugly but usually rare when it’s needed.