From fb3b3e1cf582c49665b67363348d7465c9e84b9a Mon Sep 17 00:00:00 2001
From: rokosun <rokosun@noreply.git.trom.tf>
Date: Sat, 4 Feb 2023 08:33:57 +0100
Subject: [PATCH] Clean code and useful comments

Changed a lot of things but mostly to make the code easy to read and understand.
---
 bin/periodic/fix-zombie-appicons | 149 +++++++++++++++++++------------
 1 file changed, 93 insertions(+), 56 deletions(-)

diff --git a/bin/periodic/fix-zombie-appicons b/bin/periodic/fix-zombie-appicons
index f0bb5b6..1a8c793 100755
--- a/bin/periodic/fix-zombie-appicons
+++ b/bin/periodic/fix-zombie-appicons
@@ -1,77 +1,114 @@
 #!/bin/bash
+
+# Exit out if the same script is already running in the background
 pidof -sq -o %PPID -x "$(basename "$0")" && exit
-datastore="$HOME"/.local/share/fix-zombie-appicons
-bakdir="$datastore"/backup
-data="$datastore"/tweaked-desktop-files
+
+# Setting read-only variables
+declare -r \
+	datadir="$HOME"/.local/share/fix-zombie-appicons \
+	backup_dir="$datadir"/backup \
+	datafile="$datadir"/tweaked-desktop-files
 
 # Directories where desktop files are stored
-user="$HOME"/.local/share/applications
-pacman_global=/usr/share/applications
-pacman_local=/usr/local/share/applications
-flatpak_global=/var/lib/flatpak/exports/share/applications
-flatpak_local="$HOME"/.local/share/flatpak/exports/share/applications
-snap=/var/lib/snapd/desktop/applications
+declare -r home_local_applications_dir="$HOME/.local/share/applications" \
+	home_local_flatpak_applications_dir="$HOME/.local/share/flatpak/exports/share/applications"
 
-mkdir -p "$bakdir" "$user" "$flatpak_local" ||
-	{ echo "failed to make directoris $bakdir, $user & $flatpak_local"; exit 1; }
+# All the other directories except home_local_applications_dir where desktop files are stored
+declare -r other_applications_dirs=(
+	'/usr/share/applications'
+	'/usr/local/share/applications'
+	'/var/lib/flatpak/exports/share/applications'
+	"$home_local_flatpak_applications_dir"
+	'/var/lib/snapd/desktop/applications'
+)
 
-detectfiles() {
-	for file in "$user"/*.desktop; do
-		name=$(basename "$file") || continue
-		[ -d "$file" ] || grep -xq "$name" "$data" && continue
-		[ -f "$pacman_global/$name" ] ||
-			[ -f "$pacman_local/$name" ] ||
-			[ -f "$flatpak_global/$name" ] ||
-			[ -f "$flatpak_local/$name" ] ||
-			[ -f "$snap/$name" ] &&
-			echo "$name" >> "$data"
+# Create tempfile which is used by the fix_desktop_files function
+tempfile="$(mktemp)"
+trap 'rm $tempfile' EXIT # Delete tempfile when the script exits
+declare -r tempfile
+
+# This function will check for new desktop files in home_local_applications_dir and
+# record them in datafile if the same file is found in any of the other_applications_dirs.
+detect_new_desktop_files() {
+	for file in "$home_local_applications_dir"/*.desktop; do
+		# This is added as a failsafe to ensure its a file and not a directory
+		[ -d "$file" ] && continue
+		filename=$(basename "$file")
+		# Continue looping if the filename is already recorded in the datafile
+		grep -xq "$filename" "$datafile" && continue
+		# If the same file is present in any of the other_applications_dirs then add its filename to datafile
+		for dir in "${other_applications_dirs[@]}"; do
+			[ -f "$dir/$filename" ] && echo "$filename" >> "$datafile" && break
+		done
 	done
 }
 
-fixfiles() {
-	lastmod=$(stat -c "%Y" "$data")
-	tmp="$(mktemp)"
-	trap 'rm $tmp' EXIT
-	[ -f "$data" ] && cp "$data" "$tmp" && copied='true'
-	lineno=0
-	[ "$copied" = 'true' ] && while IFS= read -r name; do
-		((lineno++))
-		file="$user/$name"
-		[ -f "$file" ] || { sed -i "${lineno}d" "$tmp" && ((lineno--)) ; continue; }
-		[ -f "$pacman_global/$name" ] ||
-			[ -f "$pacman_local/$name" ] ||
-			[ -f "$flatpak_global/$name" ] ||
-			[ -f "$flatpak_local/$name" ] ||
-			[ -f "$snap/$name" ] ||
-			mv "$file" "$bakdir/$name.bak"
-	done < "$data"
+# The following function will:
+#    1. Go through all the files recorded in datafile and move them to backup_dir if
+#       the same file is not found in other_applications_dirs anymore.
+#    2. Go through all the bakked up files in backup_dir and move them back to
+#       its original_file_path if the same file is present in any of
+#       the other_applications_dirs. This will not overwrite if a new
+#       file already exists at original_file_path.
 
-	[ "$copied" = 'true' ] && [ "$(stat -c '%Y' "$data")" = "$lastmod" ] && sort -u "$tmp" > "$data"
+fix_desktop_files() {
+	# This is added as a failsafe to avoid overwriting datafile in case the file got edited during this period
+	local -r last_modified_time=$(stat -c "%Y" "$datafile")
+	# Copy datafile to tempfile because its not possible to directly edit the file inside a while-read loop
+	[ -f "$datafile" ] && cp "$datafile" "$tempfile" && local -r datafile_copied='true'
+	local -i line_number=0
 
-	for bakfile in "$bakdir"/*.desktop.bak; do
-		name=$(basename "${bakfile%.bak}") || continue
-		origfile="$user/$name"
-		[ -f "$origfile" ] && continue
-		[ -f "$pacman_global/$name" ] ||
-			[ -f "$pacman_local/$name" ] ||
-			[ -f "$flatpak_global/$name" ] ||
-			[ -f "$flatpak_local/$name" ] ||
-			[ -f "$snap/$name" ] &&
-			mv "$bakfile" "$origfile"
+	# Loop through each line in datafile
+	[ "$datafile_copied" = 'true' ] && while IFS='' read -r filename; do
+		line_number+=1
+		file="$home_local_applications_dir/$filename"
+		# If the file don't exist anymore then remove the line from datafile (as tempfile) and continue looping
+		[ -f "$file" ] || { sed -i "${line_number}d" "$tempfile" && line_number+=-1 ; continue; }
+
+		# If the same file is not present in any of the other_applications_dirs then move the file to backup_dir
+		unset file_exists
+		for dir in "${other_applications_dirs[@]}"; do
+			[ -f "$dir/$filename" ] && file_exists='true' && break
+		done
+		[ "$file_exists" != 'true' ] && mv "$file" "$backup_dir/$filename.bak"
+	done < "$datafile"
+
+	# Write the contents from tempfile back to datafile after sorting and removing duplicate lines
+	[ "$datafile_copied" = 'true' ] && [ "$(stat -c '%Y' "$datafile")" = "$last_modified_time" ] && sort -u "$tempfile" > "$datafile"
+
+	for bakfile in "$backup_dir"/*.desktop.bak; do
+		filename=$(basename "${bakfile%.bak}")
+		original_file_path="$home_local_applications_dir/$filename"
+
+		# Continue looping if a new file is already present at the original_file_path,
+		# this will prevent the backup from overwriting the new file
+		[ -f "$original_file_path" ] && continue
+
+		# If the same bakked up file is present in any of the other_applications_dirs then move it back to its original_file_path
+		for dir in "${other_applications_dirs[@]}"; do
+			[ -f "$dir/$filename" ] && mv "$bakfile" "$original_file_path" && break
+		done
 	done
 }
 
-for dir in "$user" "$pacman_global" "$pacman_local" "$flatpak_global" "$flatpak_local" "$snap"; do
+# Create some directories in the user's home directory if they don't already exist
+mkdir -p "$backup_dir" "$home_local_applications_dir" "$home_local_flatpak_applications_dir" ||
+	{ echo "failed to make directories $backup_dir, $home_local_applications_dir & $home_local_flatpak_applications_dir"; exit 1; }
+
+# Check which directories currently exist on the user's system so that inotifywait can monitor them for changes
+for dir in "$home_local_applications_dir" "${other_applications_dirs[@]}"; do
 	[ -d "$dir" ] && existing_dirs+=("$dir")
 done
 
-detectfiles
-fixfiles
+# Both functions will be run once when the script first starts
+detect_new_desktop_files
+fix_desktop_files
 
-while read -r line; do
-	if [ "$line" = "$user/" ]; then
-		detectfiles
+# Inotifywait monitors existing_dirs and the functions are run each time when there is a change in relevant directories
+while IFS='' read -r line; do
+	if [ "$line" = "$home_local_applications_dir/" ]; then
+		detect_new_desktop_files
 	else
-		fixfiles
+		fix_desktop_files
 	fi
 done < <(inotifywait -qm --format '%w' --include '\.desktop$' -e 'move,move_self,create,delete,delete_self,unmount' "${existing_dirs[@]}")