Fix the no_convert option of the convert plugin stopping conversion when there is only a partial match.

[sanpoChew]

Description

I was running the convert plugin with the following config:

no_convert: samplerate:..48000 bitdepth:..16

but anything that was 24/48 was also not being converted, this was due to the code returning False for should_transcode if any part of the query matches, rather than considering the whole query. This meant that bitdepth:...16 was being ignored.

I have changed this so the no_convert value is considered as one query.

To Do

• Documentation.
• Changelog.
• Tests.

[bal-e]

I'm looking at the documentation for no_convert:

Does not transcode items matching the query string provided (see Queries). For example, to not convert AAC or WMA formats, you can use format:AAC, format:WMA or path::\.(m4a|wma)$. If you only want to transcode WMA format, you can use a negative query, e.g., ^path::\.(wma)$, to not convert any other format except WMA.

I understand that the comma should work as a logical OR operator. @sanpoChew, can you confirm that format:AAC, format:WMA works as the documentation suggests, with your patches applied?

Also, this may break users' configurations, please add some information in the changelog so that users know how to adjust.

[Serene-Arc]

Thanks for the PR, and for catching this bug! This seems like a good fix.

[Serene-Arc]

Actually, just one request. Can you add a test that confirms that the comma OR functionality still works as expected with this change? Just as a test-coverage thing.

After that, if @bal-e is fine, this can be merged.

[sanpoChew]

I'm looking at the documentation for no_convert:

Does not transcode items matching the query string provided (see Queries). For example, to not convert AAC or WMA formats, you can use format:AAC, format:WMA or path::\.(m4a|wma)$. If you only want to transcode WMA format, you can use a negative query, e.g., ^path::\.(wma)$, to not convert any other format except WMA.

I understand that the comma should work as a logical OR operator. @sanpoChew, can you confirm that format:AAC, format:WMA works as the documentation suggests, with your patches applied?

Also, this may break users' configurations, please add some information in the changelog so that users know how to adjust.

That is correct yes, if users want to maintain the existing behaviour they would need to add a comma. I have updated the changelog entry so that it is more helpful 👍

[sanpoChew]

Actually, just one request. Can you add a test that confirms that the comma OR functionality still works as expected with this change? Just as a test-coverage thing.

After that, if @bal-e is fine, this can be merged.

That makes sense, have added that test 👍

[bal-e]

Sorry for the delay, @sanpoChew. LGTM!

[sanpoChew]

There seems to be some failures unrelated to my code in the lint checks

[snejus]

There seems to be some failures unrelated to my code in the lint checks

Seems like this was caused by GitHub failing DNS resolution internally. Have re-ran the workflows and it worked fine now.

[snejus]

Suggested something which should simplify testing

accident

[snejus]

Amazing stuff, looks super clean now! Thanks for catching this.

[sanpoChew]

fantastic! thanks a lot for the guidance all 👍