ci: add pre-commit hook to remove trailing whitespaces

flavono123 commented 1 year ago

Goal

reduce the tedious chore of removing trailing whitespaces
ref. https://github.com/joelparkerhenderson/architecture-decision-record/pull/69#issuecomment-1804542664

Descriptions

add pre-commit hook to remove trailing whitespaces
flow
- collect the staged files
- check if any of these files have whitespace issues
- if so, cleanup process
- loop through the files and apply git-stripspace

Requirements

git config core.hooksPath .githooks to activate the hook
git config core.quotepath false for non-ascii paths
- also need to be documented(where?)

Test

trailing spaces in following diff are cleaned up

works on all type of file even for codes, not only for markdowns(docs)


diff --git a/.githooks/pre-commit b/.githooks/pre-commit
new file mode 100644
index 0000000..4d25c49
--- /dev/null
+++ b/.githooks/pre-commit
@@ -0,0 +1,37 @@
+#!/bin/sh

+# Temporary file to store the list of files that need cleanup +TEMP_FILE=$(mktemp)
+# Collect staged files with potential whitespace issues +git diff --cached --name-only --diff-filter=ACM > "$TEMP_FILE"
+# Check if any of these files have whitespace issues +NEEDS_CLEANUP=false +while read -r FILE; do
if ! git diff --cached --check -- "$FILE"; then
NEEDS_CLEANUP=true
break
fi +done < "$TEMP_FILE"
+# Cleanup process +if [ "$NEEDS_CLEANUP" = true ]; then
Loop through the files and apply git-stripspace
while read -r FILE; do
git show ":$FILE" | git stripspace > "$FILE"
git add "$FILE"
done < "$TEMP_FILE"
Inform the user
echo 'Cleanup trailing whitespaces is done' +fi
+# Clean up - remove the temporary file +rm "$TEMP_FILE"
+if [ "$NEEDS_CLEANUP" = false ]; then
echo "no need to cleanup" +fi +# Continue with the original commit +exit 0 diff --git "a/locales/ko/\355\205\234\355\224\214\353\246\277/\352\262\260\354\240\225 \352\270\260\353\241\235 \355\205\234\355\224\214\353\246\277\353\263\204 edgex/index.md" "b/locales/ko/\355\205\234\355\224\214\353\246\277/\352\262\260\354\240\225 \352\270\260\353\241\235 \355\205\234\355\224\214\353\246\277\353\263\204 edgex/index.md" index f4c35a5..bdbed75 100644 --- "a/locales/ko/\355\205\234\355\224\214\353\246\277/\352\262\260\354\240\225 \352\270\260\353\241\235 \355\205\234\355\224\214\353\246\277\353\263\204 edgex/index.md" +++ "b/locales/ko/\355\205\234\355\224\214\353\246\277/\352\262\260\354\240\225 \352\270\260\353\241\235 \355\205\234\355\224\214\353\246\277\353\263\204 edgex/index.md" @@ -5,7 +5,7 @@ EdgeX Foundry의 ADR 템플릿입니다. 출처: https://docs.edgexfoundry.org/2.3/design/adr/template/

-### 제출자 +### 제출자

ADR 제출자를 나열합니다.



Signed-off-by: flavono123 <flavono123@gmail.com>

flavono123 commented 1 year ago

a suggestion for https://github.com/joelparkerhenderson/architecture-decision-record/pull/69#issuecomment-1804542664 by just creating this PR

joelparkerhenderson commented 1 year ago

Good idea. A few questions....

Can this kind of proofreading and changing have more capabilities?

Can the code be minimized in shell, by calling on installed software, such as installed on a CI server and/or GitHub actions and/or the developer's system?

Do you foresee what you're aiming for generally heading along the direction of a formatter and/or linter?

A few questions about the shell code...

Should this be ACMR?

--diff-filter=ACM

I believe that stripspace is intended mostly for metadata, and there's a different whitespace=fix for committed files...?

git stripspace

Ideally can a successful run be silent, akin to the Unix convention of silent success?

-if [ "$NEEDS_CLEANUP" = false ]; then
-  echo "no need to cleanup"
-fi

Can it run with guard flags, such as set -euf? Such as this:

https://github.com/SixArm/unix-shell-script-tactics/tree/main/doc/protect-scripts-by-using-set-flags

Can you trap the mktemp? Because it's a good idea to trap an exception, such as this:

https://github.com/SixArm/unix-shell-script-tactics/tree/main/doc/temporary-file-using-mktemp-and-trap

Could it be worthwhile to look for a longer-term larger-scope solution to make your idea even more powerful and valuable? Here are some that turned up when I looked... do any of these look like they can give you more capabilities?

Overall, my opinion is your code is fine to commit and use. I favor bias for action, and you're doing action. I appreciate you!

flavono123 commented 1 year ago

thank you for valuable and various points. but by answering the last one, that would resolve others too i think. and totally agree with the idea, using "a longer-term larger-scope solution", open-source standards, in other words, rather than the custom code snippet should be maintained.

in a nutshell, in advance, i prefer/think the choice is github action + megalinter:

i'm familiar with github action.
but not for megalinter; actually knowing that this time, though:
- it provides a reusable workflow for github action(https://megalinter.io/latest/install-github/). that's enough
- free(https://megalinter.io/latest/license-explanations/)
- transparent
- what it uses for md(https://megalinter.io/latest/descriptors/markdown/)
- even it supports other many langs(too many features, and other linters you found also are)
- can pruning other no required features(https://megalinter.io/latest/config-activation/)
dismissed:
- prettier is not a linter(https://prettier.io/docs/en/comparison) and we want that, such as removing trailing spaces, i think
- super-linter is beaten by megalinter? 👆
- pre-commit is another git hook. as a workflow, we should maintain that
- trunk; no good experience in person, as a vscode extension

the matter is pricing, no for megalinter but github action. but personal free tier is enough i guess: https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#included-storage-and-minutes

joelparkerhenderson commented 1 year ago

Excellent research! Yes GitHub Action + MegaLinter sounds good. And I expect that the pricing on the free tier is OK.

If you want to try it, go ahead on this whole repo.

joelparkerhenderson / architecture-decision-record