PICARD-3121: Allow user to set metadata processor plugin execution order

[rdswift]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences: Adds ability for the user to set the execution order of metadata processing plugins.

Problem

Currently each plugin is registered with a priority (1-100) which sets the order of execution. Most plugin authors (myself included) don't pay sufficient attention to this, which can result in plugins being executed in the wrong order, yielding undesired results.

This is the Picard v2.x version of the change related to PICARD-3104 which is marked for Picard v3 implementation.

I wasn't sure if we wanted to include this new functionality in the final release version of Picard 2.x, but even if we don't it can serve as a possible model for the Picard 3 implementation.

• JIRA ticket (optional): PICARD-3121

Solution

Provide a new dialog from the Options... > Plugins page to provide a list of the registered metadata processor plugins, which can be rearranged to determine the plugin execution order. The execution order is stored in a new user setting.

Modify the PluginFunctions class to use the new execution order setting.

Action

Additional actions required:

• Update Picard documentation (please include a reference to this PR) PICARD-3122
• Other (please specify below)

We need to determine if we want to include this setting in the user profiles. Currently we have no plugin settings as part of the option profiles, and I am reluctant to add this one.

[rdswift]

Here is a screen shot of the new dialog. Note that each line shows the plugin name, and a tooltip for each line shows the plugin description. Only installed metadata processing plugins are listed. Plugins can be reordered using the up and down buttons, or by drag 'n' drop.

[rdswift]

In the discussion on the ticket, it was suggested to also have a Reset button to restore the original order. Is this something that should be included?

EDIT: I could reproduce the original ordering for currently registered (enabled) plugins because the registration priority information is available. This information is not available for installed but currently disabled plugins. For the new v3 plugin system, this could be done for all plugins if the default priority was stored in the plugin's metadata as suggested in the ticket discussion.

[Sophist-UK]

1. There are different types of plugin registration, and some plugins register for multiple hooks perhaps with different priorities. I am unclear whether this means that the pop-up should have a dropdown with the different registration types and whether it should maintain a different order for each type. (And if we do have a dropdown to choose a type, then it should have options where there are no registered plugins greyed out.)

2. I would personally like to see the priority that the plugin registers with in a first column of this pop-up box.

3. I would also like to see a drag handle to the left or right of each row, allowing the user to drag a plugin to a different position as an alternative to using the up and down buttons.

4. I can't tell from a static image, but it should be possible to select multiple rows to move up and down together.

5. I can't tell from a static image, but if no rows are selected, then the up and down buttons should be disabled (greyed out).

6. Cover art plugins are plugins (some of which are built-in) and there is an existing options list for adjusting the order of these. Should this new pop-up replace this functionality - or should cover art plugins be excluded from the new functionality in this PR?

   

[rdswift]

1. There are different types of plugin registration, and some plugins register for multiple hooks perhaps with different priorities. I am unclear whether this means that the pop-up should have a dropdown with the different registration types and whether it should maintain a different order for each type. (And if we do have a dropdown to choose a type, then it should have options where there are no registered plugins greyed out.)

Yes, it's possible for the same plugin to register with both the album metadata processor and the track metadata processor, or even register multiple different methods for each, with different priorities. I thought that having one entry per plugin would simplify things for the user. I suppose I could have a separate line for each processor/module/method combination. That would allow the greatest amount of control over the processing order.

2. I would personally like to see the priority that the plugin registers with in a first column of this pop-up box.

I think having it in a separate box in the first column could be confusing. I would rather see it as an annotation such as "(Original Priority: xxx)" added to the end of the title.

3. I would also like to see a drag handle to the left or right of each row, allowing the user to drag a plugin to a different position as an alternative to using the up and down buttons.

As covered in the explanation, the user can click on a line and drag it to a new position. I specifically included that in the text at the top so that it was clear that the drag 'n' drop functionality was available.

4. I can't tell from a static image, but it should be possible to select multiple rows to move up and down together.

Not currently. I haven't looked into it, but I suspect that it would significantly complicate the code. For something that would be used so infrequently, I wasn't convinced that it would be worth implementing.

5. I can't tell from a static image, but if no rows are selected, then the up and down buttons should be disabled (greyed out).

They aren't currently because I didn't think it was necessary. I suppose it's possible if it's critically important.

6. Cover art plugins are plugins (some of which are built-in) and there is an existing options list for adjusting the order of these. Should this new pop-up replace this functionality - or should cover art plugins be excluded from the new functionality in this PR?

This new functionality is only for metadata processing plugins. Other plugins (including cover art plugins, scripting functions, context menus, etc.) are not included in this new functionality. Again, I tried to make this clear in the instruction text at the top of the dialog.

[rdswift]

Revised screen shot. With the additional information, I'm thinking this might be better presented in a table format, but I don't have any more time to work on it tonight (and I'm not sure I want to put a lot more effort into something that we might not even consider adding to the v2.x branch).

[rdswift]

New display using a table. Each column header has a tooltip to explain the contents of the column.

[rdswift]

I'm guessing that the checks failing to complete has something to do with the revised CI settings from the implementation of ruff?

[rdswift]

Yet another screen shot to show the latest changes. The "Move row" buttons now show as enabled or disabled as appropriate. I also added a label before the buttons to clarify their use.

I think this now addresses all of @Sophist-UK's comments/requests other than being able to select multiple lines to move together, and the (abbreviated) titles on the buttons.

[rdswift]

Playing around with the placement of the up/down arrows a bit in the dialog. What do you think of moving them to the right side of the table instead of having them on the bottom? It might be a bit more intuitive, even though it makes the dialog a bit larger horizontally. (I haven't committed this change.)

[phw]

I generally like this idea. But not sure about some details of the order dialog:

• I agree with sophist that it is a bit confusing if multiple processors are involved. It would probably be better to be able to configure the order per processor.
• Showing the numeric priority is not very helpful IMHO. I would remove this. While technically the order is an int, practically we only use HIGH=100, NORMAL=0, LOW=-100

Also I don't think the placement of the new button works well. The horizontal space there is already very limited. Shortening the texts makes the buttons less clear, breaks existing translations and makes it generally hard to translate them. This seems like a bad move to release as a version we actually don't want to maintain too much anymore.

Either we find another place for this or we better reserve this change for Picard 3 only. We need to redo the plugins dialog anyway.

[rdswift]

Either we find another place for this or we better reserve this change for Picard 3 only. We need to redo the plugins dialog anyway.

Okay, let's just leave this for Picard 3. I'll cancel the PR.

[rdswift]

Also I don't think the placement of the new button works well. The horizontal space there is already very limited. Shortening the texts makes the buttons less clear, breaks existing translations and makes it generally hard to translate them.

It could be placed on a new line below the existing buttons, and not change the text on the existing buttons. Something like:

[rdswift]

@phw,

I'm considering resurrecting this PR for consideration. Would you prefer to have the functionality available from the Plugins Options page with the button added below the current buttons (as shown in the previous message), or from a separate Options page under the "Advanced" section? Personally, I'm leaning toward the separate page under "Advanced" because it really is more of a niche advanced setting. That should also help eliminate any extra work of trying to incorporate this into a revised Plugins page for Picard 3.

My intent is to allow setting the order for each Plugin / Processor / Method Called combination, as shown in my most recent screen shot. I agree that displaying the numeric value of the originally registered plugin priority is not very helpful, and will remove this column (which will help reduce the screen real estate required).

Any other UI thoughts you'd like me to consider?

[phw]

I'm leaning toward the separate page under "Advanced" because it really is more of a niche advanced setting. That should also help eliminate any extra work of trying to incorporate this into a revised Plugins page for Picard 3.

@rdswift I agree with that. It has less impact on existing functionality.

Any other UI thoughts you'd like me to consider?

Can't think of something specific right now

[rdswift]

Okay, I'm force pushing the version with the functionality accessed from a new options page.

[rdswift]

Example screen shots of latest revision.

Hovering over the plugin name will display the plugin description in a tooltip. Hovering over the method/function called will display the method's docstring in a tooltip.

[rdswift]

Alternate layout with the up/down buttons to the right of the list of plugins. Is this more intuitive for the user (or looks better)?

The other option is to place the up/down buttons to the left, although this looks odd to me.

I guess I'm looking for your preferences for the final layout of the dialog:

1. Order of columns:
   a) Plugin Name, Processing Type, Method Name
   b) Plugin Name, Method Name, Processing Type
   c) Processing Type, Plugin Name, Method Name
   d) Other order

2. Up/Down button location:
   a) Below the list (left justified on the same line as the Ok and Cancel buttons)
   b) To the left of the list (vertically centered)
   c) To the right of the list (vertically centered)
   d) Other location

Note that I decided to make the actual reordering list a separate dialog that is opened from the option page. This is because it uses a lot more horizontal space than would typically be available in the option page itself. This also allowed adding a detailed explanation of the whole process on the option page.

Are there any other changes that you would like to see? Does this functionality and format make sense for the new plugin system in Picard 3? If we're going to add it to both the 2.x and 3.x versions, I think we should try to be consistent in the way it's presented to the user if possible. Thanks.

[Sophist-UK]

I like the column order and prefer the buttons on the right - but I am a single user so don't read too much into that.

Otherwise the UI looks good to me.

[phw]

Looks good to me. Let's have this in 2.14