← Back to team overview

kicad-developers team mailing list archive

Re: Via stitching


On 20/05/2019 23:48, Frank Severinsen wrote:

When using clang-format on the entire file, it more or less changed every line in one way or another. should this be committed as well or is git clutter worse than codingstyle issues?

You should not commit unrelated formatting clutter in the same commit as actual code changes. Just format the changed bits. If that's too inconsistent, either reformat as a separate commit, or match the existing style in the area, even if it's "wrong".

The trick here is to use git-clang-format. This will only format the lines you have changed. There are instructions in the developer documentation[1], a git-hook to report errors when you commit, and a script to help you fix it.

@Wayne: I have a patch here for the formatting document to add some aliases to make it easier. It finally dawned on me that we already distribute aliases for the 'fixes' alias, so let's do it for the formatting too!

[1]: http://docs.kicad-pcb.org/doxygen/md_Documentation_development_coding-style-policy.html
>From fb053d8cc27179542320bc3b297087aa28fd440a Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 22 May 2019 10:26:20 +0100
Subject: [PATCH] Format: add some formatting aliases, improve dev docs

The aliases are easier than calling check_coding.sh manually,
and we already provide an alias file for the fixes alias, so
do the same for these.
 .../development/coding-style-policy.md        | 34 +++++++++++++------
 helpers/git/format_alias                      | 10 ++++++
 tools/check_coding.sh                         |  2 +-
 3 files changed, 34 insertions(+), 12 deletions(-)
 create mode 100644 helpers/git/format_alias

diff --git a/Documentation/development/coding-style-policy.md b/Documentation/development/coding-style-policy.md
index 420c9041c..46514c536 100644
--- a/Documentation/development/coding-style-policy.md
+++ b/Documentation/development/coding-style-policy.md
@@ -76,25 +76,37 @@ to "true" for the KiCad repo:
     git config kicad.check-format true
 Without this config, the format checker will not run on commit, but you can
-still check files staged for commit manually:
-    tools/check_coding.sh --diff
+still check files staged for commit manually (see below).
 If the hook is 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 your
-staged changes automatically with this tool:
-    tools/check_coding.sh
+have caused any style violations (only in your changed code). You can then fix
+the errors, either manually, or with the tools below.
-Or you can proceed anyway, if you are sure your style is correct:
+If you are warned about formatting errors, but you are sure your style is correct,
+you can still commit:
     git commit --no-verify
-The `check_coding.sh` tool has other modes:
+### Correcting Formatting Errors ## {#correcting-formatting-errors}
-* Make (or see only) changes to files modified in the previous commit:
-    * `check_coding.sh --amend [--diff]`
+There is a Git aliases file that provides the right commands to show and correct
+formatting errors. Add to your repository config by running this command from
+the source directory:
+    git config --add include.path $(pwd)/helpers/git/format_alias
+Then you can use the following aliases:
+* `git check-format`: show any formatting warnings (but make no changes)
+* `git fix-format`: correct formatting (you will need to `git add` afterwards)
+These aliases use a script, `tools/check-coding.sh`, which takes care of only
+checking the formatting for files that should be formatted. This script has
+other uses:
+* Make (or see only) violations in files modified in the previous commit (useful
+when interactive-rebasing):
+    * `check_coding.sh --amend [--diff]`
 # 2. Naming Conventions # {#naming_conventions}
 Before delving into anything as esoteric as indentation and formatting,
diff --git a/helpers/git/format_alias b/helpers/git/format_alias
new file mode 100644
index 000000000..bac8c5b9a
--- /dev/null
+++ b/helpers/git/format_alias
@@ -0,0 +1,10 @@
+	# Alias to check the formatting since the last commit and show the issues
+	# that are found. No files will be changed. Only lines changed since the
+	# last commit are considered (and only for files that should be reformatted)
+	check-format = "! f() { tools/check_coding.sh --diff; }; f"
+	# Alias to check and fix formatting issues in files changed since the last
+	# commit. Only lines changed since the last commit are considered (and only
+	# for files that should be reformatted)
+	fix-format = "! f() { tools/check_coding.sh; }; f"
diff --git a/tools/check_coding.sh b/tools/check_coding.sh
index c5f730dfb..2e6c967f9 100755
--- a/tools/check_coding.sh
+++ b/tools/check_coding.sh
@@ -30,7 +30,7 @@ usage='usage: check_coding.sh [<options>] [--]
      --diff                     Only show proposed changes, do not format files
-     --cached                   Re-format changes currently staged for commit
+     --cached                   Re-format changes currently staged for commit (default)
      --amend                    Re-format changes made in the previous commit

Follow ups