Skip to content

Commit

Permalink
Change how check-file-format.sh checks changed files (#130)
Browse files Browse the repository at this point in the history
<!-- 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 #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)
  • Loading branch information
regularfry authored Sep 26, 2023
1 parent 2ba6d0d commit 3928625
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 35 deletions.
2 changes: 1 addition & 1 deletion .github/actions/check-file-format/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ runs:
shell: bash
run: |
export BRANCH_NAME=origin/${{ github.event.repository.default_branch }}
./scripts/githooks/check-file-format.sh
check=branch ./scripts/githooks/check-file-format.sh
2 changes: 1 addition & 1 deletion scripts/config/pre-commit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ repos:
hooks:
- id: check-file-format
name: Check File Format
entry: ./scripts/githooks/check-file-format.sh
entry: check=staged-changes./scripts/githooks/check-file-format.sh
language: script
pass_filenames: false
- repo: local
Expand Down
81 changes: 48 additions & 33 deletions scripts/githooks/check-file-format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,61 +7,75 @@ set +e
# according to the style defined in the `.editorconfig` file.
#
# Usage:
# $ ./check-file-format.sh
# $ check={all,staged-changes,working-tree-changes,branch} [dry_run=true] ./check-file-format.sh
#
# Options:
# BRANCH_NAME=other-branch-than-main # Branch to compare with, default is `origin/main`
# ALL_FILES=true # Check all files, default is `false`
# VERBOSE=true # Show all the executed commands, default is `false`
#
# Exit codes:
# 0 - All files are formatted correctly
# 1 - Files are not formatted correctly
#
#
# The `check` parameter controls which files are checked, so you can
# limit the scope of the check according to what is appropriate at the
# point the check is being applied.
#
# check=all: check all files in the repository
# check=staged-changes: check only files staged for commit.
# check=working-tree-changes: check modified, unstaged files. This is the default.
# check=branch: check for all changes since branching from $BRANCH_NAME
#
# If the `dry_run` parameter is set to a truthy value, the list of
# files that ec would check is output, with no check done.
#
# Notes:
# 1) Please, make sure to enable EditorConfig linting in your IDE. For the
# Please, make sure to enable EditorConfig linting in your IDE. For the
# Visual Studio Code editor it is `editorconfig.editorconfig` that is already
# specified in the `./.vscode/extensions.json` file.
# 2) Due to the file name escaping issue files are checked one by one.

# ==============================================================================

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

# ==============================================================================


function main() {

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

if is-arg-true "$ALL_FILES"; then

# Check all files
docker run --rm --platform linux/amd64 \
--volume $PWD:/check \
mstruebing/editorconfig-checker:$image_version \
ec \
--exclude '.git/'

else

# Check changed files only
files=$( (git diff --diff-filter=ACMRT --name-only ${BRANCH_NAME:-origin/main}; git diff --name-only) | sort | uniq )
if [ -n "$files" ]; then
while read file; do
docker run --rm --platform linux/amd64 \
--volume=$PWD:/check \
mstruebing/editorconfig-checker:$image_version \
ec \
--exclude '.git/' \
"$file"
[ $? != 0 ] && exit_code=1 ||:
done < <(echo "$files")
fi

fi
is-arg-true "$dry_run" && dry_run_opt="--dry-run"

check=${check:-working-tree-changes}
case $check in
"all")
filter="git ls-files"
;;
"staged-changes")
filter="git diff --diff-filter=ACMRT --name-only --cached"
;;
"working-tree-changes")
filter="git diff --diff-filter=ACMRT --name-only"
;;
"branch")
filter="git diff --diff-filter=ACMRT --name-only ${BRANCH_NAME:-origin/main}"
;;
*)
echo "Unrecognised check mode: $check" >&2 && exit 1
;;
esac


# We use /dev/null here as a backstop in case there are no files in the state
# we choose. If the filter comes back empty, adding `/dev/null` onto it has
# the effect of preventing `ec` from treating "no files" as "all the files".
docker run --rm --platform linux/amd64 \
--volume=$PWD:/check \
mstruebing/editorconfig-checker:$image_version \
sh -c "ec --exclude '.git/' $dry_run_opt \$($filter) /dev/null"
}

function is-arg-true() {
Expand All @@ -75,8 +89,9 @@ function is-arg-true() {

# ==============================================================================


is-arg-true "$VERBOSE" && set -x

main $*

exit $exit_code
exit 0

0 comments on commit 3928625

Please sign in to comment.