From d13cde52937927e82dc912f795648a2e8e73c6e3 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Mon, 8 Apr 2024 01:04:22 -0400
Subject: [PATCH] Extract constants; rename functions/variables for clarity

This also refactors slightly in other ways, clarifies a comment,
and improves an error message.
---
 etc/copy-packetline.sh | 86 ++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 41 deletions(-)

diff --git a/etc/copy-packetline.sh b/etc/copy-packetline.sh
index 81d8c72378b..688f16aa4f0 100755
--- a/etc/copy-packetline.sh
+++ b/etc/copy-packetline.sh
@@ -2,12 +2,16 @@
 
 set -euC -o pipefail
 
+readonly source_dir='gix-packetline/src'
+readonly target_parent_dir='gix-packetline-blocking'
+readonly target_dir="$target_parent_dir/src"
+
 function fail () {
   printf '%s: error: %s\n' "$0" "$1" >&2
   exit 1
 }
 
-function chdir_toplevel() {
+function chdir_toplevel () {
   local root_padded root
 
   # Find the working tree's root. (Padding is for the trailing-newline case.)
@@ -29,25 +33,26 @@ function merging () {
   test -e "$git_dir/MERGE_HEAD"
 }
 
-function target_status () {
-  git status --short --ignored=traditional -- gix-packetline-blocking/src ||
+function target_dir_status () {
+  git status --short --ignored=traditional -- "$target_dir" ||
     fail 'git-status failed'
 }
 
-function check_target () {
-  if ! test -e "gix-packetline-blocking/src"; then
+function check_target_dir () {
+  if ! test -e "$target_dir"; then
     # The target does not exist on disk, so nothing will be lost. Proceed.
     return
   fi
 
   if merging; then
     # In a merge, it would be confusing to replace anything at the target.
-    if target_status | grep -q '^'; then
+    if target_dir_status | grep -q '^'; then
       fail 'target exists, and a merge is in progress'
     fi
   else
     # We can lose data if anything of value at the target is not in the index.
-    if target_status | grep -q '^.[^ ]'; then
+    # (Even unstaged deletions, for we can forget what was and wasn't deleted.)
+    if target_dir_status | grep -q '^.[^ ]'; then
       fail 'target exists, with unstaged changes or ignored files'
     fi
   fi
@@ -73,74 +78,73 @@ function first_line_ends_crlf () {
 }
 
 function make_header () {
-  local source endline
+  local source_file endline
 
-  source="$1"
+  source_file="$1"
   endline="$2"
 
   # shellcheck disable=SC2016  # The backticks are intentionally literal.
   printf '//! DO NOT EDIT - this is a copy of %s. Run `just copy-packetline` to update it.%s%s' \
-    "$source" "$endline" "$endline"
+    "$source_file" "$endline" "$endline"
 }
 
 function copy_with_header () {
-  local source target endline
+  local source_file target_file endline
 
-  source="$1"
-  target="$2"
+  source_file="$1"
+  target_file="$2"
 
-  if first_line_ends_crlf "$source"; then
+  if first_line_ends_crlf "$source_file"; then
     endline=$'\r\n'
   else
     endline=$'\n'
   fi
 
-  make_header "$source" "$endline" | cat - -- "$source" >"$target"
+  make_header "$source_file" "$endline" |
+    cat - -- "$source_file" >"$target_file"
 }
 
 function generate_one () {
-  local source target
+  local source_file target_file
 
-  source="$1"
-  target="gix-packetline-blocking/src/${source#gix-packetline/src/}"
+  source_file="$1"
+  target_file="$target_dir/${source_file#"$source_dir"/}"
 
-  if test -d "$source"; then
-    mkdir -p -- "$target"
-  elif test -L "$source"; then
+  if test -d "$source_file"; then
+    mkdir -p -- "$target_file"
+  elif test -L "$source_file"; then
     # Cover this case separately, for more useful error messages.
-    fail "source file is symbolic link: $source"
-  elif ! test -f "$source"; then
+    fail "source file is symbolic link: $source_file"
+  elif ! test -f "$source_file"; then
     # This covers less common kinds of files we can't or shouldn't process.
-    fail "source file neither regular file nor directory: $source"
-  elif [[ "$source" =~ \.rs$ ]]; then
-    copy_with_header "$source" "$target"
+    fail "source file neither regular file nor directory: $source_file"
+  elif [[ "$source_file" =~ \.rs$ ]]; then
+    copy_with_header "$source_file" "$target_file"
   else
-    fail "source file not named as Rust source code: $source"
+    fail "source file not named as Rust source code: $source_file"
   fi
 }
 
 function generate_all () {
-  local source
-
-  chdir_toplevel
+  local source_file
 
-  if ! test -d gix-packetline/src; then
-    fail 'no source directory: gix-packetline/src'
+  if ! test -d "$source_dir"; then
+    fail "no source directory: $source_dir"
   fi
-  if ! test -d gix-packetline-blocking; then
-    fail 'no target parent directory: gix-packetline-blocking'
+  if ! test -d "$target_parent_dir"; then
+    fail "no target parent directory: $target_parent_dir"
   fi
+  check_target_dir
 
-  check_target
-
-  rm -rf gix-packetline-blocking/src  # No trailing "/" as it may be a symlink.
-  if test -e gix-packetline-blocking/src; then
-    fail 'unable to remove target'
+  rm -rf -- "$target_dir"  # It may be a directory, symlink, or regular file.
+  if test -e "$target_dir"; then
+    fail 'unable to remove target location'
   fi
 
-  find gix-packetline/src/ -print0 | while IFS= read -r -d '' source; do
-    generate_one "$source"
+  find "$source_dir/" -print0 | while IFS= read -r -d '' source_file; do
+    generate_one "$source_file"
   done
 }
 
+chdir_toplevel
 generate_all