Replace cross-thread calls with signals.

borgmatic/actions/browse/archives_list.py

@@ -28,15 +28,54 @@ class Archives_list(textual.widgets.OptionList):
         super().__init__(classes='panel')
         self.border_title = '📚 archives'
         self.highlighted_option_changed = False
+        self.archive_loaded = borgmatic.actions.browse.workers.Archive_loaded(
+            self, 'archive loaded'
+        )
 
-        timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self)
+        self.loading_timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self)
 
         borgmatic.actions.browse.workers.add_repository_archives(
             self.app,
-            archives_list=self,
+            archive_loaded=self.archive_loaded,
             config=self.config,
             repository=self.repository,
-            loading_timer=timer,
+            loading_timer=self.loading_timer,
+        )
+
+    def on_mount(self):
+        '''
+        When this widget gets mounted in the DOM, subscribe to archive loaded events so that we can
+        find out about archives as they load.
+        '''
+        self.archive_loaded.subscribe(self, self.on_archive_loaded)
+
+    def on_archive_loaded(self, data):
+        '''
+        When an archive loads, add it as an option to this archives list. But if we get a
+        signal that all path loading is complete, stop and remove our loading indicator.
+        '''
+        if data is borgmatic.actions.browse.workers.LOADING_DONE:
+            self.loading_timer.stop()
+            self.remove_option('loading-indicator')
+            return
+
+        label_pieces = (data, '[dim](latest)[/dim]') if len(self.options) == 1 else (data,)
+        highlighted_option = self.highlighted_option
+
+        loading_indicator = self.get_option('loading-indicator')
+        self.remove_option('loading-indicator')
+        self.add_options(
+            (
+                textual.widgets.option_list.Option(' '.join(label_pieces), id=data),
+                loading_indicator,
+            ),
+        )
+
+        # Retain the highlighted option position even as other options load around it.
+        self.highlighted = (
+            self.get_option_index(highlighted_option.id)
+            if highlighted_option and self.highlighted_option_changed
+            else 0
         )
 
     def on_option_list_option_highlighted(self, event):

borgmatic/actions/browse/configuration_files_list.py

@@ -15,6 +15,7 @@ class Configuration_files_list(textual.widgets.OptionList):
     files. The item selection event is handled in a Carousel instance, the parent widget of an
     Configuration_files_list.
     '''
+
     BINDINGS = borgmatic.actions.browse.bindings.OPTION_LIST_BINDINGS
 
     def __init__(self, configs):

borgmatic/actions/browse/directory_list.py

@@ -159,7 +159,7 @@ class Directory_list(textual.widgets.OptionList):
         if not self.path_components:
             borgmatic.actions.browse.workers.load_archive_paths(
                 self.app,
-                directory_list=self,
+                path_loaded=self.path_loaded,
                 config=self.config,
                 repository=self.repository,
                 archive_name=self.archive_name,
@@ -167,7 +167,7 @@ class Directory_list(textual.widgets.OptionList):
 
     def on_mount(self):
         '''
-        When this widgets gets mounted in the DOM, subcribe to path loaded events so that we can
+        When this widget gets mounted in the DOM, subcribe to path loaded events so that we can
         find out about relevant archive paths as they load. And if this is a non-root directory
         list, add any already loaded archive paths to this widget as options. This is done *after*
         subscribing to path loaded signals so that there's not a gap where we might miss out on any

borgmatic/actions/browse/file_preview.py

@@ -1,6 +1,7 @@
 import contextlib
 import os
 
+import rich.syntax
 import textual.binding
 import textual.widgets
 
@@ -42,15 +43,36 @@ class File_preview(textual.widgets.RichLog):
         super().__init__(classes='panel')
         self.border_title = ' '.join(('📄', self.file_path, 'preview'))
         self.auto_scroll = False
+        self.file_preview_loaded = borgmatic.actions.browse.workers.File_preview_loaded(
+            self, 'file preview loaded'
+        )
 
-        timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self)
+        self.loading_timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self)
 
         borgmatic.actions.browse.workers.load_file_preview(
             self.app,
-            file_preview=self,
+            file_preview_loaded=self.file_preview_loaded,
             config=self.config,
             repository=self.repository,
             archive_name=self.archive_name,
             file_path=self.file_path,
-            loading_timer=timer,
+            loading_timer=self.loading_timer,
         )
+
+    def on_mount(self):
+        '''
+        When this widget gets mounted in the DOM, subscribe to archive loaded events so that we can
+        find out about archives as they load.
+        '''
+        self.file_preview_loaded.subscribe(self, self.on_file_preview_loaded)
+
+    def on_file_preview_loaded(self, data):
+        self.loading_timer.stop()
+        self.clear()
+
+        if data is None:
+            self.write('Cannot display a preview for this file')
+        else:
+            self.write(
+                rich.syntax.Syntax(data, rich.syntax.Syntax.guess_lexer(self.file_path, data))
+            )

borgmatic/actions/browse/workers.py

@@ -2,7 +2,6 @@ import contextlib
 import logging
 import os
 
-import rich.syntax
 import textual
 import textual.signal
 import textual.widgets.option_list
@@ -13,10 +12,27 @@ import borgmatic.actions.browse.archive
 logger = logging.getLogger('__name__')
 
 
-@textual.work(thread=True)
-def add_repository_archives(browse_app, archives_list, config, repository, loading_timer):
+LOADING_DONE = object()
+
+
+class Archive_loaded(textual.signal.Signal):
     '''
-    Given a running Browse_app instance, an Archives_list instance, a configuration dict, a
+    A signal that publishes when each subsequent archive is loaded from a repository, intended for
+    consumption in widgets that display archives as they are loaded. This signal also publishes
+    when loading is complete.
+
+    Each subscribed callback call includes the archive as an archive name string. Given the lack of
+    other identifying information (configuration file, repository), there should be a separate
+    Archive_loaded instance per repository.
+    '''
+
+    pass
+
+
+@textual.work(thread=True)
+def add_repository_archives(browse_app, archive_loaded, config, repository, loading_timer):
+    '''
+    Given a running Browse_app instance, an Archive_loaded instance, a configuration dict, a
     repository dict, and a loading indicator timer, load a list of the archives from the repository
     and add them as options in the archives list. Reverse the order so the most recent archive is
     first.
@@ -25,34 +41,13 @@ def add_repository_archives(browse_app, archives_list, config, repository, loadi
     loading indicator and stop its timer.
     '''
     archives_data = borgmatic.actions.browse.archive.get_repository_archives(config, repository)
-    loading_option = archives_list.get_option('loading-indicator')
 
     # Reverse the archives, so the common case of accessing the latest archive is easy because it's
     # at the top.
     for index, archive in enumerate(reversed(archives_data['archives'])):
-        label_pieces = (
-            (archive['archive'], '[dim](latest)[/dim]') if index == 0 else (archive['archive'],)
-        )
-        highlighted_option = archives_list.highlighted_option
+        archive_loaded.publish(archive['archive'])
 
-        browse_app.call_from_thread(archives_list.remove_option, 'loading-indicator')
-        browse_app.call_from_thread(
-            archives_list.add_options,
-            (
-                textual.widgets.option_list.Option(' '.join(label_pieces), id=archive['archive']),
-                loading_option,
-            ),
-        )
-
-        # Retain the highlighted option position even as other options load around it.
-        archives_list.highlighted = (
-            archives_list.get_option_index(highlighted_option.id)
-            if highlighted_option and archives_list.highlighted_option_changed
-            else 0
-        )
-
-    browse_app.call_from_thread(archives_list.remove_option, 'loading-indicator')
-    browse_app.call_from_thread(loading_timer.stop)
+    archive_loaded.publish(LOADING_DONE)
 
 
 def record_path(archive_path, hierarchy, path_components):
@@ -108,9 +103,6 @@ def get_paths(hierarchy, path_components, full_path_components=None):
     return get_paths(hierarchy[path_components[0]], path_components[1:], full_path_components)
 
 
-LOADING_DONE = object()
-
-
 class Archive_path_loaded(textual.signal.Signal):
     '''
     A signal that publishes when each subsequent path is loaded from an archive, intended for
@@ -118,7 +110,12 @@ class Archive_path_loaded(textual.signal.Signal):
     complete filesystem hierarchy seen thus far, so new widgets that get created after loading has
     started can "catch up" with existing known paths. Lastly, this signal publishes and tracks when
     loading is complete.
+
+    Each subscrided callback call includes the loaded path as an Archive_path instance. There is
+    intended to be a separate Archive_path_loaded instance per archive, but that instance should be
+    shared among several different widgets for the same archive for performance reasons.
     '''
+
     def __init__(self, owner, name):
         self.path_hierarchy = {}
         self.complete = False
@@ -135,9 +132,9 @@ class Archive_path_loaded(textual.signal.Signal):
 
 
 @textual.work(thread=True)
-def load_archive_paths(browse_app, directory_list, config, repository, archive_name):
+def load_archive_paths(browse_app, path_loaded, config, repository, archive_name):
     '''
-    Given a running Browse_app instance, a Directory_list instance, a configuration dict, a
+    Given a running Browse_app instance, an Archive_path_loaded instance, a configuration dict, a
     repository dict, and an archive name, load the paths in this archive and publish each one via
     the Archive_path_loaded signal, so interested widgets can subscribe. Also send a "loading done"
     signal when loading completes.
@@ -147,30 +144,35 @@ def load_archive_paths(browse_app, directory_list, config, repository, archive_n
     for archive_path in borgmatic.actions.browse.archive.get_archive_paths(
         config, repository, archive_name
     ):
-        directory_list.path_loaded.publish(archive_path)
+        path_loaded.publish(archive_path)
 
-    directory_list.path_loaded.publish(LOADING_DONE)
+    path_loaded.publish(LOADING_DONE)
+
+
+class File_preview_loaded(textual.signal.Signal):
+    '''
+    A signal that publishes when file contents are loaded from an archive, intended for consumption
+    in widgets that display loaded files. This signal also publishes when loading is complete.
+
+    Each published callback includes a the file's contents as a string. Given the lack of other
+    identifying information (configuration file, repository, archive), there should be a separate
+    Archive_loaded instance per previewed file.
+    '''
+
+    pass
 
 
 @textual.work(thread=True)
-def load_file_preview(browse_app, file_preview, config, repository, archive_name, file_path,
-                      loading_timer):
+def load_file_preview(
+    browse_app, file_preview_loaded, config, repository, archive_name, file_path, loading_timer
+):
     '''
-    Given a running Browse_app instance, a File_preview instance, a configuration dict, a repository
-    dict, an archive name, the path of a file in that archive, and a loading indicator timer, load
-    the contents of the file and write it into the given file preview widget.
+    Given a running Browse_app instance, a File_preview_loaded instance, a configuration dict, a
+    repository dict, an archive name, the path of a file in that archive, and a loading indicator
+    timer, load the contents of the file and write it into the given file preview widget.
     '''
-    file_content = borgmatic.actions.browse.archive.get_archive_file_content(
+    file_contents = borgmatic.actions.browse.archive.get_archive_file_content(
         config, repository, archive_name, file_path
     )
 
-    browse_app.call_from_thread(loading_timer.stop)
-    browse_app.call_from_thread(file_preview.clear)
-
-    if file_content is None:
-        browse_app.call_from_thread(file_preview.write, 'Cannot display a preview for this file')
-    else:
-        syntax_lexer = rich.syntax.Syntax.guess_lexer(file_path, file_content)
-        browse_app.call_from_thread(
-            file_preview.write, rich.syntax.Syntax(file_content, syntax_lexer)
-        )
+    file_preview_loaded.publish(file_contents)

tests/integration/actions/browse/test_logs.py

@@ -16,7 +16,8 @@ def test_log_to_widget_adds_our_handler_and_removes_default_handler():
 
         with contextlib.suppress(StopIteration):
             browse_log_handler = next(
-                handler for handler in root_logger.handlers
+                handler
+                for handler in root_logger.handlers
                 if isinstance(handler, module.Browse_log_handler)
             )
 
@@ -26,11 +27,11 @@ def test_log_to_widget_adds_our_handler_and_removes_default_handler():
             root_logger.removeHandler(browse_log_handler)
 
     assert not any(
-        handler for handler in root_logger.handlers
+        handler
+        for handler in root_logger.handlers
         if isinstance(handler, module.borgmatic.logger.Multi_stream_handler)
     )
 
 
-
 def test_logs_does_not_raise():
     module.Logs()