From 551043ceaf4fc7620caf1308569db4c0d14209bc Mon Sep 17 00:00:00 2001
From: Bob Swift <bswift@rsds.ca>
Date: Fri, 18 Nov 2022 15:57:23 -0700
Subject: [PATCH 1/6] Process command line executable commands through a queue
 object.

- Implement a queue object to hold the executable command queue
- Implement a command processor thread to parse the command queue
- Implement command file tracking to avoid circular references
- Do not queue additional commands after a `QUIT` has been queued

Still required:

- Block new command execution until current command is complete
- Test for completion must include all sub threads created
---
 picard/tagger.py                         | 171 +++++++++++++++++------
 test/data/test-command-file-1.txt        |  11 ++
 test/test_parsing_files_with_commands.py |  40 ++----
 3 files changed, 158 insertions(+), 64 deletions(-)
 create mode 100644 test/data/test-command-file-1.txt

diff --git a/picard/tagger.py b/picard/tagger.py
index 932d54854..c65bd462e 100644
--- a/picard/tagger.py
+++ b/picard/tagger.py
@@ -50,6 +50,7 @@ from hashlib import md5
 import logging
 import os
 import platform
+import queue
 import re
 import shlex
 import shutil
@@ -178,6 +179,25 @@ def plugin_dirs():
     yield USER_PLUGIN_DIR
 
 
+class CommandFiles:
+    _command_files = set()
+
+    # Maintain a flag to indicate whether a 'QUIT' command has been queued
+    quit_flag = False
+
+    @classmethod
+    def contains(cls, filepath):
+        return filepath in cls._command_files
+
+    @classmethod
+    def add(cls, filepath):
+        cls._command_files.add(filepath)
+
+    @classmethod
+    def remove(cls, filepath):
+        cls._command_files.discard(filepath)
+
+
 class ParseItemsToLoad:
 
     WINDOWS_DRIVE_TEST = re.compile(r"^[a-z]\:", re.IGNORECASE)
@@ -190,8 +210,10 @@ class ParseItemsToLoad:
 
         for item in items:
             parsed = urlparse(item)
+            log.debug(f"Parsed: {repr(parsed)}")
             if not parsed.scheme:
                 self.files.add(item)
+            # TODO: Remove the "command" scheme if it is no longer required.
             elif parsed.scheme == "command":
                 for x in item[10:].split(';'):
                     self.commands.append(x.strip())
@@ -327,6 +349,9 @@ class Tagger(QtWidgets.QApplication):
     _debug = False
     _no_restore = False
 
+    command_queue = queue.Queue()
+    command_running = False
+
     def __init__(self, picard_args, localedir, autoupdate, pipe_handler=None):
 
         super().__init__(sys.argv)
@@ -460,6 +485,8 @@ class Tagger(QtWidgets.QApplication):
         if self.autoupdate_enabled:
             self.updatecheckmanager = UpdateCheckManager(parent=self.window)
 
+        thread.run_task(self.run_commands, self._run_commands_finished)
+
     @property
     def is_wayland(self):
         return self.platformName() == 'wayland'
@@ -470,7 +497,7 @@ class Tagger(QtWidgets.QApplication):
             messages = [x for x in self.pipe_handler.read_from_pipe() if x not in IGNORED]
             if messages:
                 log.debug("pipe messages: %r", messages)
-                thread.to_main(self.load_to_picard, messages)
+                self.load_to_picard(messages)
 
     def _pipe_server_finished(self, result=None, error=None):
         if error:
@@ -478,23 +505,91 @@ class Tagger(QtWidgets.QApplication):
         else:
             log.debug('pipe server stopped')
 
+    def clear_command_running(self, *args, **kwargs):
+        self.command_running = False
+
+    def run_commands(self):
+        # Provide a set of commands that should not automatically release the 'self.command_running'
+        # flag when the command execution completes.  For example, loading a release creates a number
+        # of sub-tasks on separate threads and requires additional process checks to ensure that all
+        # sub-tasks are complete before the 'self.command_running' flag should be released.
+
+        # no_auto_complete = {'LOAD', 'CLUSTER'}
+
+        while not self.stopping:
+            if not self.command_queue.empty() and not self.command_running:
+                (cmd, arg) = self.command_queue.get()
+                cmd = cmd.upper()
+                arg = arg.strip()
+                if cmd in self.commands:
+                    log.debug("Executing command: %s %r", cmd, arg)
+                    self.command_running = True
+
+                    # FIXME: Find a way to execute each command using a block to ensure completion
+                    # before executing the next command.  Perhaps track completion of all threads
+                    # created while "command_running" is True and only clear_command_running()
+                    # when all those threads have completed?  That may also remove the need for
+                    # the special "no_auto_complete" processing.
+
+                    thread.to_main(self.commands[cmd], arg)
+                    # if cmd in no_auto_complete:
+                    #     break
+                    self.clear_command_running()
+                else:
+                    log.error("Unknown command: %r", cmd)
+                self.command_queue.task_done()
+
+    def _run_commands_finished(self, result=None, error=None):
+        if error:
+            log.error('command executor failed: %r', error)
+        else:
+            log.debug('command executor stopped')
+
     def load_to_picard(self, items):
-        parsed_items = ParseItemsToLoad(items)
-        log.debug(str(parsed_items))
+        commands = []
+        for item in items:
+            parts = str(item).split(maxsplit=1)
+            commands.append((parts[0], parts[1:] or ['']))
+        self.parse_commands_to_queue(commands)
 
-        if parsed_items.files:
-            self.add_paths(parsed_items.files)
+    def parse_commands_to_queue(self, commands):
+        # Don't queue any more commands after a QUIT command.
+        if CommandFiles.quit_flag:
+            return
+        for (cmd, cmdargs) in commands:
+            cmd = cmd.upper()
+            if cmd not in REMOTE_COMMANDS:
+                log.error("Unknown command: %s", cmd)
+                continue
+            for cmd_arg in cmdargs or ['']:
+                if cmd == 'FROM_FILE':
+                    self.handle_command_from_file(cmd_arg)
+                else:
+                    log.debug(f"Queueing command: {cmd} {repr(cmd_arg)}")
+                    self.command_queue.put([cmd, cmd_arg])
+                    # Set flag so as to not queue any more commands after a QUIT command.
+                    if cmd == 'QUIT':
+                        CommandFiles.quit_flag = True
+                        return
 
-        if parsed_items.urls or parsed_items.mbids:
-            file_lookup = self.get_file_lookup()
-            for item in parsed_items.mbids | parsed_items.urls:
-                thread.to_main(file_lookup.mbid_lookup, item, None, None, False)
-
-        for command in parsed_items.commands:
-            self.handle_command(command)
-
-        if parsed_items.non_executable_items():
-            self.bring_tagger_front()
+    @staticmethod
+    def _read_commands_from_file(filepath):
+        commands = []
+        try:
+            lines = open(filepath).readlines()
+        except Exception as e:
+            log.error("Error reading command file '%s': %s" % (filepath, e))
+            return commands
+        for line in lines:
+            line = line.strip()
+            if not line or line.startswith('#'):
+                continue
+            elements = shlex.split(line)
+            if not elements:
+                continue
+            command_args = elements[1:] or ['']
+            commands.append((elements[0], command_args))
+        return commands
 
     def iter_album_files(self):
         for album in self.albums.values():
@@ -529,35 +624,33 @@ class Tagger(QtWidgets.QApplication):
         for album_name in self.albums:
             self.analyze(self.albums[album_name].iterfiles())
 
-    @staticmethod
-    def _read_lines_from_file(filepath):
-        try:
-            yield from (line.strip() for line in open(filepath).readlines())
-        except Exception as e:
-            log.error("Error reading command file '%s': %s" % (filepath, e))
-
-    @staticmethod
-    def _parse_commands_from_lines(lines):
-        for line in lines:
-            if not line or line.startswith('#'):
-                continue
-            elements = shlex.split(line)
-            if not elements:
-                continue
-            command_args = elements[1:] or ['']
-            for element in command_args:
-                yield f"command://{elements[0]} {element}"
-
     def handle_command_from_file(self, argstring):
-        for command in self._parse_commands_from_lines(self._read_lines_from_file(argstring)):
-            self.load_to_picard((command,))
+        log.debug("Reading commands from: %r", argstring)
+        if not os.path.exists(argstring):
+            log.error("Missing command file: '%s'", argstring)
+            return
+        filepath = os.path.abspath(argstring)
+        if CommandFiles.contains(filepath):
+            log.warning("Circular command file reference ignored: '%s'", argstring)
+            return
+        CommandFiles.add(filepath)
+        self.parse_commands_to_queue(self._read_commands_from_file(filepath))
+        CommandFiles.remove(filepath)
 
     def handle_command_load(self, argstring):
         if argstring.startswith("command://"):
             log.error("Cannot LOAD a command: %s", argstring)
             return
+        parsed_items = ParseItemsToLoad([argstring])
+        log.debug(str(parsed_items))
 
-        self.load_to_picard((argstring,))
+        if parsed_items.files:
+            self.add_paths(parsed_items.files)
+
+        if parsed_items.urls or parsed_items.mbids:
+            file_lookup = self.get_file_lookup()
+            for item in parsed_items.mbids | parsed_items.urls:
+                file_lookup.mbid_lookup(item)
 
     def handle_command_lookup(self, argstring):
         if argstring:
@@ -1428,14 +1521,14 @@ If a new instance will not be spawned files/directories will be passed to the ex
     for x in args.FILE_OR_URL:
         if not urlparse(x).netloc:
             x = os.path.abspath(x)
-        args.processable.append(x)
+        args.processable.append(f"LOAD {x}")
 
     if args.exec:
         for e in args.exec:
             args.remote_commands_help = args.remote_commands_help or "HELP" in {x.upper().strip() for x in e}
             remote_command_args = e[1:] or ['']
             for arg in remote_command_args:
-                args.processable.append(f"command://{e[0]} {arg}")
+                args.processable.append(f"{e[0]} {arg}")
 
     return args
 
diff --git a/test/data/test-command-file-1.txt b/test/data/test-command-file-1.txt
new file mode 100644
index 000000000..9bcf05818
--- /dev/null
+++ b/test/data/test-command-file-1.txt
@@ -0,0 +1,11 @@
+# should be split into 2 commands
+LOAD file1.mp3 file2.mp3
+# should be added as one
+LOAD file3.mp3
+FROM_FILE command_file.txt
+CLUSTER unclustered
+  FINGERPRINT  
+# should be ignored
+ 
+
+#commented command
diff --git a/test/test_parsing_files_with_commands.py b/test/test_parsing_files_with_commands.py
index c18604134..fe09feed5 100644
--- a/test/test_parsing_files_with_commands.py
+++ b/test/test_parsing_files_with_commands.py
@@ -5,43 +5,33 @@ from picard.tagger import Tagger
 
 class TestParsingFilesWithCommands(PicardTestCase):
 
-    MOCK_FILE_CONTENTS = (
-        # should be split into 2 commands
-        "LOAD file1.mp3 file2.mp3",
-        # should be added as one
-        "FROM_FILE file0.mp3",
-        "CLUSTER",
-        "  FINGERPRINT  "
-        # should be ignored
-        "",
-        " ",
-        "\n",
-        "#commented command",
-    )
+    TEST_FILE = 'test/data/test-command-file-1.txt'
 
     def setUp(self):
         super().setUp()
-        self.result = tuple(x for x in Tagger._parse_commands_from_lines(self.MOCK_FILE_CONTENTS))
+        self.result = []
+        for (cmd, cmdargs) in Tagger._read_commands_from_file(self.TEST_FILE):
+            for cmd_arg in cmdargs or ['']:
+                self.result.append(f"{cmd} {cmd_arg}")
 
     def test_no_argument_command(self):
-        self.assertIn("command://CLUSTER ", self.result)
+        self.assertIn("CLUSTER unclustered", self.result)
 
     def test_no_argument_command_stripped_correctly(self):
-        self.assertIn("command://FINGERPRINT ", self.result)
+        self.assertIn("FINGERPRINT ", self.result)
 
     def test_single_argument_command(self):
-        self.assertIn("command://FROM_FILE file0.mp3", self.result)
+        self.assertIn("FROM_FILE command_file.txt", self.result)
+        self.assertIn("LOAD file3.mp3", self.result)
 
     def test_multiple_arguments_command(self):
-        self.assertIn("command://LOAD file1.mp3", self.result)
-        self.assertIn("command://LOAD file2.mp3", self.result)
+        self.assertIn("LOAD file1.mp3", self.result)
+        self.assertIn("LOAD file2.mp3", self.result)
 
     def test_empty_lines(self):
-        self.assertNotIn("command:// ", self.result)
-        self.assertNotIn("command://", self.result)
-        # 1 FROM_FILE
-        # 2 LOADs
-        self.assertEqual(len(self.result), 5)
+        self.assertNotIn(" ", self.result)
+        self.assertNotIn("", self.result)
+        self.assertEqual(len(self.result), 6)
 
     def test_commented_lines(self):
-        self.assertNotIn("command://#commented command", self.result)
+        self.assertNotIn("#commented command", self.result)

From 1847b4d06804d77e463e0af6ada10a51dcab4bb3 Mon Sep 17 00:00:00 2001
From: Bob Swift <bswift@rsds.ca>
Date: Sat, 19 Nov 2022 11:40:59 -0700
Subject: [PATCH 2/6] Refactor to create new `RemoteCommands` class

---
 picard/tagger.py                         | 189 ++---------------
 picard/util/remotecommands.py            | 255 +++++++++++++++++++++++
 test/data/test-command-file-1.txt        |  10 +-
 test/data/test-command-file-2.txt        |  10 +
 test/test_parsing_files_with_commands.py |  45 +++-
 5 files changed, 324 insertions(+), 185 deletions(-)
 create mode 100644 picard/util/remotecommands.py
 create mode 100644 test/data/test-command-file-2.txt

diff --git a/picard/tagger.py b/picard/tagger.py
index c65bd462e..828668886 100644
--- a/picard/tagger.py
+++ b/picard/tagger.py
@@ -50,9 +50,7 @@ from hashlib import md5
 import logging
 import os
 import platform
-import queue
 import re
-import shlex
 import shutil
 import signal
 import sys
@@ -137,6 +135,10 @@ from picard.util import (
 )
 from picard.util.cdrom import get_cdrom_drives
 from picard.util.checkupdate import UpdateCheckManager
+from picard.util.remotecommands import (
+    REMOTE_COMMANDS,
+    RemoteCommands,
+)
 from picard.webservice import WebService
 from picard.webservice.api_helpers import (
     AcoustIdAPIHelper,
@@ -179,25 +181,6 @@ def plugin_dirs():
     yield USER_PLUGIN_DIR
 
 
-class CommandFiles:
-    _command_files = set()
-
-    # Maintain a flag to indicate whether a 'QUIT' command has been queued
-    quit_flag = False
-
-    @classmethod
-    def contains(cls, filepath):
-        return filepath in cls._command_files
-
-    @classmethod
-    def add(cls, filepath):
-        cls._command_files.add(filepath)
-
-    @classmethod
-    def remove(cls, filepath):
-        cls._command_files.discard(filepath)
-
-
 class ParseItemsToLoad:
 
     WINDOWS_DRIVE_TEST = re.compile(r"^[a-z]\:", re.IGNORECASE)
@@ -241,100 +224,6 @@ class ParseItemsToLoad:
         return f"files: {repr(self.files)}  mbids: f{repr(self.mbids)}  urls: {repr(self.urls)}  commands: {repr(self.commands)}"
 
 
-class RemoteCommand:
-    def __init__(self, method_name, help_text=None, help_args=None):
-        self.method_name = method_name
-        self.help_text = help_text or ""
-        self.help_args = help_args or ""
-
-
-REMOTE_COMMANDS = {
-    "CLEAR_LOGS": RemoteCommand(
-        "handle_command_clear_logs",
-        help_text="Clear the Picard logs",
-    ),
-    "CLUSTER": RemoteCommand(
-        "handle_command_cluster",
-        help_text="Cluster all files in the cluster pane.",
-    ),
-    "FINGERPRINT": RemoteCommand(
-        "handle_command_fingerprint",
-        help_text="Calculate acoustic fingerprints for all (matched) files in the album pane.",
-    ),
-    "FROM_FILE": RemoteCommand(
-        "handle_command_from_file",
-        help_text="Load command pipeline from a file.",
-        help_args="[Absolute path to a file containing command pipeline]",
-    ),
-    "LOAD": RemoteCommand(
-        "handle_command_load",
-        help_text="Load 1 or more files/MBIDs/URLs to Picard.",
-        help_args="[supported MBID/URL or absolute path to a file]",
-    ),
-    "LOOKUP": RemoteCommand(
-        "handle_command_lookup",
-        help_text="Lookup files in the clustering pane. Defaults to all files.",
-        help_args="[clustered|unclustered|all]"
-    ),
-    "LOOKUP_CD": RemoteCommand(
-        "handle_command_lookup_cd",
-        help_text="Read CD from the selected drive and lookup on MusicBrainz. "
-        "Without argument, it defaults to the first (alphabetically) available disc drive",
-        help_args="[device/log file]",
-    ),
-    "QUIT": RemoteCommand(
-        "handle_command_quit",
-        help_text="Exit the running instance of Picard.",
-    ),
-    "REMOVE": RemoteCommand(
-        "handle_command_remove",
-        help_text="Remove the file from Picard. Do nothing if no arguments provided.",
-        help_args="[absolute path to 1 or more files]",
-    ),
-    "REMOVE_ALL": RemoteCommand(
-        "handle_command_remove_all",
-        help_text="Remove all files from Picard.",
-    ),
-    "REMOVE_EMPTY": RemoteCommand(
-        "handle_command_remove_empty",
-        help_text="Remove all empty clusters and albums.",
-    ),
-    "REMOVE_SAVED": RemoteCommand(
-        "handle_command_remove_saved",
-        help_text="Remove all saved releases from the album pane.",
-    ),
-    "REMOVE_UNCLUSTERED": RemoteCommand(
-        "handle_command_remove_unclustered",
-        help_text="Remove all unclustered files from the cluster pane.",
-    ),
-    "SAVE_MATCHED": RemoteCommand(
-        "handle_command_save_matched",
-        help_text="Save all matched releases from the album pane."
-    ),
-    "SAVE_MODIFIED": RemoteCommand(
-        "handle_command_save_modified",
-        help_text="Save all modified files from the album pane.",
-    ),
-    "SCAN": RemoteCommand(
-        "handle_command_scan",
-        help_text="Scan all files in the cluster pane.",
-    ),
-    "SHOW": RemoteCommand(
-        "handle_command_show",
-        help_text="Make the running instance the currently active window.",
-    ),
-    "SUBMIT_FINGERPRINTS": RemoteCommand(
-        "handle_command_submit_fingerprints",
-        help_text="Submit outstanding acoustic fingerprints for all (matched) files in the album pane.",
-    ),
-    "WRITE_LOGS": RemoteCommand(
-        "handle_command_write_logs",
-        help_text="Write Picard logs to a given path.",
-        help_args="[absolute path to 1 file]",
-    ),
-}
-
-
 class Tagger(QtWidgets.QApplication):
 
     tagger_stats_changed = QtCore.pyqtSignal()
@@ -349,9 +238,6 @@ class Tagger(QtWidgets.QApplication):
     _debug = False
     _no_restore = False
 
-    command_queue = queue.Queue()
-    command_running = False
-
     def __init__(self, picard_args, localedir, autoupdate, pipe_handler=None):
 
         super().__init__(sys.argv)
@@ -506,7 +392,7 @@ class Tagger(QtWidgets.QApplication):
             log.debug('pipe server stopped')
 
     def clear_command_running(self, *args, **kwargs):
-        self.command_running = False
+        RemoteCommands.set_running(False)
 
     def run_commands(self):
         # Provide a set of commands that should not automatically release the 'self.command_running'
@@ -517,13 +403,13 @@ class Tagger(QtWidgets.QApplication):
         # no_auto_complete = {'LOAD', 'CLUSTER'}
 
         while not self.stopping:
-            if not self.command_queue.empty() and not self.command_running:
-                (cmd, arg) = self.command_queue.get()
+            if not RemoteCommands.command_queue.empty() and not RemoteCommands.get_running():
+                (cmd, arg) = RemoteCommands.command_queue.get()
                 cmd = cmd.upper()
                 arg = arg.strip()
                 if cmd in self.commands:
                     log.debug("Executing command: %s %r", cmd, arg)
-                    self.command_running = True
+                    RemoteCommands.set_running(True)
 
                     # FIXME: Find a way to execute each command using a block to ensure completion
                     # before executing the next command.  Perhaps track completion of all threads
@@ -537,7 +423,7 @@ class Tagger(QtWidgets.QApplication):
                     self.clear_command_running()
                 else:
                     log.error("Unknown command: %r", cmd)
-                self.command_queue.task_done()
+                RemoteCommands.command_queue.task_done()
 
     def _run_commands_finished(self, result=None, error=None):
         if error:
@@ -545,51 +431,13 @@ class Tagger(QtWidgets.QApplication):
         else:
             log.debug('command executor stopped')
 
-    def load_to_picard(self, items):
+    @staticmethod
+    def load_to_picard(items):
         commands = []
         for item in items:
             parts = str(item).split(maxsplit=1)
             commands.append((parts[0], parts[1:] or ['']))
-        self.parse_commands_to_queue(commands)
-
-    def parse_commands_to_queue(self, commands):
-        # Don't queue any more commands after a QUIT command.
-        if CommandFiles.quit_flag:
-            return
-        for (cmd, cmdargs) in commands:
-            cmd = cmd.upper()
-            if cmd not in REMOTE_COMMANDS:
-                log.error("Unknown command: %s", cmd)
-                continue
-            for cmd_arg in cmdargs or ['']:
-                if cmd == 'FROM_FILE':
-                    self.handle_command_from_file(cmd_arg)
-                else:
-                    log.debug(f"Queueing command: {cmd} {repr(cmd_arg)}")
-                    self.command_queue.put([cmd, cmd_arg])
-                    # Set flag so as to not queue any more commands after a QUIT command.
-                    if cmd == 'QUIT':
-                        CommandFiles.quit_flag = True
-                        return
-
-    @staticmethod
-    def _read_commands_from_file(filepath):
-        commands = []
-        try:
-            lines = open(filepath).readlines()
-        except Exception as e:
-            log.error("Error reading command file '%s': %s" % (filepath, e))
-            return commands
-        for line in lines:
-            line = line.strip()
-            if not line or line.startswith('#'):
-                continue
-            elements = shlex.split(line)
-            if not elements:
-                continue
-            command_args = elements[1:] or ['']
-            commands.append((elements[0], command_args))
-        return commands
+        RemoteCommands.parse_commands_to_queue(commands)
 
     def iter_album_files(self):
         for album in self.albums.values():
@@ -625,19 +473,10 @@ class Tagger(QtWidgets.QApplication):
             self.analyze(self.albums[album_name].iterfiles())
 
     def handle_command_from_file(self, argstring):
-        log.debug("Reading commands from: %r", argstring)
-        if not os.path.exists(argstring):
-            log.error("Missing command file: '%s'", argstring)
-            return
-        filepath = os.path.abspath(argstring)
-        if CommandFiles.contains(filepath):
-            log.warning("Circular command file reference ignored: '%s'", argstring)
-            return
-        CommandFiles.add(filepath)
-        self.parse_commands_to_queue(self._read_commands_from_file(filepath))
-        CommandFiles.remove(filepath)
+        RemoteCommands.get_commands_from_file(argstring)
 
     def handle_command_load(self, argstring):
+        # TODO: Remove this check if "command://" no longer used.
         if argstring.startswith("command://"):
             log.error("Cannot LOAD a command: %s", argstring)
             return
diff --git a/picard/util/remotecommands.py b/picard/util/remotecommands.py
new file mode 100644
index 000000000..01d44c9e5
--- /dev/null
+++ b/picard/util/remotecommands.py
@@ -0,0 +1,255 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2022 Bob Swift
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+import datetime
+import os
+import queue
+import shlex
+import threading
+import time
+
+from picard import log
+
+
+class RemoteCommand:
+    def __init__(self, method_name, help_text=None, help_args=None):
+        self.method_name = method_name
+        self.help_text = help_text or ""
+        self.help_args = help_args or ""
+
+
+REMOTE_COMMANDS = {
+    "CLEAR_LOGS": RemoteCommand(
+        "handle_command_clear_logs",
+        help_text="Clear the Picard logs",
+    ),
+    "CLUSTER": RemoteCommand(
+        "handle_command_cluster",
+        help_text="Cluster all files in the cluster pane.",
+    ),
+    "FINGERPRINT": RemoteCommand(
+        "handle_command_fingerprint",
+        help_text="Calculate acoustic fingerprints for all (matched) files in the album pane.",
+    ),
+    "FROM_FILE": RemoteCommand(
+        "handle_command_from_file",
+        help_text="Load command pipeline from a file.",
+        help_args="[Absolute path to a file containing command pipeline]",
+    ),
+    "LOAD": RemoteCommand(
+        "handle_command_load",
+        help_text="Load 1 or more files/MBIDs/URLs to Picard.",
+        help_args="[supported MBID/URL or absolute path to a file]",
+    ),
+    "LOOKUP": RemoteCommand(
+        "handle_command_lookup",
+        help_text="Lookup files in the clustering pane. Defaults to all files.",
+        help_args="[clustered|unclustered|all]"
+    ),
+    "LOOKUP_CD": RemoteCommand(
+        "handle_command_lookup_cd",
+        help_text="Read CD from the selected drive and lookup on MusicBrainz. "
+        "Without argument, it defaults to the first (alphabetically) available disc drive",
+        help_args="[device/log file]",
+    ),
+    "QUIT": RemoteCommand(
+        "handle_command_quit",
+        help_text="Exit the running instance of Picard.",
+    ),
+    "REMOVE": RemoteCommand(
+        "handle_command_remove",
+        help_text="Remove the file from Picard. Do nothing if no arguments provided.",
+        help_args="[absolute path to 1 or more files]",
+    ),
+    "REMOVE_ALL": RemoteCommand(
+        "handle_command_remove_all",
+        help_text="Remove all files from Picard.",
+    ),
+    "REMOVE_EMPTY": RemoteCommand(
+        "handle_command_remove_empty",
+        help_text="Remove all empty clusters and albums.",
+    ),
+    "REMOVE_SAVED": RemoteCommand(
+        "handle_command_remove_saved",
+        help_text="Remove all saved releases from the album pane.",
+    ),
+    "REMOVE_UNCLUSTERED": RemoteCommand(
+        "handle_command_remove_unclustered",
+        help_text="Remove all unclustered files from the cluster pane.",
+    ),
+    "SAVE_MATCHED": RemoteCommand(
+        "handle_command_save_matched",
+        help_text="Save all matched releases from the album pane."
+    ),
+    "SAVE_MODIFIED": RemoteCommand(
+        "handle_command_save_modified",
+        help_text="Save all modified files from the album pane.",
+    ),
+    "SCAN": RemoteCommand(
+        "handle_command_scan",
+        help_text="Scan all files in the cluster pane.",
+    ),
+    "SHOW": RemoteCommand(
+        "handle_command_show",
+        help_text="Make the running instance the currently active window.",
+    ),
+    "SUBMIT_FINGERPRINTS": RemoteCommand(
+        "handle_command_submit_fingerprints",
+        help_text="Submit outstanding acoustic fingerprints for all (matched) files in the album pane.",
+    ),
+    "WRITE_LOGS": RemoteCommand(
+        "handle_command_write_logs",
+        help_text="Write Picard logs to a given path.",
+        help_args="[absolute path to 1 file]",
+    ),
+}
+
+
+class RemoteCommands:
+    # Collection of command files currently being parsed
+    _command_files = set()
+
+    # Flag to indicate whether a 'QUIT' command has been queued
+    _has_quit = False
+
+    # Flag to indicate whether a command is currently running
+    _command_running = False
+
+    _lock = threading.Lock()
+    command_queue = queue.Queue()
+    _threads = set()
+
+    @classmethod
+    def cmd_files_contains(cls, filepath):
+        with cls._lock:
+            return filepath in cls._command_files
+
+    @classmethod
+    def cmd_files_add(cls, filepath):
+        with cls._lock:
+            cls._command_files.add(filepath)
+
+    @classmethod
+    def cmd_files_remove(cls, filepath):
+        with cls._lock:
+            cls._command_files.discard(filepath)
+
+    @classmethod
+    def has_quit(cls):
+        with cls._lock:
+            return cls._has_quit
+
+    @classmethod
+    def set_quit(cls, value):
+        with cls._lock:
+            cls._has_quit = value
+
+    @classmethod
+    def thread_add(cls, thread_id):
+        with cls._lock:
+            cls._threads.add(thread_id)
+
+    @classmethod
+    def thread_remove(cls, thread_id):
+        with cls._lock:
+            cls._threads.discard(thread_id)
+
+    @classmethod
+    def processing(cls):
+        with cls._lock:
+            return (len(cls._threads) > 0)
+
+    @classmethod
+    def get_running(cls):
+        with cls._lock:
+            return cls._command_running
+
+    @classmethod
+    def set_running(cls, value):
+        with cls._lock:
+            cls._command_running = value
+
+    @classmethod
+    def clear_command_running(cls, force=False, timeout=None):
+        end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout) if timeout else None
+        while True:
+            if not cls.processing() or force or (timeout and datetime.datetime.now() > end_time):
+                with cls._lock:
+                    cls._command_running = False
+                    cls._threads = set()
+                break
+            time.sleep(.01)
+
+    @classmethod
+    def parse_commands_to_queue(cls, commands):
+        if cls.has_quit():
+            # Don't queue any more commands after a QUIT command.
+            print(f"has_quit = {cls.has_quit()}\n\n")
+            return
+
+        for (cmd, cmdargs) in commands:
+            cmd = cmd.upper()
+            if cmd not in REMOTE_COMMANDS:
+                log.error("Unknown command: %s", cmd)
+                continue
+            for cmd_arg in cmdargs or ['']:
+                if cmd == 'FROM_FILE':
+                    cls.get_commands_from_file(cmd_arg)
+                else:
+                    log.debug(f"Queueing command: {cmd} {repr(cmd_arg)}")
+                    cls.command_queue.put([cmd, cmd_arg])
+
+                    # Set flag so as to not queue any more commands after a QUIT command.
+                    if cmd == 'QUIT':
+                        cls.set_quit(True)
+                        return
+
+    @staticmethod
+    def _read_commands_from_file(filepath):
+        commands = []
+        try:
+            lines = open(filepath).readlines()
+        except Exception as e:
+            log.error("Error reading command file '%s': %s" % (filepath, e))
+            return commands
+        for line in lines:
+            line = line.strip()
+            if not line or line.startswith('#'):
+                continue
+            elements = shlex.split(line)
+            if not elements:
+                continue
+            command_args = elements[1:] or ['']
+            commands.append((elements[0], command_args))
+        return commands
+
+    @classmethod
+    def get_commands_from_file(cls, argstring):
+        log.debug("Reading commands from: %r", argstring)
+        if not os.path.exists(argstring):
+            log.error("Missing command file: '%s'", argstring)
+            return
+        filepath = os.path.abspath(argstring)
+        if cls.cmd_files_contains(filepath):
+            log.warning("Circular command file reference ignored: '%s'", argstring)
+            return
+        cls.cmd_files_add(filepath)
+        cls.parse_commands_to_queue(cls._read_commands_from_file(filepath))
+        cls.cmd_files_remove(filepath)
diff --git a/test/data/test-command-file-1.txt b/test/data/test-command-file-1.txt
index 9bcf05818..d4e63f877 100644
--- a/test/data/test-command-file-1.txt
+++ b/test/data/test-command-file-1.txt
@@ -1,11 +1,15 @@
 # should be split into 2 commands
 LOAD file1.mp3 file2.mp3
+
 # should be added as one
 LOAD file3.mp3
-FROM_FILE command_file.txt
-CLUSTER unclustered
-  FINGERPRINT  
+
+# should be ignored because circular reference
+FROM_FILE test/data/test-command-file-1.txt
+
 # should be ignored
  
 
 #commented command
+
+FROM_FILE test/data/test-command-file-2.txt
diff --git a/test/data/test-command-file-2.txt b/test/data/test-command-file-2.txt
new file mode 100644
index 000000000..1695edfd5
--- /dev/null
+++ b/test/data/test-command-file-2.txt
@@ -0,0 +1,10 @@
+# should be ignored because missing
+FROM_FILE command_file.txt
+
+CLUSTER
+  FINGERPRINT  
+LOOKUP unclustered
+QUIT
+
+# should be ignored because after QUIT command
+LOOKUP clustered
diff --git a/test/test_parsing_files_with_commands.py b/test/test_parsing_files_with_commands.py
index fe09feed5..8b94bbcb7 100644
--- a/test/test_parsing_files_with_commands.py
+++ b/test/test_parsing_files_with_commands.py
@@ -1,6 +1,27 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2022 skelly37
+# Copyright (C) 2022 Bob Swift
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
 from test.picardtestcase import PicardTestCase
 
-from picard.tagger import Tagger
+from picard.util.remotecommands import RemoteCommands
 
 
 class TestParsingFilesWithCommands(PicardTestCase):
@@ -10,28 +31,38 @@ class TestParsingFilesWithCommands(PicardTestCase):
     def setUp(self):
         super().setUp()
         self.result = []
-        for (cmd, cmdargs) in Tagger._read_commands_from_file(self.TEST_FILE):
-            for cmd_arg in cmdargs or ['']:
-                self.result.append(f"{cmd} {cmd_arg}")
+        RemoteCommands.set_quit(False)
+        RemoteCommands.get_commands_from_file(self.TEST_FILE)
+        while not RemoteCommands.command_queue.empty():
+            (cmd, arg) = RemoteCommands.command_queue.get()
+            self.result.append(f"{cmd} {arg}")
+            RemoteCommands.command_queue.task_done()
 
     def test_no_argument_command(self):
-        self.assertIn("CLUSTER unclustered", self.result)
+        self.assertIn("CLUSTER ", self.result)
 
     def test_no_argument_command_stripped_correctly(self):
         self.assertIn("FINGERPRINT ", self.result)
 
     def test_single_argument_command(self):
-        self.assertIn("FROM_FILE command_file.txt", self.result)
         self.assertIn("LOAD file3.mp3", self.result)
 
     def test_multiple_arguments_command(self):
         self.assertIn("LOAD file1.mp3", self.result)
         self.assertIn("LOAD file2.mp3", self.result)
 
+    def test_from_file_command_parsed(self):
+        self.assertNotIn("FROM_FILE command_file.txt", self.result)
+        self.assertNotIn("FROM_FILE test/data/test-command-file-1.txt", self.result)
+        self.assertNotIn("FROM_FILE test/data/test-command-file-2.txt", self.result)
+
+    def test_noting_added_after_quit(self):
+        self.assertNotIn("LOOKUP clustered", self.result)
+
     def test_empty_lines(self):
         self.assertNotIn(" ", self.result)
         self.assertNotIn("", self.result)
-        self.assertEqual(len(self.result), 6)
+        self.assertEqual(len(self.result), 7)
 
     def test_commented_lines(self):
         self.assertNotIn("#commented command", self.result)

From 2e2a88b515b4b6838ad09e616392fdd33e5b6290 Mon Sep 17 00:00:00 2001
From: Bob Swift <bswift@rsds.ca>
Date: Sat, 19 Nov 2022 13:55:47 -0700
Subject: [PATCH 3/6] Block remote command execution to allow sequential
 processing.

- Add blocking version of `thread.to_main()`
- Track thread execution in `Runnable` class
- Add `wait_for_completion()` with optional timeout to `RemoteCommands`
class
- Add sleep to loops to allow system processing
---
 picard/tagger.py              | 26 ++++++++------------------
 picard/util/remotecommands.py |  9 ++++++++-
 picard/util/thread.py         | 27 +++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/picard/tagger.py b/picard/tagger.py
index 828668886..9f7b46bdb 100644
--- a/picard/tagger.py
+++ b/picard/tagger.py
@@ -55,6 +55,7 @@ import shutil
 import signal
 import sys
 from textwrap import fill
+import time
 from urllib.parse import urlparse
 from uuid import uuid4
 
@@ -395,35 +396,24 @@ class Tagger(QtWidgets.QApplication):
         RemoteCommands.set_running(False)
 
     def run_commands(self):
-        # Provide a set of commands that should not automatically release the 'self.command_running'
-        # flag when the command execution completes.  For example, loading a release creates a number
-        # of sub-tasks on separate threads and requires additional process checks to ensure that all
-        # sub-tasks are complete before the 'self.command_running' flag should be released.
-
-        # no_auto_complete = {'LOAD', 'CLUSTER'}
-
         while not self.stopping:
             if not RemoteCommands.command_queue.empty() and not RemoteCommands.get_running():
                 (cmd, arg) = RemoteCommands.command_queue.get()
                 cmd = cmd.upper()
-                arg = arg.strip()
                 if cmd in self.commands:
+                    arg = arg.strip()
                     log.debug("Executing command: %s %r", cmd, arg)
                     RemoteCommands.set_running(True)
+                    thread.to_main_with_blocking(self.commands[cmd], arg)
 
-                    # FIXME: Find a way to execute each command using a block to ensure completion
-                    # before executing the next command.  Perhaps track completion of all threads
-                    # created while "command_running" is True and only clear_command_running()
-                    # when all those threads have completed?  That may also remove the need for
-                    # the special "no_auto_complete" processing.
-
-                    thread.to_main(self.commands[cmd], arg)
-                    # if cmd in no_auto_complete:
-                    #     break
-                    self.clear_command_running()
+                    # 30 second timeout to avoid hanging up command processing indefinitely
+                    RemoteCommands.wait_for_completion(30)
+                    RemoteCommands.clear_command_running()
+                    log.debug("Completed command: %s %r", cmd, arg)
                 else:
                     log.error("Unknown command: %r", cmd)
                 RemoteCommands.command_queue.task_done()
+            time.sleep(.01)
 
     def _run_commands_finished(self, result=None, error=None):
         if error:
diff --git a/picard/util/remotecommands.py b/picard/util/remotecommands.py
index 01d44c9e5..328116d36 100644
--- a/picard/util/remotecommands.py
+++ b/picard/util/remotecommands.py
@@ -201,7 +201,6 @@ class RemoteCommands:
     def parse_commands_to_queue(cls, commands):
         if cls.has_quit():
             # Don't queue any more commands after a QUIT command.
-            print(f"has_quit = {cls.has_quit()}\n\n")
             return
 
         for (cmd, cmdargs) in commands:
@@ -253,3 +252,11 @@ class RemoteCommands:
         cls.cmd_files_add(filepath)
         cls.parse_commands_to_queue(cls._read_commands_from_file(filepath))
         cls.cmd_files_remove(filepath)
+
+    @classmethod
+    def wait_for_completion(cls, timeout=None):
+        end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout) if timeout else None
+        while True:
+            time.sleep(.01)
+            if not cls.processing() or (timeout and datetime.datetime.now() > end_time):
+                return
diff --git a/picard/util/thread.py b/picard/util/thread.py
index ccb7675b0..23b865e5f 100644
--- a/picard/util/thread.py
+++ b/picard/util/thread.py
@@ -27,7 +27,9 @@
 
 
 import sys
+import time
 import traceback
+import uuid
 
 from PyQt5.QtCore import (
     QCoreApplication,
@@ -36,6 +38,7 @@ from PyQt5.QtCore import (
 )
 
 from picard import log
+from picard.util.remotecommands import RemoteCommands
 
 
 class ProxyToMainEvent(QEvent):
@@ -57,8 +60,11 @@ class Runnable(QRunnable):
         self.func = func
         self.next_func = next_func
         self.traceback = traceback
+        self.id = uuid.uuid4()
 
     def run(self):
+        if RemoteCommands.get_running():
+            RemoteCommands.thread_add(self.id)
         try:
             result = self.func()
         except BaseException:
@@ -67,6 +73,7 @@ class Runnable(QRunnable):
             to_main(self.next_func, error=sys.exc_info()[1])
         else:
             to_main(self.next_func, result=result)
+        RemoteCommands.thread_remove(self.id)
 
 
 def run_task(func, next_func, priority=0, thread_pool=None, traceback=True):
@@ -92,3 +99,23 @@ def run_task(func, next_func, priority=0, thread_pool=None, traceback=True):
 def to_main(func, *args, **kwargs):
     QCoreApplication.postEvent(QCoreApplication.instance(),
                                ProxyToMainEvent(func, *args, **kwargs))
+
+
+def to_main_with_blocking(func, *args, **kwargs):
+    """Executes a command as a user-defined event, and waits until the event has
+    closed before returning.  Note that any new threads started while processing
+    the event will not be considered when releasing the blocking of the function.
+
+    Args:
+        func: Function to run.
+    """
+    _task = ProxyToMainEvent(func, *args, **kwargs)
+    QCoreApplication.postEvent(QCoreApplication.instance(), _task)
+
+    while True:
+        try:
+            if not _task.isAccepted():
+                break
+        except Exception:
+            break
+        time.sleep(.01)

From 511eeac9a2126e28d46758edfd0c045b7af96230 Mon Sep 17 00:00:00 2001
From: Bob Swift <bswift@rsds.ca>
Date: Tue, 29 Nov 2022 18:33:43 -0700
Subject: [PATCH 4/6] Use counts of active threads in thread pools for task
 completion:

- Remove obsolete `command` scheme
- Remove thread tracking
- Add command completion checks based on thread pool active thread
counts
- add new `PAUSE` executable command
- Fix album parsing to avoid exception due to changing dictionary size
during deletion
- Add encoding when writing log file to avoid exception
- Update help text on some commands
- Add docstrings
- Add default `_no_operation()` function as `next_func` argument to
`run_task()` method
---
 picard/tagger.py              | 101 ++++++++++++++--------
 picard/util/remotecommands.py | 156 ++++++++++++++++++++--------------
 picard/util/thread.py         |  15 ++--
 3 files changed, 168 insertions(+), 104 deletions(-)

diff --git a/picard/tagger.py b/picard/tagger.py
index 9f7b46bdb..3beb54fc6 100644
--- a/picard/tagger.py
+++ b/picard/tagger.py
@@ -45,6 +45,7 @@
 
 
 import argparse
+import datetime
 from functools import partial
 from hashlib import md5
 import logging
@@ -187,7 +188,6 @@ class ParseItemsToLoad:
     WINDOWS_DRIVE_TEST = re.compile(r"^[a-z]\:", re.IGNORECASE)
 
     def __init__(self, items):
-        self.commands = []
         self.files = set()
         self.mbids = set()
         self.urls = set()
@@ -197,11 +197,7 @@ class ParseItemsToLoad:
             log.debug(f"Parsed: {repr(parsed)}")
             if not parsed.scheme:
                 self.files.add(item)
-            # TODO: Remove the "command" scheme if it is no longer required.
-            elif parsed.scheme == "command":
-                for x in item[10:].split(';'):
-                    self.commands.append(x.strip())
-            elif parsed.scheme == "file":
+            if parsed.scheme == "file":
                 # remove file:// prefix safely
                 self.files.add(item[7:])
             elif parsed.scheme == "mbid":
@@ -219,10 +215,10 @@ class ParseItemsToLoad:
         return bool(self.files or self.mbids or self.urls)
 
     def __bool__(self):
-        return bool(self.commands or self.files or self.mbids or self.urls)
+        return bool(self.files or self.mbids or self.urls)
 
     def __str__(self):
-        return f"files: {repr(self.files)}  mbids: f{repr(self.mbids)}  urls: {repr(self.urls)}  commands: {repr(self.commands)}"
+        return f"files: {repr(self.files)}  mbids: f{repr(self.mbids)}  urls: {repr(self.urls)}"
 
 
 class Tagger(QtWidgets.QApplication):
@@ -392,24 +388,58 @@ class Tagger(QtWidgets.QApplication):
         else:
             log.debug('pipe server stopped')
 
-    def clear_command_running(self, *args, **kwargs):
-        RemoteCommands.set_running(False)
-
     def run_commands(self):
         while not self.stopping:
             if not RemoteCommands.command_queue.empty() and not RemoteCommands.get_running():
                 (cmd, arg) = RemoteCommands.command_queue.get()
-                cmd = cmd.upper()
                 if cmd in self.commands:
                     arg = arg.strip()
-                    log.debug("Executing command: %s %r", cmd, arg)
-                    RemoteCommands.set_running(True)
-                    thread.to_main_with_blocking(self.commands[cmd], arg)
+                    log.info("Executing command: %s %r", cmd, arg)
+                    if cmd == 'QUIT':
+                        thread.to_main(self.commands[cmd], arg)
+                    else:
+                        RemoteCommands.set_running(True)
+                        original_priority_thread_count = self.priority_thread_pool.activeThreadCount()
+                        original_main_thread_count = self.thread_pool.activeThreadCount()
+                        original_save_thread_count = self.save_thread_pool.activeThreadCount()
+                        thread.to_main_with_blocking(self.commands[cmd], arg)
+
+                        # 30 second timeout to avoid hanging up command processing indefinitely
+                        end_time = datetime.datetime.now() + datetime.timedelta(30)
+
+                        # Continue to show the task as running until the timeout is reached or
+                        # until all of the following conditions are met:
+                        #
+                        #   - main thread pool active tasks count is less than or equal to the
+                        #     count at the start of task execution
+                        #
+                        #   - priority thread pool active tasks count is less than or equal to
+                        #     the count at the start of task execution
+                        #
+                        #   - save thread pool active tasks count is less than or equal to the
+                        #     count at the start of task execution
+                        #
+                        #   - there are no pending webservice requests
+                        #
+                        #   - there are no acoustid fingerprinting tasks running
+
+                        while True:
+                            time.sleep(0.1)
+                            if datetime.datetime.now() > end_time:
+                                break
+
+                            if self.priority_thread_pool.activeThreadCount() > original_priority_thread_count or \
+                                    self.thread_pool.activeThreadCount() > original_main_thread_count or \
+                                    self.save_thread_pool.activeThreadCount() > original_save_thread_count or \
+                                    self.webservice.num_pending_web_requests or \
+                                    self._acoustid._running:
+                                continue
+
+                            break
+
+                        log.info("Completed command: %s %r", cmd, arg)
+                        RemoteCommands.set_running(False)
 
-                    # 30 second timeout to avoid hanging up command processing indefinitely
-                    RemoteCommands.wait_for_completion(30)
-                    RemoteCommands.clear_command_running()
-                    log.debug("Completed command: %s %r", cmd, arg)
                 else:
                     log.error("Unknown command: %r", cmd)
                 RemoteCommands.command_queue.task_done()
@@ -441,16 +471,6 @@ class Tagger(QtWidgets.QApplication):
     def _init_remote_commands(self):
         self.commands = {name: getattr(self, remcmd.method_name) for name, remcmd in REMOTE_COMMANDS.items()}
 
-    def handle_command(self, command):
-        cmd, *args = command.split(' ', 1)
-        argstring = next(iter(args), "")
-        cmd = cmd.upper()
-        log.debug("Executing command: %r", cmd)
-        try:
-            thread.to_main(self.commands[cmd], argstring.strip())
-        except KeyError:
-            log.error("Unknown command: %r", cmd)
-
     def handle_command_clear_logs(self, argstring):
         self.window.log_dialog.clear()
         self.window.history_dialog.clear()
@@ -466,10 +486,6 @@ class Tagger(QtWidgets.QApplication):
         RemoteCommands.get_commands_from_file(argstring)
 
     def handle_command_load(self, argstring):
-        # TODO: Remove this check if "command://" no longer used.
-        if argstring.startswith("command://"):
-            log.error("Cannot LOAD a command: %s", argstring)
-            return
         parsed_items = ParseItemsToLoad([argstring])
         log.debug(str(parsed_items))
 
@@ -517,6 +533,20 @@ class Tagger(QtWidgets.QApplication):
             partial(self._lookup_disc, disc),
             traceback=self._debug)
 
+    def handle_command_pause(self, argstring):
+        arg = argstring.strip()
+        if arg:
+            try:
+                _delay = float(arg)
+                if _delay < 0:
+                    raise ValueError
+                log.debug(f"Pausing command execution by {_delay} seconds.")
+                thread.run_task(partial(time.sleep, _delay))
+            except ValueError:
+                log.error(f"Invalid command pause time specified: {repr(argstring)}")
+        else:
+            log.error("No command pause time specified.")
+
     def handle_command_quit(self, argstring):
         self.exit()
         self.quit()
@@ -532,7 +562,8 @@ class Tagger(QtWidgets.QApplication):
             self.remove([file])
 
     def handle_command_remove_empty(self, argstring):
-        for album in self.albums:
+        _albums = [a for a in self.albums.values()]
+        for album in _albums:
             if not any(album.iterfiles()):
                 self.remove_album(album)
 
@@ -569,7 +600,7 @@ class Tagger(QtWidgets.QApplication):
 
     def handle_command_write_logs(self, argstring):
         try:
-            with open(argstring, 'w') as f:
+            with open(argstring, 'w', encoding='utf8') as f:
                 for x in self.window.log_dialog.log_tail.contents():
                     f.write(f"{x.message}\n")
         except Exception as e:
diff --git a/picard/util/remotecommands.py b/picard/util/remotecommands.py
index 328116d36..a59956ac6 100644
--- a/picard/util/remotecommands.py
+++ b/picard/util/remotecommands.py
@@ -18,12 +18,10 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
-import datetime
 import os
 import queue
 import shlex
 import threading
-import time
 
 from picard import log
 
@@ -50,13 +48,13 @@ REMOTE_COMMANDS = {
     ),
     "FROM_FILE": RemoteCommand(
         "handle_command_from_file",
-        help_text="Load command pipeline from a file.",
-        help_args="[Absolute path to a file containing command pipeline]",
+        help_text="Load commands from a file.",
+        help_args="[Path to a file containing commands]",
     ),
     "LOAD": RemoteCommand(
         "handle_command_load",
-        help_text="Load 1 or more files/MBIDs/URLs to Picard.",
-        help_args="[supported MBID/URL or absolute path to a file]",
+        help_text="Load one or more files/MBIDs/URLs to Picard.",
+        help_args="[supported MBID/URL or path to a file]",
     ),
     "LOOKUP": RemoteCommand(
         "handle_command_lookup",
@@ -69,6 +67,11 @@ REMOTE_COMMANDS = {
         "Without argument, it defaults to the first (alphabetically) available disc drive",
         help_args="[device/log file]",
     ),
+    "PAUSE": RemoteCommand(
+        "handle_command_pause",
+        help_text="Pause executable command processing.",
+        help_args="[number of seconds to pause]",
+    ),
     "QUIT": RemoteCommand(
         "handle_command_quit",
         help_text="Exit the running instance of Picard.",
@@ -76,7 +79,7 @@ REMOTE_COMMANDS = {
     "REMOVE": RemoteCommand(
         "handle_command_remove",
         help_text="Remove the file from Picard. Do nothing if no arguments provided.",
-        help_args="[absolute path to 1 or more files]",
+        help_args="[absolute path to one or more files]",
     ),
     "REMOVE_ALL": RemoteCommand(
         "handle_command_remove_all",
@@ -88,7 +91,7 @@ REMOTE_COMMANDS = {
     ),
     "REMOVE_SAVED": RemoteCommand(
         "handle_command_remove_saved",
-        help_text="Remove all saved releases from the album pane.",
+        help_text="Remove all saved files from the album pane.",
     ),
     "REMOVE_UNCLUSTERED": RemoteCommand(
         "handle_command_remove_unclustered",
@@ -96,7 +99,7 @@ REMOTE_COMMANDS = {
     ),
     "SAVE_MATCHED": RemoteCommand(
         "handle_command_save_matched",
-        help_text="Save all matched releases from the album pane."
+        help_text="Save all matched files from the album pane."
     ),
     "SAVE_MODIFIED": RemoteCommand(
         "handle_command_save_modified",
@@ -117,12 +120,14 @@ REMOTE_COMMANDS = {
     "WRITE_LOGS": RemoteCommand(
         "handle_command_write_logs",
         help_text="Write Picard logs to a given path.",
-        help_args="[absolute path to 1 file]",
+        help_args="[absolute path to one file]",
     ),
 }
 
 
 class RemoteCommands:
+    """Handler for remote commands processed from the command line using the '-e' option.
+    """
     # Collection of command files currently being parsed
     _command_files = set()
 
@@ -134,71 +139,92 @@ class RemoteCommands:
 
     _lock = threading.Lock()
     command_queue = queue.Queue()
-    _threads = set()
 
     @classmethod
-    def cmd_files_contains(cls, filepath):
+    def cmd_files_contains(cls, filepath: str):
+        """Check if the specified filepath is currently open for reading commands.
+
+        Args:
+            filepath (str): File path to check.
+
+        Returns:
+            bool: True if the filepath is open for processing, otherwise False.
+        """
         with cls._lock:
             return filepath in cls._command_files
 
     @classmethod
-    def cmd_files_add(cls, filepath):
+    def cmd_files_add(cls, filepath: str):
+        """Adds the specified filepath to the collection of files currently open
+        for reading commands.
+
+        Args:
+            filepath (str): File path to add.
+        """
         with cls._lock:
             cls._command_files.add(filepath)
 
     @classmethod
-    def cmd_files_remove(cls, filepath):
+    def cmd_files_remove(cls, filepath: str):
+        """Removes the specified filepath from the collection of files currently
+        open for reading commands.
+
+        Args:
+            filepath (str): File path to remove.
+        """
         with cls._lock:
             cls._command_files.discard(filepath)
 
     @classmethod
     def has_quit(cls):
+        """Indicates whether a 'QUIT' command has been added to the command queue.
+
+        Returns:
+            bool: True if a 'QUIT' command has been queued, otherwise False.
+        """
         with cls._lock:
             return cls._has_quit
 
     @classmethod
-    def set_quit(cls, value):
+    def set_quit(cls, value: bool):
+        """Sets the status of the 'has_quit()' flag.
+
+        Args:
+            value (bool): Value to set for the 'has_quit()' flag.
+        """
         with cls._lock:
             cls._has_quit = value
 
-    @classmethod
-    def thread_add(cls, thread_id):
-        with cls._lock:
-            cls._threads.add(thread_id)
-
-    @classmethod
-    def thread_remove(cls, thread_id):
-        with cls._lock:
-            cls._threads.discard(thread_id)
-
-    @classmethod
-    def processing(cls):
-        with cls._lock:
-            return (len(cls._threads) > 0)
-
     @classmethod
     def get_running(cls):
+        """Indicates whether a command is currently set as active regardless of
+        processing status.
+
+        Returns:
+            bool: True if there is an active command, otherwise False.
+        """
         with cls._lock:
             return cls._command_running
 
     @classmethod
-    def set_running(cls, value):
+    def set_running(cls, value: bool):
+        """Sets the status of the 'get_running()' flag.
+
+        Args:
+            value (bool): Value to set for the 'get_running()' flag.
+        """
         with cls._lock:
             cls._command_running = value
 
-    @classmethod
-    def clear_command_running(cls, force=False, timeout=None):
-        end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout) if timeout else None
-        while True:
-            if not cls.processing() or force or (timeout and datetime.datetime.now() > end_time):
-                with cls._lock:
-                    cls._command_running = False
-                    cls._threads = set()
-                break
-            time.sleep(.01)
-
     @classmethod
     def parse_commands_to_queue(cls, commands):
+        """Parses the list of command tuples, and adds them to the command queue.  If the command
+        is 'FROM_FILE' then the commands will be read from the file recursively.  Once a 'QUIT'
+        command has been queued, all further commands will be ignored and not placed in the queue.
+
+        Args:
+            commands (list): Command tuples in the form (command, [args]) to add to the queue.
+        """
         if cls.has_quit():
             # Don't queue any more commands after a QUIT command.
             return
@@ -221,7 +247,15 @@ class RemoteCommands:
                         return
 
     @staticmethod
-    def _read_commands_from_file(filepath):
+    def _read_commands_from_file(filepath: str):
+        """Reads the commands from the specified filepath.
+
+        Args:
+            filepath (str): File to read.
+
+        Returns:
+            list: Command tuples in the form (command, [args]).
+        """
         commands = []
         try:
             lines = open(filepath).readlines()
@@ -240,23 +274,21 @@ class RemoteCommands:
         return commands
 
     @classmethod
-    def get_commands_from_file(cls, argstring):
-        log.debug("Reading commands from: %r", argstring)
-        if not os.path.exists(argstring):
-            log.error("Missing command file: '%s'", argstring)
-            return
-        filepath = os.path.abspath(argstring)
-        if cls.cmd_files_contains(filepath):
-            log.warning("Circular command file reference ignored: '%s'", argstring)
-            return
-        cls.cmd_files_add(filepath)
-        cls.parse_commands_to_queue(cls._read_commands_from_file(filepath))
-        cls.cmd_files_remove(filepath)
+    def get_commands_from_file(cls, filepath: str):
+        """Reads and parses the commands from the specified filepath and adds
+        them to the command queue for processing.
 
-    @classmethod
-    def wait_for_completion(cls, timeout=None):
-        end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout) if timeout else None
-        while True:
-            time.sleep(.01)
-            if not cls.processing() or (timeout and datetime.datetime.now() > end_time):
-                return
+        Args:
+            filepath (str): File to read.
+        """
+        log.debug("Reading commands from: %r", filepath)
+        if not os.path.exists(filepath):
+            log.error("Missing command file: '%s'", filepath)
+            return
+        absfilepath = os.path.abspath(filepath)
+        if cls.cmd_files_contains(absfilepath):
+            log.warning("Circular command file reference ignored: '%s'", filepath)
+            return
+        cls.cmd_files_add(absfilepath)
+        cls.parse_commands_to_queue(cls._read_commands_from_file(absfilepath))
+        cls.cmd_files_remove(absfilepath)
diff --git a/picard/util/thread.py b/picard/util/thread.py
index 23b865e5f..1725a957a 100644
--- a/picard/util/thread.py
+++ b/picard/util/thread.py
@@ -10,6 +10,7 @@
 # Copyright (C) 2017 Sophist-UK
 # Copyright (C) 2018 Vishal Choudhary
 # Copyright (C) 2020, 2022 Philipp Wolfer
+# Copyright (C) 2022 Bob Swift
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -29,7 +30,6 @@
 import sys
 import time
 import traceback
-import uuid
 
 from PyQt5.QtCore import (
     QCoreApplication,
@@ -38,7 +38,6 @@ from PyQt5.QtCore import (
 )
 
 from picard import log
-from picard.util.remotecommands import RemoteCommands
 
 
 class ProxyToMainEvent(QEvent):
@@ -60,11 +59,8 @@ class Runnable(QRunnable):
         self.func = func
         self.next_func = next_func
         self.traceback = traceback
-        self.id = uuid.uuid4()
 
     def run(self):
-        if RemoteCommands.get_running():
-            RemoteCommands.thread_add(self.id)
         try:
             result = self.func()
         except BaseException:
@@ -73,10 +69,9 @@ class Runnable(QRunnable):
             to_main(self.next_func, error=sys.exc_info()[1])
         else:
             to_main(self.next_func, result=result)
-        RemoteCommands.thread_remove(self.id)
 
 
-def run_task(func, next_func, priority=0, thread_pool=None, traceback=True):
+def run_task(func, next_func=None, priority=0, thread_pool=None, traceback=True):
     """Schedules func to be run on a separate thread
 
     Args:
@@ -91,6 +86,12 @@ def run_task(func, next_func, priority=0, thread_pool=None, traceback=True):
     Returns:
         An instance of concurrent.futures.Future
     """
+    def _no_operation(*args, **kwargs):
+        return
+
+    if not next_func:
+        next_func = _no_operation
+
     if not thread_pool:
         thread_pool = QCoreApplication.instance().thread_pool
     thread_pool.start(Runnable(func, next_func, traceback), priority)

From 5618f2222e7f8cef0768c88adfb44ad5e06cf253 Mon Sep 17 00:00:00 2001
From: Bob Swift <bswift@rsds.ca>
Date: Wed, 30 Nov 2022 04:45:14 -0700
Subject: [PATCH 5/6] Rename local variable

---
 picard/tagger.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/picard/tagger.py b/picard/tagger.py
index 3beb54fc6..634bfcab7 100644
--- a/picard/tagger.py
+++ b/picard/tagger.py
@@ -537,11 +537,11 @@ class Tagger(QtWidgets.QApplication):
         arg = argstring.strip()
         if arg:
             try:
-                _delay = float(arg)
-                if _delay < 0:
+                delay = float(arg)
+                if delay < 0:
                     raise ValueError
-                log.debug(f"Pausing command execution by {_delay} seconds.")
-                thread.run_task(partial(time.sleep, _delay))
+                log.debug(f"Pausing command execution by {delay} seconds.")
+                thread.run_task(partial(time.sleep, delay))
             except ValueError:
                 log.error(f"Invalid command pause time specified: {repr(argstring)}")
         else:

From 939fcda53724a18b550fefb8692b2c0bbbbf373a Mon Sep 17 00:00:00 2001
From: Bob Swift <bswift@rsds.ca>
Date: Wed, 30 Nov 2022 04:51:47 -0700
Subject: [PATCH 6/6] Remove unneeded command timeout

---
 picard/tagger.py | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/picard/tagger.py b/picard/tagger.py
index 634bfcab7..33aaefbb7 100644
--- a/picard/tagger.py
+++ b/picard/tagger.py
@@ -45,7 +45,6 @@
 
 
 import argparse
-import datetime
 from functools import partial
 from hashlib import md5
 import logging
@@ -404,11 +403,8 @@ class Tagger(QtWidgets.QApplication):
                         original_save_thread_count = self.save_thread_pool.activeThreadCount()
                         thread.to_main_with_blocking(self.commands[cmd], arg)
 
-                        # 30 second timeout to avoid hanging up command processing indefinitely
-                        end_time = datetime.datetime.now() + datetime.timedelta(30)
-
-                        # Continue to show the task as running until the timeout is reached or
-                        # until all of the following conditions are met:
+                        # Continue to show the task as running until all of the following
+                        # conditions are met:
                         #
                         #   - main thread pool active tasks count is less than or equal to the
                         #     count at the start of task execution
@@ -425,16 +421,12 @@ class Tagger(QtWidgets.QApplication):
 
                         while True:
                             time.sleep(0.1)
-                            if datetime.datetime.now() > end_time:
-                                break
-
                             if self.priority_thread_pool.activeThreadCount() > original_priority_thread_count or \
                                     self.thread_pool.activeThreadCount() > original_main_thread_count or \
                                     self.save_thread_pool.activeThreadCount() > original_save_thread_count or \
                                     self.webservice.num_pending_web_requests or \
                                     self._acoustid._running:
                                 continue
-
                             break
 
                         log.info("Completed command: %s %r", cmd, arg)