Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep the profile list sorted when creating/renaming profiles #1986

Merged
merged 3 commits into from
May 25, 2024

Conversation

Parnassius
Copy link
Contributor

@Parnassius Parnassius commented Apr 7, 2024

Description

Creating a new profile adds the new profile at the bottom of the list in the main window. Similarly, editing an existing profile doesn't change its position in the list. This change keeps the list sorted alphabetically in both cases.

Related Issue

Fixes #1983

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read the CONTRIBUTING guide.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

@shivansh02
Copy link
Collaborator

Screenshot from 2024-04-12 12-28-37
Works fine for me, assuming we want to keep the sorting case sensitive.

@m3nu
Copy link
Contributor

m3nu commented Apr 12, 2024

Thanks for testing, @shivansh02! I don't have a strong preference for the case-sensitivity, but would lean towards case-insensitive sorting.

@Parnassius
Copy link
Contributor Author

The latest commit adds case-insensitive sorting to the list in the main window, as well as the list in the Backup Now tray menu.

The solution I used was to ignore the ORDER BY sql directive and simply sort the profiles with python, using both the casefolded and the normal name as the sorting key. Adding the normal name as well is required to keep the sorting stable, otherwise two profiles called test and TEST would have no fixed ordering.

In my comment on #1983 I talked about the sortItems method of QListWidget, which would be perfect for the list in the main window. However the tray menu doesn't support anything similar, so I decided against it to keep the logic the same in both places.

One final option I thought about was the COLLATE NOCASE sqlite function, but that only converts the 26 ascii uppercase letters to the lowercase equivalents, so the sorting would have remained case-sensitive for non-ascii characters.

@shivansh02
Copy link
Collaborator

The profile list is now sorted case-insensitively, couldn't test the Backup Now list because my distro doesn't have a tray menu.

@m3nu m3nu merged commit 6b5f7a7 into borgbase:master May 25, 2024
11 checks passed
@m3nu
Copy link
Contributor

m3nu commented May 25, 2024

Thanks for the contribution, @Parnassius and thanks to @shivansh02 for reviewing.

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.

Profile list in the main window are not kept sorted when adding/renaming entries
3 participants