From 4d31d5ee1148f1de8e8608a7bf0fed255136a6e1 Mon Sep 17 00:00:00 2001
From: Adam Warner <me@adamwarner.co.uk>
Date: Mon, 4 Apr 2022 22:02:26 +0100
Subject: [PATCH] Address review comments

Signed-off-by: Adam Warner <me@adamwarner.co.uk>
---
 advanced/Scripts/utils.sh   | 6 +++---
 advanced/Scripts/webpage.sh | 6 +++---
 pihole                      | 4 ++--
 test/test_any_utils.py      | 4 ++--
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index 0906ce49..9d80e446 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -56,11 +56,11 @@ addOrEditKeyValPair() {
 # Deletes a key from target file
 #
 # Example usage:
-# removeKey "PIHOLE_DNS_1" "/etc/pihole/setupVars.conf"
+# removeKey "/etc/pihole/setupVars.conf" "PIHOLE_DNS_1"
 #######################
 removeKey() {
-  local key="${1}"
-  local file="${2}"
+  local file="${1}"
+  local key="${2}"
   sed -i "/^${key}/d" "${file}"
 }
 
diff --git a/advanced/Scripts/webpage.sh b/advanced/Scripts/webpage.sh
index f63fd0ca..15418ee0 100755
--- a/advanced/Scripts/webpage.sh
+++ b/advanced/Scripts/webpage.sh
@@ -60,7 +60,7 @@ add_setting() {
 }
 
 delete_setting() {
-    removeKey "${1}" "${setupVars}"
+    removeKey "${setupVars}" "${1}"
 }
 
 change_setting() {
@@ -72,7 +72,7 @@ addFTLsetting() {
 }
 
 deleteFTLsetting() {
-    removeKey "${1}" "${FTLconf}"
+    removeKey "${FTLconf}" "${1}"
 }
 
 changeFTLsetting() {
@@ -84,7 +84,7 @@ add_dnsmasq_setting() {
 }
 
 delete_dnsmasq_setting() {
-    removeKey "${1}" "${dnsmasqconfig}"
+    removeKey "${dnsmasqconfig}" "${1}"
 }
 
 SetTemperatureUnit() {
diff --git a/pihole b/pihole
index bdce6663..83d1f45c 100755
--- a/pihole
+++ b/pihole
@@ -260,7 +260,7 @@ Options:
     exit 0
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
-    sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    addOrEditKeyValPair /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
-    sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
     addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"
diff --git a/test/test_any_utils.py b/test/test_any_utils.py
index 1c8f9531..998c1c84 100644
--- a/test/test_any_utils.py
+++ b/test/test_any_utils.py
@@ -17,13 +17,13 @@ def test_key_val_replacement_works(host):
 
 
 def test_key_val_removal_works(host):
-    ''' Confirms addOrEditKeyValPair provides the expected output '''
+    ''' Confirms removeKey provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
     addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
     addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
     addOrEditKeyValPair "./testoutput" "KEY_THREE" "value3"
-    removeKey "KEY_TWO" "./testoutput"
+    removeKey "./testoutput" "KEY_TWO"
     ''')
     output = host.run('''
     cat ./testoutput