whipper-team / whipper

Python CD-DA ripper preferring accuracy over speed
GNU General Public License v3.0
1.14k stars 90 forks source link

[User error] Using slashes in disc template causes m3u, log, .cue to have no title [Reason: template misuse] #530

Closed Bujiraso closed 3 years ago

Bujiraso commented 3 years ago

Background

Initially I used the default disc template and got m3u's named Artist - Album.m3u (e.g. Linkin Park - Hybrid Theory.m3u).

I reworked my disc template in my local config to have artist based folders and then all of the m3u files since then have been named .m3u (no filename prefix, resulting in a hidden file)

Steps to reproduce

  1. In .config/whipper/whipper.conf, use the following values
    disc_template = %%A/%%y - %%d/
  2. Load a CD and $ whipper cd rip

Expected Output

m3u file, log file, cue file take the disc template (either the trailing portion after the final slash or converting all slashes to hyphens/underscores).

Happens every time.

e.g. To Pimp a Butterfly.m3u or Kendrick Lamar _ To Pimp a Butterfly.m3u

Actual Output

m3u file, log file, cue file are all without a leading name (suffix only)

-rw-r--r-- 1 bujiraso users 1.3K Mar  8 18:12  .cue
-rw-r--r-- 1 bujiraso users 8.8K Mar  8 18:12  .log
-rw-r--r-- 1 bujiraso users  676 Mar  8 18:12  .m3u

Resolution

The expectation is not correct since the template used is "artist/year - disc/" so the m3u should be "artist/year - disc/.m3u". If I wanted the full name there, I can use %%A/%%y - %%d/%%d. The extra %%d will put on the disc name again.

JoeLametta commented 3 years ago

Hi, this happens because of whipper's path handling in Python. Please keep in mind that the canonical representation of such a path doesn't include the trailing slash: add it has a special meaning (LINK). Maybe the current behaviour can be improved but it works as expected without the trailing slash.

Freso commented 3 years ago

It would probably be fine to do a .strip('/') on the templates when interpreting them.

Bujiraso commented 3 years ago

Thanks for the feedback but I believe I simply have trouble reading these naturally.

I was looking for this pattern: %%A/%%y - %%d/%%d

I didn't input that pattern though and surprised myself. I've updated the title to reflect the PEBCAK.

Bujiraso commented 3 years ago

Re @Freso , I've considered this for a bit and I think that adding the strip may not be best if someone did want these files to be neatly hidden.

I say leave it as is!

Maybe some docs to show common patterns and outputs can help people with tired eyes/minds like myself :sweat_smile:

JoeLametta commented 3 years ago

Re @Freso , I've considered this for a bit and I think that adding the strip may not be best if someone did want these files to be neatly hidden.

That seems like an ugly hack: shouldn't that be already possible adding a leading dot to the last component of the templates?

Bujiraso commented 3 years ago

Unless it's handled specially, since the dot is included as part of the file suffix, I expect you'd get ..cue / ..log