architecture-decision-record icon indicating copy to clipboard operation
architecture-decision-record copied to clipboard

ci: add pre-commit hook to remove trailing whitespaces

Open flavono123 opened this issue 1 year ago • 5 comments

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 [email protected]

flavono123 avatar Nov 14 '23 08:11 flavono123

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

flavono123 avatar Nov 14 '23 08:11 flavono123

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?

  • https://prettier.io/

  • https://github.com/super-linter/super-linter

  • https://megalinter.github.io/

  • https://pre-commit.com/

  • https://trunk.io/

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!

joelparkerhenderson avatar Nov 14 '23 18:11 joelparkerhenderson

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: image https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#included-storage-and-minutes

flavono123 avatar Nov 15 '23 02:11 flavono123

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 avatar Nov 15 '23 05:11 joelparkerhenderson