PICARD-2519: Allow passing supported URLs on command line

[skelly37]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences:

Currently Picard supports passing file paths on command line to open the respective files. Picard also supports handling some URL patterns by drag and drop, e.g. MusicBrainz recordings, releases, release group and cdtoc URLs.

Extend Picard to also handle these supported URLs when passed via command line.

Problem

• JIRA ticket (optional): PICARD-2519

Solution

call urllib.parse.urlparse on each argument marked as FILE to determine whether it's a file or URL, then pass it to the actual Tagger.

Action

[skelly37]

I think that handling the QObject::startTimer: Timers cannot be started from another thread will make it actually done.

[skelly37]

Okay, I've found out that picard.webservice.WebService.add_task is responsible for raising QObject::startTimer errors twice. Ref

Seems like the WebService was designed to be called by the main thread, while the pipe listener runs on another one, so it cannot start the actual task (and even breaks the currently working drag'n'drop feature). I mean, drag'n'drop works until we try to pass some URL via command-line to an existing instance.

[skelly37]

@zas @phw So basically I need help from the more proficient in Qt one of you

[rdswift]

At the risk of intoducing "scope creep", in addition to handling URLs on the command line, how about being able to just enter the MBID insead of the full URL? Of course, entering the full URL should work as well.

Basically the command line should probably accept the same inputs as the lookup text box: Album, Artist (?), Track. The only thing with just entering the MBID is that Picard won't know which type of record so it may have to try as many as three calls to the API to get a response. Perhaps the command line entry could (should) include a type identifier along with the MBID, like "Album:1fa3b874-dbef-4c86-9681-3f6d6877ddba"?

[rdswift]

Seems like the WebService was designed to be called by the main thread, while the pipe listener runs on another one, so it cannot start the actual task (and even breaks the currently working drag'n'drop feature).

I wonder if you could accomplish this using a signal? According to this article on setting handlers for asynchronous events, "Python signal handlers are always executed in the main Python thread of the main interpreter, even if the signal was received in another thread."

There are also a couple of threads on Stack Overflow (Python signals between threads and Emit signal in standard python thread) that might provide some insights. In particular, the the queue class looks like it might be promising.

I don't know if these are useful because I'm about the furthest thing from an expert on threaded processing. 😉

[phw]

I wonder if you could accomplish this using a signal?

Indeed that is probably the way, and Picard has a helper for that. You can use the to_main function from picard.util.thread to run a task on the main thread.

Album, Artist (?), Track. The only thing with just entering the MBID is that Picard won't know which type of record so it may have to try as many as three calls to the API to get a response. Perhaps the command line entry could (should) include a type identifier along with the MBID, like "Album:1fa3b874-dbef-4c86-9681-3f6d6877ddba"?

I would limit it to URLs for now, otherwise it can get confused with normal valid local paths. If we would introduce MBIDs, they should be with identify identifier in the form release:{mbid}, recording:{mbid} or artist:{mbid}. This is the short form of the full MB URLs, which is in this form already supported.

[skelly37]

I tend to agree with @phw on extending the current functionality to just URLs. The GUI automations we've discussed on IRC seem to be more useful than @rdswift's suggestion imo, especially because MBID isn't something that a user would enter like other CLI arguments.

[skelly37]

Also thanks for the decent feedback, gonna read the articles and mentioned methods :)

[rdswift]

I would limit it to URLs for now, otherwise it can get confused with normal valid local paths.

Okay. Perhaps somthing to consider in the future to better facilitate batch processing of an automated rip -> tag workflow.

If we would introduce MBIDs, they should be with identify identifier in the form release:{mbid}, recording:{mbid} or artist:{mbid}.

Agreed.

[rdswift]

especially because MBID isn't something that a user would enter like other CLI arguments.

Actually it easily could be part of an automated rip -> tag batch process workflow.

[zas]

I would limit it to URLs for now, otherwise it can get confused with normal valid local paths. If we would introduce MBIDs, they should be with identify identifier in the form release:{mbid}, recording:{mbid} or artist:{mbid}. This is the short form of the full MB URLs, which is in this form already supported.

What about allowing URLs in the form of:

mbid://<entity-type>/<mbid>

Examples:

mbid://release/dbd0ce67-cae6-33eb-8f5a-1143a30c2353
mbid://artist/83d91898-7763-47d7-b03b-b92132375c47

[skelly37]

I would limit it to URLs for now, otherwise it can get confused with normal valid local paths. If we would introduce MBIDs, they should be with identify identifier in the form release:{mbid}, recording:{mbid} or artist:{mbid}. This is the short form of the full MB URLs, which is in this form already supported.

What about allowing URLs in the form of:

mbid://<entity-type>/<mbid>

Hm, it could be easily parsed by urllib.parse.urlparse to separate MBID from HTTP(S). Worth considering.

[skelly37]

Okay, I've added the "mbid://" option, you might want to review the code now :)

[zas]

LGTM

[zas]

LGTM

[phw]

Works great, thanks a lot