GNOME Bugzilla – Bug 730001
Support arrangers and multiple composers
Last modified: 2014-08-15 13:31:14 UTC
MusicBrainz allows more than one composer per work and also arrangers, use all the information available when setting the composer tag.
Created attachment 276371 [details] [review] Add function to format multiple composers. Given separate lists of composers, arrangers, orchestrators and transcribers format the composer tag appropriately. This paves the way for supporting multiple composers and arrangers.
Created attachment 276372 [details] [review] Support arrangers & multiple composers. Musicbrainz works can have more than one composer as well as arrangers, orchestrators and transcribers. Fetch the relevant work relations to get this information and format it in the composer text.
Created attachment 276397 [details] [review] Add function to format multiple composers. Corrected spelling mistake. Given separate lists of composers, arrangers, orchestrators and transcribers format the composer tag appropriately. This paves the way for supporting multiple composers and arrangers.
Created attachment 276398 [details] [review] Support arrangers & multiple composers. Musicbrainz works can have more than one composer as well as arrangers, orchestrators and transcribers. Fetch the relevant work relations to get this information and format it in the composer text.
Review of attachment 276397 [details] [review]: ::: libjuicer/sj-metadata-musicbrainz5.c @@ +445,3 @@ + } + + return g_string_free (text, FALSE); I don't think ", %s" should be translateable. If you generate a GPtrArray of composers, arrangers, ... concatenate_composers () would become a simple call to g_strjoinv @@ +564,3 @@ + g_free (orch); + g_free (trans); +} Some somehow crazy thought for potentially making this function shorter (not necessarily simpler). We could have an array of 16 elements containing the format string to use, and build an index into the array using bitflags. This way the code becomes: if (composer != NULL) { index |= 1 << 0; (or rather a constant whose value is 1 << 1) } if (arranger != NULL) { index |= 1 << 1 } ... -> we get a format string Then we can concatenate the arr, orch and trans strings making sure they are separated with a ',' and then we can check if we have 0, 1 or 2 non-NULL args and call g_strdup_printf with that. More arcane than this function, but maybe shorter and a bit easier to maintain.
(In reply to comment #5) > Review of attachment 276397 [details] [review]: > > ::: libjuicer/sj-metadata-musicbrainz5.c > @@ +445,3 @@ > + } > + > + return g_string_free (text, FALSE); > > I don't think ", %s" should be translateable. If you generate a GPtrArray of > composers, arrangers, ... concatenate_composers () would become a simple call > to g_strjoinv At the moment the list contains ArtistDetails not just the artist name so I can get the sortname as well. I guess I could have the sortname and an array of names but having a list of ArtistDetails seemed more straight forward. I can get rid of the translation of ", %s" - what do you think? > > @@ +564,3 @@ > + g_free (orch); > + g_free (trans); > +} > > Some somehow crazy thought for potentially making this function shorter (not > necessarily simpler). > We could have an array of 16 elements containing the format string to use, and > build an index into the array using bitflags. > This way the code becomes: > if (composer != NULL) { > index |= 1 << 0; (or rather a constant whose value is 1 << 1) > } > if (arranger != NULL) { > index |= 1 << 1 > } > ... > -> we get a format string > > Then we can concatenate the arr, orch and trans strings making sure they are > separated with a ',' and then we can check if we have 0, 1 or 2 non-NULL args > and call g_strdup_printf with that. > > More arcane than this function, but maybe shorter and a bit easier to maintain. Sounds interesting - I'll put something along these lines together
Created attachment 277264 [details] [review] Add function to format multiple composers. I've reworked build_composer_text as you suggested. In concatenate composers I've removed the translation for ", %s" but I'm still using a list of ArtistDetails rather than a GPtrArray of names as we need the sortname as well. Given separate lists of composers, arrangers, orchestrators and transcribers format the composer tag appropriately. This paves the way for supporting multiple composers and arrangers.
Created attachment 277265 [details] [review] Support arrangers & multiple composers. This is unchanged, I'm re-posting it to keep the patches in the right order. Musicbrainz works can have more than one composer as well as arrangers, orchestrators and transcribers. Fetch the relevant work relations to get this information and format it in the composer text.
Attachment 277264 [details] pushed as 422fb7d - Add function to format multiple composers. Attachment 277265 [details] pushed as 1121403 - Support arrangers & multiple composers.