Skip to content

Commit 3928625

Browse files
authored
Change how check-file-format.sh checks changed files (nhs-england-tools#130)
<!-- markdownlint-disable-next-line first-line-heading --> ## Description Prior to this patch, the `git diff` command used to identify which files to check for editorconfig compliance caught too many files, and did the expensive part of the check inside the per-file loop. By lucky chance, the editorconfig-checker container image has a `git` binary installed, so we can move the entire loop inside a single container invocation. Fixes nhs-england-tools#129. ## Context It makes the editorconfig check, which is called in a pre-commit hook, fast. ## Type of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply. --> - [ ] Refactoring (non-breaking change) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would change existing functionality) - [x] Bug fix (non-breaking change which fixes an issue)
1 parent 2ba6d0d commit 3928625

File tree

3 files changed

+50
-35
lines changed

3 files changed

+50
-35
lines changed

.github/actions/check-file-format/action.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@ runs:
77
shell: bash
88
run: |
99
export BRANCH_NAME=origin/${{ github.event.repository.default_branch }}
10-
./scripts/githooks/check-file-format.sh
10+
check=branch ./scripts/githooks/check-file-format.sh

scripts/config/pre-commit.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ repos:
1010
hooks:
1111
- id: check-file-format
1212
name: Check File Format
13-
entry: ./scripts/githooks/check-file-format.sh
13+
entry: check=staged-changes./scripts/githooks/check-file-format.sh
1414
language: script
1515
pass_filenames: false
1616
- repo: local

scripts/githooks/check-file-format.sh

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,61 +7,75 @@ set +e
77
# according to the style defined in the `.editorconfig` file.
88
#
99
# Usage:
10-
# $ ./check-file-format.sh
10+
# $ check={all,staged-changes,working-tree-changes,branch} [dry_run=true] ./check-file-format.sh
1111
#
1212
# Options:
1313
# BRANCH_NAME=other-branch-than-main # Branch to compare with, default is `origin/main`
14-
# ALL_FILES=true # Check all files, default is `false`
1514
# VERBOSE=true # Show all the executed commands, default is `false`
1615
#
1716
# Exit codes:
1817
# 0 - All files are formatted correctly
1918
# 1 - Files are not formatted correctly
2019
#
20+
#
21+
# The `check` parameter controls which files are checked, so you can
22+
# limit the scope of the check according to what is appropriate at the
23+
# point the check is being applied.
24+
#
25+
# check=all: check all files in the repository
26+
# check=staged-changes: check only files staged for commit.
27+
# check=working-tree-changes: check modified, unstaged files. This is the default.
28+
# check=branch: check for all changes since branching from $BRANCH_NAME
29+
#
30+
# If the `dry_run` parameter is set to a truthy value, the list of
31+
# files that ec would check is output, with no check done.
32+
#
2133
# Notes:
22-
# 1) Please, make sure to enable EditorConfig linting in your IDE. For the
34+
# Please, make sure to enable EditorConfig linting in your IDE. For the
2335
# Visual Studio Code editor it is `editorconfig.editorconfig` that is already
2436
# specified in the `./.vscode/extensions.json` file.
25-
# 2) Due to the file name escaping issue files are checked one by one.
2637

2738
# ==============================================================================
2839

2940
# SEE: https://hub.docker.com/r/mstruebing/editorconfig-checker/tags, use the `linux/amd64` os/arch
30-
image_version=2.7.0@sha256:0f8f8dd4f393d29755bef2aef4391d37c34e358d676e9d66ce195359a9c72ef3
31-
exit_code=0
41+
image_version=2.7.1@sha256:dd3ca9ea50ef4518efe9be018d669ef9cf937f6bb5cfe2ef84ff2a620b5ddc24
3242

3343
# ==============================================================================
3444

45+
3546
function main() {
3647

3748
cd $(git rev-parse --show-toplevel)
3849

39-
if is-arg-true "$ALL_FILES"; then
40-
41-
# Check all files
42-
docker run --rm --platform linux/amd64 \
43-
--volume $PWD:/check \
44-
mstruebing/editorconfig-checker:$image_version \
45-
ec \
46-
--exclude '.git/'
47-
48-
else
49-
50-
# Check changed files only
51-
files=$( (git diff --diff-filter=ACMRT --name-only ${BRANCH_NAME:-origin/main}; git diff --name-only) | sort | uniq )
52-
if [ -n "$files" ]; then
53-
while read file; do
54-
docker run --rm --platform linux/amd64 \
55-
--volume=$PWD:/check \
56-
mstruebing/editorconfig-checker:$image_version \
57-
ec \
58-
--exclude '.git/' \
59-
"$file"
60-
[ $? != 0 ] && exit_code=1 ||:
61-
done < <(echo "$files")
62-
fi
63-
64-
fi
50+
is-arg-true "$dry_run" && dry_run_opt="--dry-run"
51+
52+
check=${check:-working-tree-changes}
53+
case $check in
54+
"all")
55+
filter="git ls-files"
56+
;;
57+
"staged-changes")
58+
filter="git diff --diff-filter=ACMRT --name-only --cached"
59+
;;
60+
"working-tree-changes")
61+
filter="git diff --diff-filter=ACMRT --name-only"
62+
;;
63+
"branch")
64+
filter="git diff --diff-filter=ACMRT --name-only ${BRANCH_NAME:-origin/main}"
65+
;;
66+
*)
67+
echo "Unrecognised check mode: $check" >&2 && exit 1
68+
;;
69+
esac
70+
71+
72+
# We use /dev/null here as a backstop in case there are no files in the state
73+
# we choose. If the filter comes back empty, adding `/dev/null` onto it has
74+
# the effect of preventing `ec` from treating "no files" as "all the files".
75+
docker run --rm --platform linux/amd64 \
76+
--volume=$PWD:/check \
77+
mstruebing/editorconfig-checker:$image_version \
78+
sh -c "ec --exclude '.git/' $dry_run_opt \$($filter) /dev/null"
6579
}
6680

6781
function is-arg-true() {
@@ -75,8 +89,9 @@ function is-arg-true() {
7589

7690
# ==============================================================================
7791

92+
7893
is-arg-true "$VERBOSE" && set -x
7994

8095
main $*
8196

82-
exit $exit_code
97+
exit 0

0 commit comments

Comments
 (0)