Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1303: process DateItemField tags in YMD order #1589

Merged
merged 2 commits into from Sep 2, 2015

Conversation

jdetrey
Copy link
Contributor

@jdetrey jdetrey commented Sep 2, 2015

Hi,

This should fix bug #1303. Basically, this patch sorts the name of all metadata tags in (a somewhat tweaked) lexicographic order when they are processed by the MediaFile.update() method.

Only instances of DateItemField get sorted in the year < month < day order, so that updates to the corresponding DateField object are done in the proper order. It does so by replacing the substrings year, month, and day by date0, date1, and date2, respectively, in the field name of DateItemField objects, so that this new string can be used as a key for the sorting algorithm.

There are several other ways to do that, but I think that's the more robust. I'm of course open to discussion. 😃

Cheers,
Jérémie.

@sampsyo
Copy link
Member

sampsyo commented Sep 2, 2015

This looks like a good solution. I tried for a little while to cook up a neater sorting mechanism that wouldn't involve interpolating strings, but they all seem way more complicated than this. ✨ Thank you!

@sampsyo sampsyo merged commit 8b58af8 into beetbox:master Sep 2, 2015
sampsyo added a commit that referenced this pull request Sep 2, 2015
Fix #1303: process `DateItemField` tags in YMD order
sampsyo added a commit that referenced this pull request Sep 2, 2015
@jdetrey
Copy link
Contributor Author

jdetrey commented Sep 3, 2015

Great, thank you!

Cheers,
Jérémie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants