From b421868b4ed135e211d5aea0eb2f55cf2dd2517b Mon Sep 17 00:00:00 2001
From: Lena <126529524+acuteenvy@users.noreply.github.com>
Date: Mon, 7 Oct 2024 11:32:11 +0200
Subject: [PATCH] scripts/*.sh: establish some conventions (#13999)

Co-authored-by: Sebastiaan Speck <12570668+sebastiaanspeck@users.noreply.github.com>
---
 scripts/check-pr.sh | 48 +++++++++++++++++++++------------------------
 scripts/test.sh     | 35 ++++++++++++++++++---------------
 2 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/scripts/check-pr.sh b/scripts/check-pr.sh
index 13f65add95..c5aa0d23d4 100755
--- a/scripts/check-pr.sh
+++ b/scripts/check-pr.sh
@@ -21,21 +21,21 @@
 
 # Check for duplicated pages.
 function check_duplicates {
-  local page=$1 # page path in the format 'pages<.language_code>/platform/pagename.md'
+  local page="$1" # page path in the format 'pages<.language_code>/platform/pagename.md'
   local parts
 
   readarray -td'/' parts < <(echo -n "$page")
 
-  local language_folder=${parts[0]}
-  
-  if [[ "$language_folder" != "pages" ]]; then # only check for duplicates in English
+  local language_folder="${parts[0]}"
+
+  if [[ $language_folder != "pages" ]]; then # only check for duplicates in English
     return 1
   fi
 
-  local platform=${parts[1]}
-  local file=${parts[2]}
+  local platform="${parts[1]}"
+  local file="${parts[2]}"
 
-  case "$platform" in
+  case $platform in
     common) # skip common-platform
       ;;
     *) # check if page already exists under common
@@ -47,14 +47,14 @@ function check_duplicates {
 }
 
 function check_missing_english_page() {
-  local page=$1
+  local page="$1"
   local english_page="pages/${page#pages*\/}"
 
-  if [[ "$page" = "$english_page" ]]; then
+  if [[ $page == "$english_page" ]]; then
     return 1
   fi
-  
-  if [[ ! -f "$english_page" ]]; then
+
+  if [[ ! -f $english_page ]]; then
     printf "\x2d $MSG_NOT_EXISTS" "$page" "$english_page"
   fi
 }
@@ -86,25 +86,21 @@ function strip_commands() {
 }
 
 function check_outdated_page() {
-  local page=$1
+  local page="$1"
   local english_page="pages/${page#pages*\/}"
   local command_regex='^`[^`]\+`$'
 
-  if [[ "$page" = "$english_page" ]] || [[ ! -f "$english_page" ]]; then
+  if [[ $page == "$english_page" || ! -f $english_page ]]; then
     return 1
   fi
 
-  local english_commands
-  english_commands=$(count_commands "$english_page" "$command_regex")
-  local commands
-  commands=$(count_commands "$page" "$command_regex")
+  local english_commands commands english_commands_as_string commands_as_string
+  english_commands="$(count_commands "$english_page" "$command_regex")"
+  commands="$(count_commands "$page" "$command_regex")"
+  english_commands_as_string="$(strip_commands "$english_page" "$command_regex")"
+  commands_as_string="$(strip_commands "$page" "$command_regex")"
 
-  local english_commands_as_string
-  english_commands_as_string=$(strip_commands "$english_page" "$command_regex")
-  local commands_as_string
-  commands_as_string=$(strip_commands "$page" "$command_regex")
-  
-  if [[ "$english_commands" != "$commands" ]]; then
+  if [[ $english_commands != "$commands" ]]; then
     printf "\x2d $MSG_OUTDATED" "$page" "based on number of commands"
   elif [[ "$english_commands_as_string" != "$commands_as_string" ]]; then
     printf "\x2d $MSG_OUTDATED" "$page" "based on the command contents itself"
@@ -114,7 +110,7 @@ function check_outdated_page() {
 function check_more_info_link() {
   local page=$1
 
-  if grep "$page" "more-info-links.txt" > /dev/null; then
+  if grep -q "$page" "more-info-links.txt"; then
       printf "\x2d $MSG_MORE_INFO" "$page"
   fi
 }
@@ -122,7 +118,7 @@ function check_more_info_link() {
 function check_page_title() {
   local page=$1
 
-  if grep "$page" "page-titles.txt" > /dev/null; then
+  if grep -q "$page" "page-titles.txt"; then
       printf "\x2d $MSG_PAGE_TITLE" "$page"
   fi
 }
@@ -133,7 +129,7 @@ function check_diff {
   local line
   local entry
 
-  git_diff=$(git diff --name-status --find-copies-harder --diff-filter=ACM origin/main -- pages*/)
+  git_diff="$(git diff --name-status --find-copies-harder --diff-filter=ACM origin/main -- pages*/)"
 
   if [[ -n $git_diff ]]; then
     echo -e "Check PR: git diff:\n$git_diff" >&2
diff --git a/scripts/test.sh b/scripts/test.sh
index 90da6cd4d2..d32574e27e 100755
--- a/scripts/test.sh
+++ b/scripts/test.sh
@@ -18,10 +18,10 @@ function exists {
 # but we want to only print if there are actual errors, and not
 # the "All done!" success message.
 function run_black {
-  target_black_version=$(awk -F '==' '$1 == "black" { print $2 }' < requirements.txt)
+  target_black_version="$(awk -F '==' '$1 == "black" { print $2 }' < requirements.txt)"
 
   if grep -qw black <<< "$(pip3 --disable-pip-version-check list)"; then
-    errs=$(python3 -m black scripts --check --required-version "${target_black_version}" 2>&1 || true)
+    errs="$(python3 -m black scripts --check --required-version "$target_black_version" 2>&1 || true)"
   fi
 
   if [[ -z $errs ]]; then
@@ -31,18 +31,18 @@ function run_black {
       return 0
     fi
 
-    errs=$(black scripts --check --required-version "${target_black_version}" 2>&1 || true)
+    errs="$(black scripts --check --required-version "$target_black_version" 2>&1 || true)"
   fi
 
-  if [[ ${errs} == *"does not match the running version"* ]]; then
+  if [[ $errs == *"does not match the running version"* ]]; then
     echo -e "Skipping black check, required version not available, try running: pip3 install -r requirements.txt"
     return 0
   fi
 
   # We want to ignore the exit code from black on failure so that we can
   # do the conditional printing below
-  if [[ ${errs} != "All done!"* ]]; then
-     echo -e "${errs}" >&2
+  if [[ $errs != "All done!"* ]]; then
+     echo -e "$errs" >&2
      return 1
   fi
 }
@@ -64,9 +64,9 @@ function run_pytest {
     return 0
   fi
 
-  errs=$(pytest scripts/*.py 2>&1 || true)
-  if [[ ${errs} == *"failed"* ]]; then
-    echo -e "${errs}" >&2
+  errs="$(pytest scripts/*.py 2>&1 || true)"
+  if [[ $errs == *"failed"* ]]; then
+    echo -e "$errs" >&2
     return 1
   fi
 }
@@ -78,7 +78,7 @@ function run_shellcheck {
     return 0
   fi
 
-  shellcheck scripts/*.sh
+  shellcheck --enable require-double-brackets,avoid-nullary-conditions,quote-safe-variables scripts/*.sh
 }
 
 # Default test function, run by `npm test`.
@@ -87,14 +87,17 @@ function run_tests {
   tldr-lint ./pages
   for f in ./pages.*; do
     checks="TLDR104"
-    if [[ -L $f ]]; then
-        continue
-    elif [[ $f == *ar* || $f == *bn* || $f == *fa* || $f == *hi* || $f == *ja* || $f == *ko* || $f == *lo* || $f == *ml* || $f == *ne* || $f == *ta* || $f == *th* || $f == *tr* ]]; then
+    # Skip the `pages.en` symlink.
+    [[ -h $f ]] && continue
+    case $f in
+      *ar*|*bn*|*fa*|*hi*|*ja*|*ko*|*lo*|*ml*|*ne*|*ta*|*th*|*tr*)
         checks+=",TLDR003,TLDR004,TLDR015"
-    elif [[ $f == *zh* || $f == *zh_TW* ]]; then
+      ;;
+      *zh*)
         checks+=",TLDR003,TLDR004,TLDR005,TLDR015"
-    fi
-    tldr-lint --ignore $checks "${f}"
+      ;;
+    esac
+    tldr-lint --ignore "$checks" "$f"
   done
   run_black
   run_flake8