Skip to content

Conversation

@gastoner
Copy link
Collaborator

@gastoner gastoner commented Jan 1, 2026

Adds an option to select a drive for restoring

Closes #506

@gastoner gastoner requested a review from grulja January 1, 2026 20:22
@gastoner gastoner force-pushed the multi_restore branch 4 times, most recently from 25dbd00 to fb126b8 Compare January 1, 2026 21:51
Copy link
Collaborator

@grulja grulja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, but the implementation of the RestoreableDriveManager seems to be odd.

Here are some recommendations:

  • Add names to the roles in DriveManager, currently they are not defined and juse just Qt::UserRole + 1 and Qt::UserRole + 2. To that add something like Restoreable role so this is accessible property. This would look like:
enum ItemRole {
   NameRole = Qt::DisplayRole,
   DriveRole = Qt::UserRole + 1,
   RestoreableRole
}

And do the needed adjustments in the DriveManager implementation

  • Then the RestoreableDriveManager::filterAcceptsRow() can look like:
const QModelIndex index = sourceModel()->index(source_row, 0, source_parent);
const bool isRestoreable = sourceModel()->data(index, DriveManger::RestoreableRole).toBool();
return isRestoreable;
  • I don't think you have to override QSortFilterProxyModel::roleNames() and QSortFilterProxyModel::data().
  • And I think we can get rid of the logic with lastRestoreable or selected property that you just added and simply get the selected drive from the combobox using the DriveRole.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restore option only regards one drive when multiple are present

2 participants