search

whipper-team / whipper

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

Possible improvements in accurip subcommand #563

Open arcctgx opened 2 years ago

arcctgx commented 2 years ago

After I submitted PR #562 I started wondering: what is the reason to require the user to specify a full URL as argument to whipper accurip show, since the initial part is stripped anyway?

Additionally, it is not clear at all now what kind of URL is necessary. I had to read the source code to figure it out. :slightly_smiling_face:

Wouldn't it make more sense to only require the ID, like dBAR-006-000aadee-003920f8-4d0a3d06, on the command line? This ID contains all the information necessary to build request URL, and to perform cache lookup.

I would suggest to do the following:

  1. Rename the mandatory argument of whipper accurip show from url to id or something similar.
  2. Improve help message so that it shows an example ID.
  3. Implement verification to check if the ID given by the user is valid.
  4. Build the AccurateRip lookup URL and path for cache lookup from the ID.

What do you think?

github-actions[bot] commented 2 years ago

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

To help make it easier for us to investigate your issue, please follow the contributing instructions.