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

Display release country in matching releases #451

Closed
the-confessor opened this issue Jan 16, 2020 · 12 comments
Closed

Display release country in matching releases #451

the-confessor opened this issue Jan 16, 2020 · 12 comments
Assignees
Labels
Accepted Accepted issue on our roadmap Feature New feature Priority: low Low priority Sprintable Small enough to sprint on Stretch Optional goal for the current sprint (may not be delivered)
Milestone

Comments

@the-confessor
Copy link

When I use whipper and there are multiple matching releases I like it to prompt me to confirm which one to use.

I usually find myself opening each musicbrainz URL just to look at the country for each of them, and that is how I choose.

I could avoid having to do this in most cases if the country was included in the ouput shown to me by whipper. Is it possible to add?

@ABCbum
Copy link
Contributor

ABCbum commented Jan 16, 2020

I think this one is somewhat similar to #167 right ?

Edit:

When I use whipper and there are multiple matching releases I like it to prompt me to confirm which one to use.

Also there is a --prompt option for this i guess here

@the-confessor
Copy link
Author

the-confessor commented Jan 17, 2020

Correct - my statement would have been better said "I am using the --prompt option so I can select the disc when there is more than one match."

#167 sounds like some enhancements related to the interactivity; I am happy with the current controls for interactivity, I would just like to see the country field presented to me when there are multiple matches.

e.g. this is what I saw for a CD I recently started to rip:

MusicBrainz disc id TW8p_wRBO1mXLKCI_H2HX873Qtk-
MusicBrainz lookup URL https://musicbrainz.org/cdtoc/attach?toc=1+10+187755+150+19597+34725+53272+72175+86125+102867+119312+138572+159210&tracks=10&id=TW8p_wRBO1mXLKCI_H2HX873Qtk-
Disc duration: 00:41:41.400, 10 audio tracks

Matching releases:

Artist  : Tilly and the Wall
Title   : Bottoms of Barrels
Duration: 00:41:52.000
URL     : https://musicbrainz.org/release/f4e74e9e-e58a-4e6e-ad94-d9db0de7e510
Release : f4e74e9e-e58a-4e6e-ad94-d9db0de7e510
Type    : Album
Cat no  : TL 09

Artist  : Tilly and the Wall
Title   : Bottoms of Barrels
Duration: 00:41:52.000
URL     : https://musicbrainz.org/release/6e515011-8e1e-3042-994d-9fdf50923fc1
Release : 6e515011-8e1e-3042-994d-9fdf50923fc1
Type    : Album

Please select a release [f4e74e9e-e58a-4e6e-ad94-d9db0de7e510]: 

This is what I would like to see:

MusicBrainz disc id TW8p_wRBO1mXLKCI_H2HX873Qtk-
MusicBrainz lookup URL https://musicbrainz.org/cdtoc/attach?toc=1+10+187755+150+19597+34725+53272+72175+86125+102867+119312+138572+159210&tracks=10&id=TW8p_wRBO1mXLKCI_H2HX873Qtk-
Disc duration: 00:41:41.400, 10 audio tracks

Matching releases:

Artist  : Tilly and the Wall
Title   : Bottoms of Barrels
Duration: 00:41:52.000
URL     : https://musicbrainz.org/release/f4e74e9e-e58a-4e6e-ad94-d9db0de7e510
Release : f4e74e9e-e58a-4e6e-ad94-d9db0de7e510
Type    : Album
Country : US
Cat no  : TL 09

Artist  : Tilly and the Wall
Title   : Bottoms of Barrels
Duration: 00:41:52.000
URL     : https://musicbrainz.org/release/6e515011-8e1e-3042-994d-9fdf50923fc1
Release : 6e515011-8e1e-3042-994d-9fdf50923fc1
Type    : Album
Country : Australia

Please select a release [f4e74e9e-e58a-4e6e-ad94-d9db0de7e510]: 

@ABCbum
Copy link
Contributor

ABCbum commented Jan 17, 2020

Ahh i get what you mean now. If that's the case how about using the --country flag (i guess you did so let me check if the code is doing what you are suggesting). ( ╹▽╹ )

Edit: I checked and it seems to work fine (here).

@the-confessor
Copy link
Author

It's slightly different. The --country flag will filter that matches by country. I would like to show matches from all countries, but include the country information in the whipper output.

Use of the --country flag would require me to think about it before I put the CD in. Including country information in the output would mean I don't need to think about it unless there happens to be multiple matches.

@ABCbum
Copy link
Contributor

ABCbum commented Jan 17, 2020

Yeah you are right. Now it depends on what others and maintainers think about this feature (I think it should be fine). What do you guys think, @JoeLametta , @Freso , @MerlijnWajer ?

@JoeLametta
Copy link
Collaborator

JoeLametta commented Jan 17, 2020

I think this is a worthwhile improvement.

JoeLametta added a commit that referenced this issue Jan 29, 2020
Untested.

Fixes #451.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Copy link
Collaborator

JoeLametta commented Jan 29, 2020

@the-confessor Hi, i've added the feature you've requested in commit 85ccd29 (branch feature/issue-451-display-country). I haven't tested it so, if you can, please try it and let me know if it works as expected.

Cheers,
Joe

@JoeLametta JoeLametta added Accepted Accepted issue on our roadmap Feature New feature Priority: low Low priority Sprintable Small enough to sprint on Stretch Optional goal for the current sprint (may not be delivered) labels Jan 29, 2020
@JoeLametta JoeLametta added this to the 1.0 milestone Jan 29, 2020
@JoeLametta JoeLametta self-assigned this Jan 29, 2020
JoeLametta added a commit that referenced this issue Jan 29, 2020
Untested.

Fixes #451.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Copy link
Collaborator

JoeLametta commented Jan 29, 2020

@the-confessor My bad: I made a mistake. The correct commit to test is 6fab8e1.

EDIT: commit identifier updated.

@JoeLametta JoeLametta added the Status: in progress Issue/pull request which is currently being worked on label Jan 29, 2020
@JoeLametta
Copy link
Collaborator

Related to #380.

@the-confessor
Copy link
Author

I applied as a patch and it seems to do what I was expecting.

Incidentally, the first two CDs I tested with revealed a release can have multiple countries. Examples follow. Ideally in such cases it would list all the countries.

https://musicbrainz.org/release/aef7bf54-2081-4151-96e8-07b612704878
https://musicbrainz.org/release/4c55906c-349b-362d-922e-956762912b42

@JoeLametta
Copy link
Collaborator

I applied as a patch and it seems to do what I was expecting.

Good.

Incidentally, the first two CDs I tested with revealed a release can have multiple countries.

@the-confessor I've just pushed a new commit (af74f31) which should make whipper report all available countries (if available). I haven't tested it so, if you can, please try it and let me know if it works as expected.

Cheers,
Joe

@the-confessor
Copy link
Author

I applied 6fab8e1 + af74f31 as a patch and confirm it works, it shows me all countries.

@JoeLametta JoeLametta removed the Status: in progress Issue/pull request which is currently being worked on label Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature Priority: low Low priority Sprintable Small enough to sprint on Stretch Optional goal for the current sprint (may not be delivered)
Projects
None yet
Development

No branches or pull requests

3 participants