diff --git a/borgmatic/actions/browse/directory_list.py b/borgmatic/actions/browse/directory_list.py index 6e39c912..569076e8 100644 --- a/borgmatic/actions/browse/directory_list.py +++ b/borgmatic/actions/browse/directory_list.py @@ -6,8 +6,8 @@ import textual.widgets import borgmatic.actions.browse.archive import borgmatic.actions.browse.bindings -import borgmatic.actions.browse.loading import borgmatic.actions.browse.icons +import borgmatic.actions.browse.loading import borgmatic.actions.browse.workers @@ -65,7 +65,10 @@ def add_archive_paths( ): ''' Given a DirectoryList instance, a configuration dict, a repository dict, an archive name, and a - sequence of ArchivePath instances, add the archive paths to the directory list as options. + sequence of ArchivePath instances, add the paths to the directory list as options, sorting and + deduplicating the resulting directory list's options. + + After all of this reshuffling, make sure the orignal highlighted option remains highlighted. ''' highlighted_option = directory_list.highlighted_option original_options_count = len(directory_list.options) @@ -86,6 +89,7 @@ def add_archive_paths( if not relative_path_components[0] in directory_list._id_to_option ), ), + # The loading indicator "option" always goes to the bottom. key=lambda option: ((option.id == 'loading-indicator'), option.prompt), ) @@ -93,6 +97,7 @@ def add_archive_paths( if len(sorted_options) == original_options_count: return + # Retain the highlighted option position even as other options load around it. directory_list.set_options(sorted_options) directory_list.highlighted = ( directory_list.get_option_index(highlighted_option.id) @@ -102,9 +107,23 @@ def add_archive_paths( class Directory_list(textual.widgets.OptionList): + ''' + A widget for selecting a path from among the contents of a particular directory in a Borg + archive. The item selection event is handled in a Carousel instance, the parent widget of a + Directory_list. + ''' + BINDINGS = borgmatic.actions.browse.bindings.OPTION_LIST_BINDINGS def __init__(self, config, repository, archive_name, path_loaded=None, path_components=None): + ''' + 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. + ''' self.config = config self.repository = repository self.archive_name = archive_name @@ -115,9 +134,7 @@ class Directory_list(textual.widgets.OptionList): self.border_title = ' '.join( ( - borgmatic.actions.browse.icons.PATH_TYPE_ICONS[ - borgmatic.actions.browse.archive.Path_type.DIRECTORY.value - ], + '📁', os.path.sep.join(self.path_components) if self.path_components else f'{archive_name}', @@ -127,7 +144,7 @@ class Directory_list(textual.widgets.OptionList): if self.path_components: self.add_option( textual.widgets.option_list.Option( - f'{borgmatic.actions.browse.icons.PATH_TYPE_ICONS[borgmatic.actions.browse.archive.Path_type.DIRECTORY.value]} ..', + '📁 ..', id='..', ), ) @@ -139,6 +156,25 @@ 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, + directory_list=self, + config=self.config, + repository=self.repository, + archive_name=self.archive_name, + ) + + def on_mount(self): + ''' + When this widgets 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. + ''' + self.path_loaded.subscribe(self, self.on_archive_path_loaded) + if self.path_components: add_archive_paths( directory_list=self, @@ -149,19 +185,12 @@ class Directory_list(textual.widgets.OptionList): self.path_loaded.path_hierarchy, self.path_components ), ) - else: - borgmatic.actions.browse.workers.load_archive_paths( - self.app, - directory_list=self, - config=self.config, - repository=self.repository, - archive_name=self.archive_name, - ) - - def on_mount(self): - self.path_loaded.subscribe(self, self.on_archive_path_loaded) def on_archive_path_loaded(self, data): + ''' + When an archive path loads, add it as an option to this directory 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.timer.stop() self.remove_option('loading-indicator') @@ -176,5 +205,10 @@ class Directory_list(textual.widgets.OptionList): ) def on_option_list_option_highlighted(self, event): + ''' + When the highlighted option changes, record that fact. This flag is consumed in + add_archive_paths() in order to retain the highlighted option even as other options load + around it. + ''' if self.highlighted not in {None, 0}: self.highlighted_option_changed = True diff --git a/tests/integration/actions/browse/test_directory_list.py b/tests/integration/actions/browse/test_directory_list.py new file mode 100644 index 00000000..19f506a0 --- /dev/null +++ b/tests/integration/actions/browse/test_directory_list.py @@ -0,0 +1,296 @@ +from borgmatic.actions.browse import directory_list as module + +from flexmock import flexmock +import textual.widgets +import textual.widgets.option_list + + +def test_add_archive_paths_with_only_duplicate_paths_bails(): + directory_list = textual.widgets.OptionList() + directory_list.path_components = ('etc',) + directory_list.add_option(textual.widgets.option_list.Option('foo', id='foo')) + directory_list.add_option(textual.widgets.option_list.Option('bar', id='bar')) + config = {'repositories': [{'path': 'test.borg'}]} + flexmock(directory_list).should_receive('set_options').never() + + module.add_archive_paths( + directory_list=directory_list, + config=config, + repository=config['repositories'][0], + archive_name='archive', + archive_paths=( + flexmock(path_type='-', file_path='etc/foo/one.txt', link_target=''), + flexmock(path_type='-', file_path='etc/foo/two.txt', link_target=''), + ), + ) + + +def test_add_archive_paths_adds_ands_sorts_and_filters_and_deduplicates(): + directory_list = textual.widgets.OptionList() + directory_list.path_components = ('etc',) + directory_list.add_option(textual.widgets.option_list.Option('📄 foo', id='foo')) + directory_list.add_option(textual.widgets.option_list.Option('📄 bar', id='bar')) + directory_list.highlighted = 1 + directory_list.highlighted_option_changed = True + config = {'repositories': [{'path': 'test.borg'}]} + + module.add_archive_paths( + directory_list=directory_list, + config=config, + repository=config['repositories'][0], + archive_name='archive', + archive_paths=( + flexmock(path_type='d', file_path='etc/quux', link_target=''), + flexmock(path_type='-', file_path='etc/foo', link_target=''), + flexmock(path_type='-', file_path='etc/baz', link_target=''), + flexmock(path_type='d', file_path='root/nope', link_target=''), + flexmock(path_type='d', file_path='etc/other', link_target=''), + ), + ) + + assert len(directory_list.options) == 5 + assert directory_list.options[0].prompt == '📁 other' + assert directory_list.options[0].id == 'other' + assert directory_list.options[1].prompt == '📁 quux' + assert directory_list.options[1].id == 'quux' + assert directory_list.options[2].prompt == '📄 bar' + assert directory_list.options[2].id == 'bar' + assert directory_list.options[3].prompt == '📄 baz' + assert directory_list.options[3].id == 'baz' + assert directory_list.options[4].prompt == '📄 foo' + assert directory_list.options[4].id == 'foo' + assert directory_list.highlighted == 2 + + +def test_add_archive_paths_highlights_first_option_if_highlight_has_not_changed(): + directory_list = textual.widgets.OptionList() + directory_list.path_components = ('etc',) + directory_list.add_option(textual.widgets.option_list.Option('📄 foo', id='foo')) + directory_list.add_option(textual.widgets.option_list.Option('📄 bar', id='bar')) + directory_list.highlighted = None + directory_list.highlighted_option_changed = False + config = {'repositories': [{'path': 'test.borg'}]} + + module.add_archive_paths( + directory_list=directory_list, + config=config, + repository=config['repositories'][0], + archive_name='archive', + archive_paths=(flexmock(path_type='-', file_path='etc/baz', link_target=''),), + ) + + assert len(directory_list.options) == 3 + assert directory_list.options[0].prompt == '📄 bar' + assert directory_list.options[0].id == 'bar' + assert directory_list.options[1].prompt == '📄 baz' + assert directory_list.options[1].id == 'baz' + assert directory_list.options[2].prompt == '📄 foo' + assert directory_list.options[2].id == 'foo' + assert directory_list.highlighted == 0 + + +def test_add_archive_paths_retains_loading_indicator_at_bottom(): + directory_list = textual.widgets.OptionList() + directory_list.path_components = ('etc',) + directory_list.add_option(textual.widgets.option_list.Option('📄 foo', id='foo')) + directory_list.add_option(textual.widgets.option_list.Option('📄 bar', id='bar')) + directory_list.add_option( + textual.widgets.option_list.Option('loading!!!', id='loading-indicator') + ) + directory_list.highlighted = 0 + directory_list.highlighted_option_changed = True + config = {'repositories': [{'path': 'test.borg'}]} + + module.add_archive_paths( + directory_list=directory_list, + config=config, + repository=config['repositories'][0], + archive_name='archive', + archive_paths=(flexmock(path_type='-', file_path='etc/baz', link_target=''),), + ) + + assert len(directory_list.options) == 4 + assert directory_list.options[0].prompt == '📄 bar' + assert directory_list.options[0].id == 'bar' + assert directory_list.options[1].prompt == '📄 baz' + assert directory_list.options[1].id == 'baz' + assert directory_list.options[2].prompt == '📄 foo' + assert directory_list.options[2].id == 'foo' + assert directory_list.options[3].prompt == 'loading!!!' + assert directory_list.options[3].id == 'loading-indicator' + 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()) + + directory_list = module.Directory_list( + config=flexmock(), repository=flexmock(), archive_name='archive' + ) + assert directory_list.border_title == '📁 archive' + assert len(directory_list.options) == 0 + 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' + ).never() + 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=True), + 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_on_mount_with_root_directory_skips_adding_archives_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).should_receive('add_archive_paths').never() + directory_list = module.Directory_list( + config=flexmock(), repository=flexmock(), archive_name='archive' + ) + flexmock(directory_list.path_loaded).should_receive('subscribe') + + directory_list.on_mount() + + +def test_directory_list_on_mount_with_non_root_directory_adds_archive_paths(): + flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') + flexmock(module.borgmatic.actions.browse.directory_list.Directory_list).should_receive( + 'app' + ).and_return(flexmock()) + flexmock(module).should_receive('add_archive_paths').once() + directory_list = module.Directory_list( + config=flexmock(), repository=flexmock(), archive_name='archive', + path_loaded=flexmock(complete=False, path_hierarchy={'etc': {}}), + path_components=('etc',), + ) + flexmock(directory_list.path_loaded).should_receive('subscribe') + + directory_list.on_mount() + + +def test_on_archive_path_loaded_with_loading_done_signal_removes_loading_indicator(): + flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') + flexmock(module.borgmatic.actions.browse.directory_list.Directory_list).should_receive( + 'app' + ).and_return(flexmock()) + flexmock(module).should_receive('add_archive_paths').never() + directory_list = module.Directory_list( + config=flexmock(), repository=flexmock(), archive_name='archive', + path_loaded=flexmock(complete=False, path_hierarchy={'etc': {}}), + path_components=('etc',), + ) + directory_list.timer = flexmock(stop=lambda: None) + flexmock(directory_list).should_receive('remove_option').once() + + directory_list.on_archive_path_loaded(data=module.borgmatic.actions.browse.workers.LOADING_DONE) + + +def test_on_archive_path_loaded_with_path_loaded_signal_adds_archive_path(): + flexmock(module.borgmatic.actions.browse.loading).should_receive('add_inline_loading_indicator') + flexmock(module.borgmatic.actions.browse.directory_list.Directory_list).should_receive( + 'app' + ).and_return(flexmock()) + flexmock(module).should_receive('add_archive_paths').once() + directory_list = module.Directory_list( + config=flexmock(), repository=flexmock(), archive_name='archive', + path_loaded=flexmock(complete=False, path_hierarchy={'etc': {}}), + path_components=('etc',), + ) + flexmock(directory_list).should_receive('remove_option').never() + + directory_list.on_archive_path_loaded(data=flexmock()) + + +def test_directory_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.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_hierarchy={'etc': {}}), + path_components=('etc',), + ) + directory_list.highlighted = None + + directory_list.on_option_list_option_highlighted(event=flexmock()) + + assert directory_list.highlighted_option_changed is False + + +def test_directory_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.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_hierarchy={'etc': {}}), + path_components=('etc',), + ) + directory_list.add_option(textual.widgets.option_list.Option('zero', id='zero')) + directory_list.highlighted = 0 + + directory_list.on_option_list_option_highlighted(event=flexmock()) + + assert directory_list.highlighted_option_changed is False + + +def test_directory_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.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_hierarchy={'etc': {}}), + path_components=('etc',), + ) + directory_list.add_option(textual.widgets.option_list.Option('zero', id='zero')) + directory_list.add_option(textual.widgets.option_list.Option('one', id='one')) + directory_list.highlighted = 1 + + directory_list.on_option_list_option_highlighted(event=flexmock()) + + assert directory_list.highlighted_option_changed is True diff --git a/tests/unit/actions/browse/test_directories_list.py b/tests/unit/actions/browse/test_directory_list.py similarity index 100% rename from tests/unit/actions/browse/test_directories_list.py rename to tests/unit/actions/browse/test_directory_list.py