kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

Hi,

This is a patch to add a hook to check formatting for *changed lines
only* when committing to Git.

It is good that KiCad has a style, but it is a highly unique one and
many people will not be used to writing it. Making it easier for
people to find mistakes without having to waste the maintainers' time
and their own ping-ponging patches on the mailing list should be the
aim.

By using git-clang-format, we can allow style checking on the changes
only, so we avoid a huge reformat of the whole code-base (which is,
despite best efforts, highly inconsistent, so there would be many many
changes).

Instructions are in the dev docs, but basically:

* Install `git-clang-format` (comes with the `clang` package on Arch, at least)
* Enable the hooks: `git config core.hooksPath .githooks`
* Make sure it uses the right style source: `git config clangFormat.style file`

Then it should just work - if you try to commit badly-styled code it
will tell you what it would change if you ran git clang-format on it.
If you disagree, you can commit anyway with `git commit --no-verify`
(aka `-n`).

Also included is a general "hook-chain" implementation for running
multiple hooks.

Cheers,

John

From 7221522aebba949446979437f55de1995e936c4b Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Fri, 19 Oct 2018 14:36:03 +0100
Subject: [PATCH] Add git format commit hook

This adds a pre-commit hook to warn of any style errors.

Also adds a 'hook-chain' script to simplify future hooks.

Add dev-doc note about how to use the formatter.
---
 .githooks/hook-chain                          | 49 +++++++++++++++++++
 .githooks/pre-commit                          |  1 +
 .githooks/pre-commit.d/50-check-format        | 43 ++++++++++++++++
 .../development/coding-style-policy.md        | 30 ++++++++++++
 4 files changed, 123 insertions(+)
 create mode 100755 .githooks/hook-chain
 create mode 120000 .githooks/pre-commit
 create mode 100755 .githooks/pre-commit.d/50-check-format

diff --git a/.githooks/hook-chain b/.githooks/hook-chain
new file mode 100755
index 000000000..bb480d60c
--- /dev/null
+++ b/.githooks/hook-chain
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+#
+# Git "hook chain", used to execute multiple scripts per hook.
+# To use:
+#  * create a directory called <hookname>.d
+#  * add scripts to this directory (executable)
+#  * ln -s hook-chain <hookname>
+#
+# Now the scripts in that directory should be called in order.
+#
+# Set $HOOKCHAIN_DEBUG to see the names of invoked scripts.
+#
+# Based on script by Oliver Reflalo:
+# https://stackoverflow.com/questions/8730514/chaining-git-hooks
+#
+
+hookname=`basename $0`
+
+# Temp file for stdin, cleared at exit
+FILE=`mktemp`
+trap 'rm -f $FILE' EXIT
+cat - > $FILE
+
+# Git hooks directory (this dir)
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
+
+# Execute hooks in the directory one by one
+for hook in $DIR/$hookname.d/*;
+do
+    if [ -x "$hook" ]; then
+
+    if [ "$HOOKCHAIN_DEBUG" ]; then
+        echo "Running hook $hook"
+    fi
+
+        cat $FILE | $hook "$@"
+        status=$?
+
+        if [ $status -ne 0 ]; then
+            echo "Hook $hook failed with error code $status"
+            echo "To commit anyway, use --no-verify"
+            exit $status
+        else
+        if [ "$HOOKCHAIN_DEBUG" ]; then
+                echo "Hook passed: $hook"
+            fi
+    fi
+    fi
+done
diff --git a/.githooks/pre-commit b/.githooks/pre-commit
new file mode 120000
index 000000000..84d936b2f
--- /dev/null
+++ b/.githooks/pre-commit
@@ -0,0 +1 @@
+hook-chain
\ No newline at end of file
diff --git a/.githooks/pre-commit.d/50-check-format b/.githooks/pre-commit.d/50-check-format
new file mode 100755
index 000000000..d6d19cea4
--- /dev/null
+++ b/.githooks/pre-commit.d/50-check-format
@@ -0,0 +1,43 @@
+#!/usr/bin/env bash
+#
+# Runs clang-format on changed regions before commit.
+#
+# Remaining installation checks/instructions will be printed when you commit.
+#
+# Based on clang-format pre-commit hook by Alex Eagle
+# https://gist.github.com/alexeagle/c8ed91b14a407342d9a8e112b5ac7dab
+
+check_clang_format() {
+    if hash git-clang-format 2>/dev/null; then
+        return
+    else
+        echo "SETUP ERROR: no git-clang-format executable found, or it is not executable"
+        exit 1
+    fi
+}
+
+check_git_config() {
+    if [[ "$(git config --get clangFormat.style)" != "file" ]]; then
+        echo "SETUP ERROR: git config clangFormat.style is wrong (should be 'file'). Fix this with:"
+        echo "git config clangFormat.style file"
+        exit 1
+    fi
+}
+
+check_clang_format
+check_git_config
+
+readonly out=$(git clang-format -v --diff)
+
+# In these cases, there is no formatting issues, so we succeed
+if [[ "$out" == *"no modified files to format"* ]]; then exit 0; fi
+if [[ "$out" == *"clang-format did not modify any files"* ]]; then exit 0; fi
+
+# Any other case implies formatting results
+echo "ERROR: you need to run git clang-format on your commit"
+
+# print the errors to show what's the issue
+git clang-format -v --diff
+
+# fail the pre-commit
+exit 1
diff --git a/Documentation/development/coding-style-policy.md b/Documentation/development/coding-style-policy.md
index 3267defa4..1d3fc87f1 100644
--- a/Documentation/development/coding-style-policy.md
+++ b/Documentation/development/coding-style-policy.md
@@ -50,6 +50,35 @@ developers. The other KiCad developers will appreciate your effort.
 **Do not modify this document without the consent of the project
 leader. All changes to this document require approval.**
 
+## 1.3 Tools
+
+There are some tools that can help you format your code easily.
+
+[`clang-format`][clang-format] is a formatting tool that can both be used to
+provide code-style automation to your editor of choice, as well as allow git to
+check formatting when committing (using a "Git hook"). You should install this
+program to be able to use the Git hooks.
+
+The style config file is `_clang-format`, and should be picked up automatically
+by `clang-format` when the `--style=file` option is set.
+
+To enable the Git hooks (only needs to be done once per Git repo):
+
+    git config core.hooksPath .githooks
+
+Set the `git clang-format` tool to use the provided `_clang-format` file:
+
+    git config clangFormat.style file
+
+Now, when you commit a change, you will be told if you have caused any style
+violations (only in your changed code). You can fix them automatically with:
+
+    git clang-format
+
+Or you can proceed anyway, if you are sure your style is correct:
+
+    git commit --no-verify
+
 
 # 2. Naming Conventions # {#naming_conventions}
 Before delving into anything as esoteric as indentation and formatting,
@@ -788,6 +817,7 @@ learn something new.
 - [C++ Operator Overloading Guidelines][overloading]
 - [Wikipedia's Programming Style Page][style]
 
+[clang-format]: https://clang.llvm.org/docs/ClangFormat.html
 [cppstandard]:http://www.possibility.com/Cpp/CppCodingStandard.html
 [kernel]:http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/Documentation/CodingStyle
 [overloading]:http://www.cs.caltech.edu/courses/cs11/material/cpp/donnie/cpp-ops.html
-- 
2.19.0