Musicbrainz: Improved messages in case of empty results #13672
Conversation
src/musicbrainz/tagfetcher.cpp
Outdated
| @@ -165,7 +166,8 @@ void TagFetcher::slotAcoustIdTaskSucceeded( | |||
|
|
|||
| emit resultAvailable( | |||
| std::move(pTrack), | |||
| QList<mixxx::musicbrainz::TrackRelease>()); | |||
| {}, | |||
| tr("Could not identify track through Acoustid.")); | |||
There was a problem hiding this comment.
I have also fixed another occourenc, and will also fix Translations after merge.
src/musicbrainz/tagfetcher.cpp
Outdated
| QString whyEmptyMessage; | ||
| if (guessedTrackReleases.empty()) { | ||
| whyEmptyMessage = tr("Could not find this track in the MusicBrainz database."); | ||
| } | ||
|
|
||
| emit resultAvailable( | ||
| std::move(pTrack), | ||
| std::move(guessedTrackReleases)); | ||
| std::move(guessedTrackReleases), | ||
| whyEmptyMessage); |
There was a problem hiding this comment.
the check is clunky and unnecessary from what I can tell. Can you just inline like you did with the other calls?
There was a problem hiding this comment.
so just
| QString whyEmptyMessage; | |
| if (guessedTrackReleases.empty()) { | |
| whyEmptyMessage = tr("Could not find this track in the MusicBrainz database."); | |
| } | |
| emit resultAvailable( | |
| std::move(pTrack), | |
| std::move(guessedTrackReleases)); | |
| std::move(guessedTrackReleases), | |
| whyEmptyMessage); | |
| emit resultAvailable( | |
| std::move(pTrack), | |
| std::move(guessedTrackReleases), | |
| tr("Could not find this track in the MusicBrainz database."); |
There was a problem hiding this comment.
if unconditionally looking up the translated string presents too much overhead, we can also introduce an enum and switch on it only in the failure case (in DlgTagFetcher::fetchTagFinished), thereby deferring the tr() call in exchange for a little extra boilerplate.
There was a problem hiding this comment.
This way we add the text even if we have a list of tracks. I think it is clearer to have to dedicated emmit calls.
There was a problem hiding this comment.
yes, one in case of success and a different one in case of error would also be better.
There was a problem hiding this comment.
I rather thought of two different signals (with differing parameters), but the current solution works too ig. not widening the scope too much.
|
Done |
Allow to use different messages in case Musikbrainz lookup ends without results.
This fixes #13666
Since this PR moves a translatable text form on object to another we need an Transifex cycle to not loose the translation in the stable 2.4 branch.