PICARD-3133: Adds a file presave hook

[frcooper]

Summary

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

Problem

I may want to have multiple copies of a release i.e. a flac, a low compression and high compression copy for different uses. I want to be able to set my own disambiguation patterns instead of rely on the built-in (n) routine.

The post save hook is too late - the file has already moved, and nothing preserves that information.

https://picard-docs.musicbrainz.org/en/appendices/plugins_api.html#file-post-save-processor-file exists, but no pre-save equivalent does.

See ticket PICARD-3133.

Solution

With a pre-save hook, I can capture file collisions and handle them as part of the file naming script. So I created one.

Action

Additional actions required:

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

[zas]

Please search (or create) a matching ticket (and prefix title with the ticket number, PICARD-XXXX: )
Ensure tests are passing (especially ruff format)

[phw]

Code looks clean, thanks.

My issue with this is that such a hook can modify the metadata and hence something different will be saved then originally shown in the UI.

There might be some use cases for this, but we should be extremely careful with plugins making use of this hook. There must be a real need for what the plugin does. If the result is doable by other hooks those must be preferred.

I think most use cases, including the one in your ticket about different format metadata, can be handled by file_post_addition_to_track_processor.

A real use case is the sidecar files I think.

[frcooper]

@phw If my use case could be handled by those other hooks, I wouldnt have written this.

My issue with this is that such a hook can modify the metadata and hence something different will be saved then originally shown in the UI.

This is already what happens when it writes the files with a (n) extension. My hook is explicitly to catch that and give control to the user. If the UI actually previewed the metadata and file state before the file was written, there'd be no issue because you could in fact alert the user to the preexisting files. However Picard is blind to the state of anything not in memory, and doesnt even update those states until they are commited during a save.

If you don't like plugins that use the hook, don't use them. That has no bearing on the existence of the hook itself.

[phw]

If you don't like plugins that use the hook, don't use them. That has no bearing on the existence of the hook itself.

All I was saying that I think we should be very careful reviewing plugins that make use of the hook :) And specifically changing metadata is something that (official) plugins likely should not do in this hook.

[frcooper]

I've reread the conversation. I think that my initial choice of the term metadata when I really meant file state has caused unneeded confusion. I apologize for that. The comments in the code could have been better.