Merge lp:~openshift/picard/commlang into lp:~musicbrainz-developers/picard/trunk

Proposed by Carlin Mangar
Status: Needs review
Proposed branch: lp:~openshift/picard/commlang
Merge into: lp:~musicbrainz-developers/picard/trunk
Diff against target: 30 lines
1 file modified
picard/formats/id3.py (+7/-2)
To merge this branch: bzr merge lp:~openshift/picard/commlang
Reviewer Review Type Date Requested Status
Philipp Wolfer Abstain
Review via email: mp+14217@code.launchpad.net
To post a comment you must log in.
lp:~openshift/picard/commlang updated
1006. By Philipp Wolfer

Fixed loading of remaster ARs and release events with empty date in original release date plugin.

1007. By Philipp Wolfer

Fixed deletion of all COMM frames in ID3, which was introduced in rev. 989

1008. By Philipp Wolfer

Release 0.12.1

1009. By Carlin Mangar

Preserves language of comments while retaining backward compat.

Revision history for this message
Philipp Wolfer (phw) wrote :

This is now mostly backward compatible, although

I like the functionality and I have no better idea to implement this cleanly. I don't believe there are many (any?) plugins out there which read the comments in this way. Lastfmplus writes comments, but that would not break.

The only thing about the patch is that we maybe should set the language to "eng" by default instead of not setting it at all.

Lukáš, what do you think?

review: Abstain
Revision history for this message
Carlin Mangar (openshift) wrote :

Personally, this is better, some of my files have different types of comments, some with eng, some without. It's not in the specification to force eng on the comment, so I wouldn't want Picard to be cleaning up stuff unnecessarily. We could have an option for that if users want it, but is it ok to force eng on every comment that has no language set? I think we could work on some way to set the default, but I prefer compatibility over setting everything.

Unmerged revisions

1009. By Carlin Mangar

Preserves language of comments while retaining backward compat.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'picard/formats/id3.py'
2--- picard/formats/id3.py 2009-11-01 17:49:45 +0000
3+++ picard/formats/id3.py 2009-11-02 10:11:15 +0000
4@@ -146,6 +146,9 @@
5 elif frameid == 'COMM':
6 for text in frame.text:
7 if text:
8+ if frame.lang:
9+ metadata.add('%s:%s:%s' % (name, frame.desc, frame.lang), unicode(text))
10+ else:
11 metadata.add('%s:%s' % (name, frame.desc), unicode(text))
12 else:
13 metadata.add(name, unicode(frame))
14@@ -242,12 +245,14 @@
15 for value in values:
16 tmcl.people.append([role, value])
17 elif name.startswith('comment:'):
18- desc = name.split(':', 1)[1]
19+ lang = None
20+ try: comm, desc, lang = name.split(':')
21+ except ValueError: desc = name.split(':',1)[1]
22 if desc.lower()[:4]=="itun":
23 tags.delall('COMM:' + desc)
24 tags.add(id3.COMM(encoding=0, desc=desc, lang='eng', text=[v+u'\x00' for v in values]))
25 else:
26- tags.add(id3.COMM(encoding=encoding, desc=desc, lang='eng', text=values))
27+ tags.add(id3.COMM(encoding=encoding, desc=desc, lang=lang, text=values))
28 elif name.startswith('lyrics:') or name == 'lyrics':
29 if ':' in name:
30 desc = name.split(':', 1)[1]

Subscribers

People subscribed via source and target branches

to status/vote changes: