Improved the display artist name construction when tags are not set as expected, ref #513

src/libs/metadata/impl/Parser.cpp

@@ -297,11 +297,26 @@ namespace lms::metadata
         track.medium = getMedium(tagReader);
         track.artists = getArtists(tagReader, { TagType::Artists, TagType::Artist }, { TagType::ArtistSortOrder }, { TagType::MusicBrainzArtistID }, _artistTagDelimiters);
 
-        // We consider the artist display name is put in the Artist tag (picard case)
-        // But to please most users, if we find a custom delimiter in the Artist tag, we construct the artist diplay string with a "nicer" join
-        if (!_artistTagDelimiters.empty()
-            && track.artists.size() > 1
-            && getTagValuesAs<std::string>(tagReader, TagType::Artist, _artistTagDelimiters).size() > 1)
+        auto needReconstructArtistDisplayName{ [&] {
+            // We consider the artist display name is put in the Artist tag (picard case)
+
+            // To please most users, if we find a custom delimiter in the Artist tag, we construct the artist display string with a "nicer" join
+            if (!_artistTagDelimiters.empty()
+                && track.artists.size() > 1
+                && getTagValuesAs<std::string>(tagReader, TagType::Artist, _artistTagDelimiters).size() > 1)
+            {
+                return true;
+            }
+            // We have (true) multiple entries in the Artist tag or nothing
+            else if (getTagValuesAs<std::string>(tagReader, TagType::Artist, {}).size() != 1)
+            {
+                return true;
+            }
+
+            return false;
+        } };
+
+        if (needReconstructArtistDisplayName())
         {
             std::vector<std::string_view> artistNames;
             std::transform(std::cbegin(track.artists), std::cend(track.artists), std::back_inserter(artistNames), [](const Artist& artist) -> std::string_view { return artist.name; });

src/libs/metadata/test/Parser.cpp

@@ -252,4 +252,68 @@ namespace lms::metadata
         EXPECT_EQ(track->artists[1].name, "Other Artist");
         EXPECT_EQ(track->artistDisplayName, "This /  is ; One Artist, Other Artist"); // reconstruct artist display name since a custom delimiter is hit
     }
+
+    TEST(Parser, noArtistInArtist)
+    {
+        const TestTagReader testTags{
+            {
+                // nothing in Artist!
+            }
+        };
+
+        std::unique_ptr<Track> track{ Parser{}.parse(testTags) };
+
+        ASSERT_EQ(track->artists.size(), 0);
+        EXPECT_EQ(track->artistDisplayName, "");
+    }
+
+    TEST(Parser, singleArtistInArtist)
+    {
+        const TestTagReader testTags{
+            {
+                // nothing in Artist!
+                { TagType::Artists, { "Artist1" } },
+            }
+        };
+
+        std::unique_ptr<Track> track{ Parser{}.parse(testTags) };
+
+        ASSERT_EQ(track->artists.size(), 1);
+        EXPECT_EQ(track->artists[0].name, "Artist1");
+        EXPECT_EQ(track->artistDisplayName, "Artist1");
+    }
+
+    TEST(Parser, multipleArtistsInArtist)
+    {
+        const TestTagReader testTags{
+            {
+                // nothing in Artists!
+                { TagType::Artist, { "Artist1", "Artist2" } },
+            }
+        };
+
+        std::unique_ptr<Track> track{ Parser{}.parse(testTags) };
+
+        ASSERT_EQ(track->artists.size(), 2);
+        EXPECT_EQ(track->artists[0].name, "Artist1");
+        EXPECT_EQ(track->artists[1].name, "Artist2");
+        EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2"); // reconstruct artist display name since multiple entries are found
+    }
+
+    TEST(Parser, multipleArtistsInArtists)
+    {
+        const TestTagReader testTags{
+            {
+                // nothing in Artist!
+                { TagType::Artists, { "Artist1", "Artist2" } },
+            }
+        };
+
+        std::unique_ptr<Track> track{ Parser{}.parse(testTags) };
+
+        ASSERT_EQ(track->artists.size(), 2);
+        EXPECT_EQ(track->artists[0].name, "Artist1");
+        EXPECT_EQ(track->artists[1].name, "Artist2");
+        EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2"); // reconstruct artist display name since multiple entries are found and nothing is set in artist
+    }
 } // namespace lms::metadata