Skip to content

Conversation

@Oondanomala
Copy link

Allows setting a custom ExifTool config file.
Because the -config option has to be the first one specified, it required modifying the strategies directly.

The optimal solution would have been to add configPath as a parameter for execute() just like whats done for the ExifTool path, but that would have broken backwards compatibility, so instead I added a setConfigFilePath() method to ExecutionStrategy.

Closes #247

@mjeanroy
Copy link
Owner

Why the config option cannot be added to the StandardOptions?
Then, we would "just" need to handle it first in the serialize method?

@Oondanomala
Copy link
Author

Because the strategies always add -sep before the given arguments and -config must be the first option.
And it cant add that after since then it would be after -execute.
I guess each strategy could manually insert -sep in between the arguments, but that would be worse IMO.

@Oondanomala
Copy link
Author

It might be better to make ExifTool.toArguments add -sep instead,
BUT to keep backwards compatibility getRawExifToolOutput would have to be modified to add -sep to the argument list,
which would in turn break compatibility for programs that subclass ExecutionStrategy, make it not add -sep, and use getRawExifToolOutput, expecting it to not have -sep as an argument.
Also it would be a compatibility breakage for every ExecutionStrategy subclass since they would now get an unexpected -sep argument, but that might not be as big of a deal as AFAIK duplicate -sep arguments get ignored.

Honestly the whole api is very inconsistent (like #205), but sadly it cant be cleaned up without a compatibility breakage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for advanced options

2 participants