Fix #10209: Show message when rescanning with no library folders

[Swarnadip-Kar]

Fixes #10209

Problem

Previously, when users clicked "Rescan Library" with no music folders configured, nothing happened. The scan would silently complete instantly without any feedback, leaving users confused about whether the feature was working.

Solution

This PR adds a helpful dialog that appears when users try to rescan with no folders configured. The dialog clearly explains:

• No music directories are currently configured
• Users should add directories in the library preferences

Screenshot

After this fix:

The dialog now provides clear feedback and guidance to users.

Implementation

• Added noDirectoriesConfigured boolean flag to LibraryScanResultSummary struct
• Set the flag in LibraryScanner::slotStartScan() when no directories are found
• Modified MixxxMainWindow::slotLibraryScanSummaryDlg() to display a custom message when the flag is set
• Used explicit flag instead of sentinel values for clarity.

Testing

Tested on macOS by:

1. Removing all library folders in Preferences → Library
2. Clicking Library → Rescan Library
3. Verifying the new dialog appears with the helpful message
4. Adding folders back and confirming normal scan behavior still works

Feedback welcome - this is my first contribution to Mixxx and I'm eager to learn!

[ronso0]

Welcome and thanks fo rthis PR!

Though, I think you overlooked the fact that the scanner is not only responsible for the selected directories and the tracks inside those, but also for checking the existence of "free" tracks. By that I mean tracks outside the dirs that have been added to the library implicitly by loading them to a player, eg. DnD from file browser. I think that also applies to played tracks from external features like Traktor and Rekordbox, but I'm not sure about that.

So the current "no dirs" early return is wrong. I think we should remove that throw that message (or different messages) only when trackLocations(m_trackDao.getAllTrackLocations()) is empty.

[Swarnadip-Kar]

Thank you for the feedback. You're correct about the free tracks - I overlooked that case in my initial implementation.

Changes

I've updated the code to address this:

• Removed the early return when m_libraryRootDirs.isEmpty()
• The scan now runs to completion regardless of configured directories
• The "no directories configured" message is displayed only when both conditions are met:
  • No directories are configured (noDirectoriesConfigured == true)
  • The library is empty (tracksTotal == 0)

This ensures tracks added via drag-and-drop or external features (Traktor, Rekordbox) are properly handled.

Testing

I tested both scenarios:

No directories configured, empty library

The dialog displays the "no directories configured" message.

No directories configured, one free track present

The dialog shows the standard summary ("1 track in total") without the no-directories message.

Please let me know if this addresses your concern.

[ronso0]

Please, don't move the conversation to commit messages (are you using AI for coding and/or responding??).
Also, you don't need to follow reviewer requests blindly. Instead, let's discuss pros/cons etc. here first.
Thank you!

[Swarnadip-Kar]

Sorry About That. I should have discussed here first.

About AI - I wrote this PR msg in Code with AI for formatting and spellings.

Right Now - the only bug I see is that the user gets no response when he scans Library with no free tracks and library setup

I was thinking about letting the scan run and later check if no tracks are found also with no directories to scan - then we can probably be certain about showing a Response in the UI.

[Swarnadip-Kar]

I’ve updated LibraryScanner::slotStartScan() to check both m_libraryRootDirs and trackLocations before early return, and emit a LibraryScanResultSummary with noDirectoriesConfigured = true when both are empty. MixxxMainWindow::slotLibraryScanSummaryDlg() now shows the no music directories configured dialog in that case. Could you please take another look when you have time, @ronso0?

[ronso0]

Thank you, looks good already, left some commenst.

[ronso0]

I think we need to set the scanner state first, then emit the signal

[Swarnadip-Kar]

Good catch, fixed the order — changeScannerState(IDLE) now comes before emit scanSummary(result).

[ronso0]

see above (do we need this?)
and if we want to have it false by default we should rather add a default constructor for this struct, no?

[Swarnadip-Kar]

Done, added a default constructor to LibraryScanResultSummary that zero-initializes all fields. Removed the in-class initializer.

[Swarnadip-Kar]

All review comments addressed:

• Updated comment to "independently added tracks"
• Simplified the condition comment to a single line
• Kept noDirectoriesConfigured as an explicit flag (avoids false positives when dirs are configured but empty)
• Fixed state order: changeScannerState(IDLE) now called before emit scanSummary(result)
• Added default constructor to LibraryScanResultSummary with zero-initialized fields, removed in-class initializer

Thanks for the thorough review, @ronso0!

[ronso0]

Btw merging the base branch is usually only required if conflicts emerged, or when we need to pull urgent bugfixes that affect the feature that is being worked on.
The reason, or rather my reason, to avoid unneeded base merges is that in small bugfix/features PRs like this one we may want to squash small fixups just before merging the PR. In such cases the Compare view that's shown in the Conversation timeline is really helpful for both reviewers and contributors for verifying that no undesired changes slipped in. With base merges however these views may be quite large and undesired changes are hard(er) to spot.
-- doesn't matter here that much, but please keep it in mind. Thanks!

[Swarnadip-Kar]

Btw merging the base branch is usually only required if conflicts emerged, or when we need to pull urgent bugfixes that affect the feature that is being worked on. The reason, or rather my reason, to avoid unneeded base merges is that in small bugfix/features PRs like this one we may want to squash small fixups just before merging the PR. In such cases the Compare view that's shown in the Conversation timeline is really helpful for both reviewers and contributors for verifying that no undesired changes slipped in. With base merges however these views may be quite large and undesired changes are hard(er) to spot. -- doesn't matter here that much, but please keep it in mind. Thanks!

Understood, thanks for explaining. I merged base assuming it was required to keep the branch up to date — I'll avoid unnecessary base merges going forward and only rebase when there are actual conflicts.

[ronso0]

Okay, great! LGTM and works as expected.

Please squash the commits and rebase onto the parent of the initial commit.
If you've never done that, this should work:
git rebase -i 5f07c48^1

• remove all merge commits in the editor, only your commits shoudl remain
• for your first commit replace pick with r (reword message)
• for the others replace pick with f (fixup) for all of your commits
• confirm, then edit the commit message. Something like this would suffice IMO (taken from your first commit):

  Show message when rescanning with no library folders or tracks
  
  When users try to rescan the library with no folders configured,
  show a helpful message guiding them to add folders instead of
  silently doing nothing.
  

• confirm and proceed with squashing
• force-push

Then hopefully the compare view wil be empty (thumbs pressed ; )

[Swarnadip-Kar]

Done, squashed and force-pushed to a single commit on top of upstream/main.
Quick note on what happened behind the scenes — I had accidentally done two base merges earlier which mixed upstream commits into my branch. To clean it up I had to reset my branch to upstream/main and cherry-pick my 5 commits back on top one by one. (which is why the hashes look different from the originals). Also used HEAD~5 instead of 5f07c48^1 as you suggested, since the original hash no longer existed after the cherry-pick — but the result is the same. Sorry for the mess, learned a lot about keeping branches clean from this!

[Swarnadip-Kar]

Hi @ronso0, just a gentle ping — I've addressed all the feedback on both this PR and #16025 (BPM lock toggle). Let me know if there's anything else to fix!

[ronso0]

(the more often we look at code, the better it gets over time : )
Just an idea to make this if/else flow a bit cleaner, and avoid the double re-assignment of summary:

• move the QMessageBox instantiation up, below the if (result.autoscan && ... block
• then comes the current if (result.noDirectoriesConfigured) {, plus

   pMsg->setText(summary);
   pMsg->show();
   return;
  

• then continue with the other if/else

What do you think?

[Swarnadip-Kar]

Good call, agreed this is cleaner. Moved the QMessageBox instantiation up and made noDirectoriesConfigured an early return — the summary string is now only built when a scan actually ran, so the workaround comment is gone too. Updated.

[ronso0]

Thank you, LGTM and works nicely!
Thanks for your patience.

[ronso0]

Please squash the 2nd commit into the1st one (leave the commit message as is), then we can finally merge.

[Swarnadip-Kar]

Please squash the 2nd commit into the1st one (leave the commit message as is), then we can finally merge.

Done, squashed and force-pushed.

[ronso0]

Thank you! waiting for CI..