Address:

- Review Comments
 - Stickler Complaints

advanced/Scripts/utils.sh

@@ -1,4 +1,6 @@
 #!/usr/bin/env sh
+# shellcheck disable=SC3043 #https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
+
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
 # Network-wide ad blocking via your own hardware.
@@ -16,28 +18,20 @@
 
 #######################
 # Takes either
-#   - Three arguments: key, value, and file.
+#   - Three arguments: file, key, and value.
-#   - Two arguments: key, and file
+#   - 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 "BLOCKING_ENABLED" "true" "/etc/pihole/setupVars.conf"
+# addOrEditKeyValuePair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
 #######################
 addOrEditKeyValPair() {
-  local key="${1}"
+  local file="${1}"
-  local value
+  local key="${2}"
-  local file
+  local value="${3}"
-  
-  # If two arguments have been passed, then the second one is the file - there is no value
-  if [ $# -lt 3 ]; then    
-    file="${2}"
-  else    
-    value="${2}"
-    file="${3}"
-  fi
 
   if [ "${value}" != "" ]; then
     # value has a value, so it is a key pair

advanced/Scripts/webpage.sh

@@ -1,5 +1,7 @@
 #!/usr/bin/env bash
 # shellcheck disable=SC1090
+# shellcheck disable=SC2154
+
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
@@ -54,7 +56,7 @@ Options:
 }
 
 add_setting() {
-    addOrEditKeyValPair "${1}" "${2}" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
 }
 
 delete_setting() {
@@ -62,11 +64,11 @@ delete_setting() {
 }
 
 change_setting() {
-    addOrEditKeyValPair "${1}" "${2}" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
 }
 
 addFTLsetting() {
-    addOrEditKeyValPair "${1}" "${2}" "${FTLconf}"
+    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
 }
 
 deleteFTLsetting() {
@@ -74,11 +76,11 @@ deleteFTLsetting() {
 }
 
 changeFTLsetting() {
-    addOrEditKeyValPair "${1}" "${2}" "${FTLconf}"
+    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
 }
 
 add_dnsmasq_setting() {
-    addOrEditKeyValPair "${1}" "${2}" "${dnsmasqconfig}"
+    addOrEditKeyValPair "${dnsmasqconfig}" "${1}" "${2}"
 }
 
 delete_dnsmasq_setting() {

pihole

@@ -226,7 +226,7 @@ Time:
       fi
 
       local str="Pi-hole Disabled"
-      addOrEditKeyValPair "BLOCKING_ENABLED" "false" "${setupVars}"
+      addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "false"
     fi
   else
     # Enable Pi-hole
@@ -238,7 +238,7 @@ Time:
     echo -e "  ${INFO} Enabling blocking"
     local str="Pi-hole Enabled"
 
-    addOrEditKeyValPair "BLOCKING_ENABLED" "true" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "true"
   fi
 
   restartDNS reload-lists
@@ -261,7 +261,7 @@ Options:
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
     sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf
-    addOrEditKeyValPair "QUERY_LOGGING" "false" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false"
     if [[ "${2}" != "noflush" ]]; then
       # Flush logs
       "${PI_HOLE_BIN_DIR}"/pihole -f
@@ -271,7 +271,7 @@ Options:
   elif [[ "${1}" == "on" ]]; then
     # Enable logging
     sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf
-    addOrEditKeyValPair "QUERY_LOGGING" "true" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"
   else

test/test_any_utils.py

@@ -2,12 +2,12 @@ def test_key_val_replacement_works(host):
     ''' Confirms addOrEditKeyValPair provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
-    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
-    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
-    addOrEditKeyValPair "KEY_ONE" "value3" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value3"
-    addOrEditKeyValPair "KEY_FOUR" "value4" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_FOUR" "value4"
-    addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
-    addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
     ''')
     output = host.run('''
     cat ./testoutput
@@ -15,13 +15,14 @@ def test_key_val_replacement_works(host):
     expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\nKEY_FIVE_NO_VALUE\n'
     assert expected_stdout == output.stdout
 
+
 def test_key_val_removal_works(host):
     ''' Confirms addOrEditKeyValPair provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
-    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
-    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
-    addOrEditKeyValPair "KEY_THREE" "value3" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_THREE" "value3"
     removeKey "KEY_TWO" "./testoutput"
     ''')
     output = host.run('''