diff --git a/borgmatic/actions/browse/archives_list.py b/borgmatic/actions/browse/archives_list.py index e7d5636f..6335a8cd 100644 --- a/borgmatic/actions/browse/archives_list.py +++ b/borgmatic/actions/browse/archives_list.py @@ -19,8 +19,9 @@ class Archives_list(textual.widgets.OptionList): def __init__(self, config, repository): ''' - Given a configuration dict and a repository dict, start loading the archives from the - repository for eventual display in this widget. + Given a configuration dict and a repository dict, prepare to load the archives from the + repository for eventual display in this widget. Actual loading kicks off in on_mount() + below. ''' self.config = config self.repository = repository @@ -34,6 +35,16 @@ class Archives_list(textual.widgets.OptionList): self.loading_timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self) + 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. Also start loading archives from the repository. + + Loading is started *after* subscribing to archive loaded signals so that there's not a gap + where we might miss out on signal publishes. + ''' + self.archive_loaded.subscribe(self, self.on_archive_loaded) + borgmatic.actions.browse.workers.add_repository_archives( self.app, archive_loaded=self.archive_loaded, @@ -42,31 +53,26 @@ class Archives_list(textual.widgets.OptionList): 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): + def on_archive_loaded(self, archive_name): ''' 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: + if archive_name 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,) + label_pieces = ( + (archive_name, '[dim](latest)[/dim]') if len(self.options) == 1 else (archive_name,) + ) 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), + textual.widgets.option_list.Option(' '.join(label_pieces), id=archive_name), loading_indicator, ), ) diff --git a/borgmatic/actions/browse/directory_list.py b/borgmatic/actions/browse/directory_list.py index 7479d621..95a8b747 100644 --- a/borgmatic/actions/browse/directory_list.py +++ b/borgmatic/actions/browse/directory_list.py @@ -119,10 +119,9 @@ class Directory_list(textual.widgets.OptionList): ''' Given a configuration dict, a repository dict, an archive name, an optional Archive_path_loaded instance for signalling new paths as they load, and an optional tuple of - path components indicating this directory's position in the backed up filesystem, start - loading paths from the archive for eventual display in this widget. Or, if paths have - already started loading (by the root directory list), just listen for new paths as they come - in. + path components indicating this directory's position in the backed up filesystem, prepare to + load paths from the archive for eventual display in this widget. Actual loading kicks off in + on_mount() below. ''' self.config = config self.repository = repository @@ -156,22 +155,15 @@ class Directory_list(textual.widgets.OptionList): if not self.path_loaded.complete: self.timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self) - if not self.path_components: - borgmatic.actions.browse.workers.load_archive_paths( - self.app, - path_loaded=self.path_loaded, - config=self.config, - repository=self.repository, - archive_name=self.archive_name, - ) - def on_mount(self): ''' - 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 - paths. + 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 root directory list, start + loading paths from the archive. If this is a non-root directory list, add any already loaded + archive paths to this widget as options. + + Loading is started *after* subscribing to path loaded signals so that there's not a gap + where we might miss out on any paths. ''' self.path_loaded.subscribe(self, self.on_archive_path_loaded) @@ -185,6 +177,14 @@ class Directory_list(textual.widgets.OptionList): self.path_loaded.path_hierarchy, self.path_components ), ) + else: + borgmatic.actions.browse.workers.load_archive_paths( + self.app, + path_loaded=self.path_loaded, + config=self.config, + repository=self.repository, + archive_name=self.archive_name, + ) def on_archive_path_loaded(self, data): ''' diff --git a/borgmatic/actions/browse/file_preview.py b/borgmatic/actions/browse/file_preview.py index 1e9131b9..88b403d6 100644 --- a/borgmatic/actions/browse/file_preview.py +++ b/borgmatic/actions/browse/file_preview.py @@ -1,3 +1,4 @@ +import logging import contextlib import os @@ -9,6 +10,9 @@ import borgmatic.actions.browse.loading import borgmatic.actions.browse.workers +logger = logging.getLogger('__name__') + + class File_preview(textual.widgets.RichLog): ''' A widget for extracting and previewing the contents of a file stored in a Borg archive. @@ -33,7 +37,8 @@ class File_preview(textual.widgets.RichLog): def __init__(self, config, repository, archive_name, file_path): ''' Given a configuration dict, a repository dict, an archive name, and the path of a file in - the archive, start loading the file's contents for eventual display in this widget. + the archive, prepare to load the file's contents for eventual display in this widget. Actual + loading kicks off in on_mount() below. ''' self.config = config self.repository = repository @@ -49,6 +54,16 @@ class File_preview(textual.widgets.RichLog): self.loading_timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self) + 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. Also start loading file contents from the archive. + + Loading is started *after* subscribing to file preview loaded signals so that there's not a + gap where we might miss out on signal publishes. + ''' + self.file_preview_loaded.subscribe(self, self.on_file_preview_loaded) + borgmatic.actions.browse.workers.load_file_preview( self.app, file_preview_loaded=self.file_preview_loaded, @@ -59,20 +74,20 @@ class File_preview(textual.widgets.RichLog): 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, file_contents): + import time - def on_file_preview_loaded(self, data): + logger.debug(('on_file_preview_loaded', time.time())) self.loading_timer.stop() self.clear() - if data is None: + if file_contents is None: self.write('Cannot display a preview for this file') else: + logger.debug(('before write', time.time())) self.write( - rich.syntax.Syntax(data, rich.syntax.Syntax.guess_lexer(self.file_path, data)) + rich.syntax.Syntax( + file_contents, rich.syntax.Syntax.guess_lexer(self.file_path, file_contents) + ) ) + logger.debug(('after write', time.time())) diff --git a/borgmatic/actions/browse/workers.py b/borgmatic/actions/browse/workers.py index bf80cf4c..d84bf187 100644 --- a/borgmatic/actions/browse/workers.py +++ b/borgmatic/actions/browse/workers.py @@ -171,6 +171,9 @@ def load_file_preview( 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. ''' + import time + + logger.debug(('before get_archive_file_content', time.time())) file_contents = borgmatic.actions.browse.archive.get_archive_file_content( config, repository, archive_name, file_path ) diff --git a/tests/integration/actions/browse/test_archives_list.py b/tests/integration/actions/browse/test_archives_list.py index 5bf205e6..4f00c0e6 100644 --- a/tests/integration/actions/browse/test_archives_list.py +++ b/tests/integration/actions/browse/test_archives_list.py @@ -6,10 +6,6 @@ import textual.widgets.option_list def test_archives_list_on_option_list_option_highlighted_with_highlighted_none_marks_it_unchanged(): flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('add_repository_archives') - flexmock(module.borgmatic.actions.browse.archives_list.Archives_list).should_receive( - 'app' - ).and_return(flexmock()) archives_list = module.Archives_list(config=flexmock(), repository=flexmock()) archives_list.highlighted = None @@ -20,10 +16,6 @@ def test_archives_list_on_option_list_option_highlighted_with_highlighted_none_m def test_archives_list_on_option_list_option_highlighted_with_highlighted_zero_marks_it_unchanged(): flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('add_repository_archives') - flexmock(module.borgmatic.actions.browse.archives_list.Archives_list).should_receive( - 'app' - ).and_return(flexmock()) archives_list = module.Archives_list(config=flexmock(), repository=flexmock()) archives_list.highlighted = 0 @@ -34,10 +26,6 @@ def test_archives_list_on_option_list_option_highlighted_with_highlighted_zero_m def test_archives_list_on_option_list_option_highlighted_with_highlighted_zero_marks_it_unchanged(): flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('add_repository_archives') - flexmock(module.borgmatic.actions.browse.archives_list.Archives_list).should_receive( - 'app' - ).and_return(flexmock()) archives_list = module.Archives_list(config=flexmock(), repository=flexmock()) archives_list.add_option(textual.widgets.option_list.Option('zero', id='zero')) @@ -49,10 +37,6 @@ def test_archives_list_on_option_list_option_highlighted_with_highlighted_zero_m def test_archives_list_on_option_list_option_highlighted_with_highlighted_non_zero_marks_it_changed(): flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('add_repository_archives') - flexmock(module.borgmatic.actions.browse.archives_list.Archives_list).should_receive( - 'app' - ).and_return(flexmock()) archives_list = module.Archives_list(config=flexmock(), repository=flexmock()) archives_list.add_option(textual.widgets.option_list.Option('zero', id='zero')) diff --git a/tests/integration/actions/browse/test_directory_list.py b/tests/integration/actions/browse/test_directory_list.py index a6d29cca..99dd358b 100644 --- a/tests/integration/actions/browse/test_directory_list.py +++ b/tests/integration/actions/browse/test_directory_list.py @@ -121,12 +121,8 @@ def test_add_archive_paths_retains_loading_indicator_at_bottom(): assert directory_list.highlighted == 2 -def test_directory_list_with_root_directory_starts_loading_archive_paths(): - flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('load_archive_paths').once() - flexmock(module.borgmatic.actions.browse.directory_list.Directory_list).should_receive( - 'app' - ).and_return(flexmock()) +def test_directory_list_with_root_directory_adds_loading_indicator(): + flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator').once() directory_list = module.Directory_list( config=flexmock(), repository=flexmock(), archive_name='archive' @@ -136,26 +132,6 @@ def test_directory_list_with_root_directory_starts_loading_archive_paths(): assert not directory_list.path_loaded.complete -def test_directory_list_with_non_root_directory_relies_on_existing_path_loading_worker(): - flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('load_archive_paths').never() - flexmock(module.borgmatic.actions.browse.directory_list.Directory_list).should_receive( - 'app' - ).and_return(flexmock()) - - directory_list = module.Directory_list( - config=flexmock(), - repository=flexmock(), - archive_name='archive', - path_loaded=flexmock(complete=False), - path_components=('etc',), - ) - assert directory_list.border_title == '📁 etc' - assert len(directory_list.options) == 1 - assert directory_list.options[0].prompt == '📁 ..' - assert directory_list.options[0].id == '..' - - def test_directory_list_with_already_complete_loading_skips_loading_indicator(): flexmock(module.borgmatic.actions.browse.loading).should_receive( 'add_inline_loading_indicator' @@ -178,12 +154,12 @@ def test_directory_list_with_already_complete_loading_skips_loading_indicator(): assert directory_list.options[0].id == '..' -def test_directory_list_on_mount_with_root_directory_skips_adding_archives_paths(): +def test_directory_list_on_mount_with_root_directory_loads_archive_paths(): flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('load_archive_paths') flexmock(module.borgmatic.actions.browse.directory_list.Directory_list).should_receive( 'app' ).and_return(flexmock()) + flexmock(module.borgmatic.actions.browse.workers).should_receive('load_archive_paths').once() flexmock(module).should_receive('add_archive_paths').never() directory_list = module.Directory_list( config=flexmock(), repository=flexmock(), archive_name='archive' @@ -198,6 +174,7 @@ def test_directory_list_on_mount_with_non_root_directory_adds_archive_paths(): flexmock(module.borgmatic.actions.browse.directory_list.Directory_list).should_receive( 'app' ).and_return(flexmock()) + flexmock(module.borgmatic.actions.browse.workers).should_receive('load_archive_paths').never() flexmock(module).should_receive('add_archive_paths').once() directory_list = module.Directory_list( config=flexmock(), diff --git a/tests/integration/actions/browse/test_file_preview.py b/tests/integration/actions/browse/test_file_preview.py index 72735ae8..4c9ef44e 100644 --- a/tests/integration/actions/browse/test_file_preview.py +++ b/tests/integration/actions/browse/test_file_preview.py @@ -6,10 +6,6 @@ import textual.widgets.option_list def test_file_preview_does_not_raise(): flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') - flexmock(module.borgmatic.actions.browse.workers).should_receive('load_file_preview') - flexmock(module.borgmatic.actions.browse.file_preview.File_preview).should_receive( - 'app' - ).and_return(flexmock()) module.File_preview( config=flexmock(), repository=flexmock(), archive_name='archive', file_path='foo/bar.txt'