kicad-developers team mailing list archive

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

Hi Wayne,

If you have git-clang-format installed (you will do if you have clang on
Arch, for example), you can just run `git clang-format --diff` to see
violations in the uncommitted changes (according to the `_clang-format`
file). This is already possible, though you *should* still set `git config
clangFormat.style file` to ensure it uses the KiCad `_clang-format` file.

I have added a env var switch to the format pre-commit hook. If you set
`KICAD_CHECK_FORMAT` to something in the shell, it will run the check. Then
the default action is "do not check at all" (as the var is not set).
Developers can then add to shell configs and/or set/unset/export as they
need.

We can use the same technique to selectively enable other hooks in future.

Documentation also updated.

Cheers,

John

On 22 October 2018 17:50:41 BST, Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:
>
> Hey John,
>
> Is there a way to set this up in git where you can run clang-format on
> demand rather than using --no-verify to ignore the hook?  I don't know
> if I want to force this on developers so it would be nice if it would
> only be run when requested.
>
> Cheers,
>
> Wayne
>
> On 10/19/2018 12:30 PM, John Beard wrote:
>
>> 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
>> ------------------------------
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>> ------------------------------
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>

From 289b324d1d6734536ae11f0922c38f52ac1ca8d1 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        | 49 +++++++++++++++++++
 .../development/coding-style-policy.md        | 35 +++++++++++++
 4 files changed, 134 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..dacab7579
--- /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..8de58b51e
--- /dev/null
+++ b/.githooks/pre-commit.d/50-check-format
@@ -0,0 +1,49 @@
+#!/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
+
+# Set KICAD_CHECK_FORMAT to allow this hook to run
+# if not set, the hook always succeeds.
+if [ -z "$KICAD_CHECK_FORMAT" ]; then
+    exit 0
+fi
+
+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..5287d2897 100644
--- a/Documentation/development/coding-style-policy.md
+++ b/Documentation/development/coding-style-policy.md
@@ -50,6 +50,40 @@ 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
+
+Then, to enable the format checker, set the `KICAD_CHECK_FORMAT` environment
+variable in your shell. Without this variable, the format checker will not
+run on commit, but you can still use `git clang-format --diff` to check manually.
+
+If enabled, 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 +822,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