search

metabrainz / picard-plugins

Picard plugins: use 1.0 branch for Picard < 2.0 (python 2/Qt4) and 2.0 branch for Picard >= 2.0 (python 3/Qt5)
https://picard.musicbrainz.org/plugins/
145 stars 95 forks source link

PICARD-2743: Add support for custom post tagging actions #374

Closed twodoorcoupe closed 7 months ago

twodoorcoupe commented 8 months ago

This plugins tries to solve PICARD-2743. It adds the ability to specify "actions" that run with a context menu click. Basically, it can run external programs. Besides what was initially proposed, I took inspiration from what other taggers offer, like mp3tag tools.

The commands can be set in the plugin's option page, the UI looks like this:

ui

The commands run in the order they appear in the table. It lets you choose:

Also, it lets you specify variables to use in the command. These are taken either from the metadata like %title% or %artist%, or from extra "special" variables like %filepath% or %is_complete%.

I uploaded a user guide here where everything is hopefully explained more thoroughly, it includes a list of all the special variables as well. There's a link to it both in the options page and in the plugin's metadata.

twodoorcoupe commented 8 months ago

Thank you @Sophist-UK! I made the number of worker threads an option and made it so that the number of pending requests is displayed in the status bar like you asked.

Sophist-UK commented 8 months ago

@twodoorcoupe Giorgio - We are getting pretty close as far as I am concerned. Keep going....

twodoorcoupe commented 8 months ago

@Sophist-UK, I tried using the Pending files indicator in the status bar, but I ran into a few issues with concurrency. For example: refreshing the album, saving the files, or running acoustid scans would all change the files' states.

Therefore, I opted for adding another indicator in the status bar to keep track of the pending actions. It look like this:

pending_actions

It is only visible when there are actions pending, otherwise it stays hidden. The icon is the same as the one used for Plugins and Run scripts in the context menu.

I think this solution is better since, besides avoiding issues with other functions, it also makes the meaning of the indicator more clear.

twodoorcoupe commented 8 months ago

Thanks @phw! I saw your comment a minute too late. I changed it now.

Sophist-UK commented 8 months ago

Therefore, I opted for adding another indicator in the status bar to keep track of the pending actions. It is only visible when there are actions pending, otherwise it stays hidden. The icon is the same as the one used for Plugins and Run scripts in the context menu.

I think this solution is better since, besides avoiding issues with other functions, it also makes the meaning of the indicator more clear.

I think this is a great solution.

Does this new status icon have a tooltip? If not can you add one please.

twodoorcoupe commented 8 months ago

Does this new status icon have a tooltip? If not can you add one please.

Yes, it says "Pending actions", keeping the "style" of the other icons' tooltips.

Sophist-UK commented 8 months ago

Great! As far as my desk review goes, this seems like it is good to go.

twodoorcoupe commented 8 months ago

Thanks for testing it @phw. I've been trying to reproduce the issue you are having, but I'm out of luck. I've tried with both Picard 2.10 and 2.11 on both windows and linux, and for me it works out of the box. So I wanted to ask you, does the icon never show up at all? Even after you restart Picard? I ask this because the widget is instantiated differently whether the plugin is loaded before or after the main window is created.

phw commented 8 months ago

@twodoorcoupe Found the issue, not yet sure how to fix. This happens if the plugin is already installed, like it was for me, since I checked out your branch and placed a symlink in ~/.config/MusicBrainz/Picard/plugins. On the run where you load Picard with the plugin disabled but then enable and use it the UI widget does not get set up properly.

That's because registering all the hooks happens when Picard is started and loads the plugins, regardless on whether the plugin is enabled or not. That means the code where it decided whether to initialize the widget immediately or use the register_ui_init hook (https://github.com/metabrainz/picard-plugins/pull/374/files#diff-dff6ecca533b5cccfc28ffd3c93229fc405c7f00c781d451b1c7766dfa8f6713R194-R198) it uses the UI init hook. But because the plugin is disabled, it does not actually run. And this hook only runs on start, not later if a plugin actually gets enabled (which essentially implies the use of register_ui_init always requires a restart).

The register_ui_init is kind of a kludge really, I'm not too happy about this.

But for now I think the best solution would be to live with the fact that showing the status widget requires a restart and just make sure that the call tagger.register_cleanup(self.stop) only happens after self.update_widget.start() got called in _create_widget. That will ensure there is no crash on exit.

It's all really a limitation of the plugin system currently, something we want to improve with Picard 3 by having plugins provide explicit hooks for lifecycle events like "enable" and not running anything if a plugin is disabled (more on that soon).

twodoorcoupe commented 8 months ago

Thank you so much for finding the issue @phw! I guess I have the habit of always enabling plugins as soon as I install them, so I would have never figured it out without your help.