I tried to do too many things in one function, vastly overcomplicating what should have been _this_ all along

Signed-off-by: Adam Warner <me@adamwarner.co.uk>

advanced/Scripts/utils.sh

@@ -17,43 +17,49 @@
 #  - New functions must have a test added for them in test/test_any_utils.py
 
 #######################
-# Takes either
+# Takes Three arguments: file, key, and value.
-#   - Three arguments: file, key, and value.
-#   - Two arguments: file, and key.
 #
 # Checks the target file for the existence of the key
 #   - If it exists, it changes the value
 #   - If it does not exist, it adds the value
 #
 # Example usage:
-# addOrEditKeyValuePair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
+# addOrEditKeyValPair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
 #######################
 addOrEditKeyValPair() {
   local file="${1}"
   local key="${2}"
   local value="${3}"
 
-  if [ "${value}" != "" ]; then
+  if grep -q "^${key}=" "${file}"; then
-    # value has a value, so it is a key-value pair
-    if grep -q "^${key}=" "${file}"; then
       # Key already exists in file, modify the value
       sed -i "/^${key}=/c\\${key}=${value}" "${file}"
-    else
-      # Key does not already exist, add it and it's value
-      echo "${key}=${value}" >> "${file}"
-    fi
   else
-    # value has no value, so it is just a key. Add it if it does not already exist
+    # Key does not already exist, add it and it's value
-    if ! grep -q "^${key}" "${file}"; then
+    echo "${key}=${value}" >> "${file}"
+  fi
+}
+
+#######################
+# Takes two arguments: file, and key.
+# Adds a key to target file
+#
+# Example usage:
+# addKey "/etc/dnsmasq.d/01-pihole.conf" "log-queries"
+#######################
+addKey(){
+  local file="${1}"
+  local key="${2}"
+
+  if ! grep -q "^${key}" "${file}"; then
       # Key does not exist, add it.
       echo "${key}" >> "${file}"
-    fi
   fi
 }
 
 #######################
-# Takes two arguments file, and key.
+# Takes two arguments: file, and key.
-# Deletes a key from target file
+# Deletes a key or key/value pair from target file
 #
 # Example usage:
 # removeKey "/etc/pihole/setupVars.conf" "PIHOLE_DNS_1"

pihole

@@ -260,7 +260,7 @@ Options:
     exit 0
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
-    addOrEditKeyValPair /etc/dnsmasq.d/01-pihole.conf "log-queries"
+    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
     addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false"
     if [[ "${2}" != "noflush" ]]; then
       # Flush logs
@@ -270,7 +270,7 @@ Options:
     local str="Logging has been disabled!"
   elif [[ "${1}" == "on" ]]; then
     # Enable logging
-    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
+    addKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
     addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"

test/test_any_utils.py

@@ -6,8 +6,8 @@ def test_key_val_replacement_works(host):
     addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
     addOrEditKeyValPair "./testoutput" "KEY_ONE" "value3"
     addOrEditKeyValPair "./testoutput" "KEY_FOUR" "value4"
-    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
+    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
-    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
+    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
     ''')
     output = host.run('''
     cat ./testoutput