Skip to content

Improve parsing of artist/title from file names (+ tests)#2304

Merged
daschuer merged 4 commits intomixxxdj:masterfrom
uklotzde:parse_artist_title
Sep 29, 2019
Merged

Improve parsing of artist/title from file names (+ tests)#2304
daschuer merged 4 commits intomixxxdj:masterfrom
uklotzde:parse_artist_title

Conversation

@uklotzde
Copy link
Copy Markdown
Contributor

The "smart" parsing of artist and title from file names should only be triggered for tracks without a title. Splitting into artist and title should only be applied if both artist and title are empty to avoid inconsistencies.

  • Only parse track properties from file name if title is empty
  • An empty artist should not trigger the file name parser because some tracks might not have an artist, e.g. sample sounds
  • If the track already has an artist then the whole file name will be used as the title to avoid inconsistencies
  • Move the parsing function into TrackInfo and add extensive tests that verify the intended behavior
  • Preserve space characters in the file name and don't replace them by underscores in the resulting artist and title properties
  • Only accept " - " or "_-_" as artist/title separators and not just a single '-' hyphen character
  • Add an option (only in the code, no user setting) to prevent splitting into artist/title

Personally, I'm not a fan of this pseudo-smart parsing function and would simply copy the file name (without extension) into empty title fields, i.e. by setting splitArtistTitle = false independent of the artist field. But removing this function entirely might surprise some users, so let's keep and improve it slightly.

Some of the complaints (that this PR still won't fix as requested): https://www.mixxx.org/forums/viewtopic.php?f=3&t=12838

@uklotzde uklotzde added this to the 2.3.0 milestone Sep 28, 2019
Copy link
Copy Markdown
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking care. I have just a minor comment. The rest LGTM.

IMHO a better solution for the forum user would be to swap artist and title:

Stairway to Heaven 8'03 - Led Zeppelin (Led Zeppelin IV 1974 Atlantic)

but that is another story.

Comment thread src/track/trackinfo.cpp

namespace {

const QString kArtistTitleSeparator = "_-_";
Copy link
Copy Markdown
Member

@daschuer daschuer Sep 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define " - " as well? This will avoid the assumption that only "_-_" is supported.

@uklotzde
Copy link
Copy Markdown
Contributor Author

The discussion shows that this simplistic feature should have never been added. I had the pleasure to keep it alive during all the metadata rework and now once again. It solves a single, isolated use case and causes confusion for everybody else with a slightly different use case. I'm still convinced that simply using the file name (without the file extension that already becomes the file type) as the title would be the way to go, transparent and straight-forward.

@daschuer
Copy link
Copy Markdown
Member

Thank you. LGTM.
I am quite happy that this feature exists though, because it helps me sometimes, especially since we write the metadata back.

@daschuer daschuer merged commit 5998ee2 into mixxxdj:master Sep 29, 2019
@uklotzde uklotzde deleted the parse_artist_title branch September 29, 2019 07:40
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.

3 participants