Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exact mode, no-comparison-image mode & updated exit codes for diff-image #36

Open
xyzzy-plugh-plover opened this issue Sep 3, 2023 · 4 comments

Comments

@xyzzy-plugh-plover
Copy link

xyzzy-plugh-plover commented Sep 3, 2023

Would you accept a PR (or separate PRs) for diff-image that:

  1. adds an exact mode that only considers the image data the same if they are exactly the same. Is this already handled by -f 0? (i.e. does 0 fuzz require an exact match?). If not already handled by -f 0, this could be specified by -f x or -x (the latter should either be mutually exclusive with -f, or one should always override the other, or there should be a defined order precedence, like first argument is used or last argument is used)
  2. adds a "no comparison image" mode that does not generate a comparison file, but that will output a message to stdout if the image data for the files is different (and not output anything to stdout if they're the same). If this option is enabled along with "exact mode", then identify -format %# <file> could be run for each argument file & compared, which seems to be faster than the compare. If "exact mode" isn't specified, and if compare can output programmatic values other than equals/not equals (like "10% different", or "491 pixels different"), then that info could be included in the stdout message.
  3. updates the exit codes to be:
    0: both metadata & image data same
    1: metadata different, image data same
    2: metadata same, image data different
    3: both metadata & image data different
    -1: error (could have different negatives for different errors as negative exit codes count down from 255)
@xyzzy-plugh-plover xyzzy-plugh-plover changed the title Exit codes, exact mode & no-comparison-image mode for diff-image Exact mode & no-comparison-image mode for diff-image Sep 3, 2023
@xyzzy-plugh-plover xyzzy-plugh-plover changed the title Exact mode & no-comparison-image mode for diff-image Exact mode, no-comparison-image mode & updated exit codes for diff-image Sep 3, 2023
@ewanmellor
Copy link
Owner

@xyzzy-plugh-plover

  1. We use ImageMagick's compare tool for the actual comparison, so refer to that for details. We are not passing a -metric flag, so as far as I know we should be getting precise matches with -f 0.
  2. Any extra output modes you wanted to add would be fine with me.
  3. We would have to check to make sure that doesn't confuse git diff, but in principle I have no problem with that. The exit codes for this script haven't been defined up to now.

So yes, I'd be happy to take those patches. Are you sure you want to put them here though? It sounds like whatever you're doing might just be better working directly with ImageMagick. Is diff-image adding any value for you? It sounds like what you want is only a few lines of wrapper around ImageMagick compare or identify anyway.

@xyzzy-plugh-plover
Copy link
Author

xyzzy-plugh-plover commented Sep 4, 2023

I found your project when I was looking for various image comparison features. Given that I'm implementing them for myself, I figured it makes sense to share them so others can use them, too, and that it's better to have one good tool (yours) that provides all the functionality instead of 2 separate tools with slightly different capabilities.

I've made some other improvements while understanding your script which I'd be happy to also share. Many of them just simplify the code & simultaneously make it more performant, while some improve, in my mind, the stdout output; if you prefer the existing or other output, I can modify my changes to conform to your preference.

Do you want a bunch of small PRs, or could I just submit one PR, then you tell me if you want me to back out or modify any of my proposed changes? Let me know what works for you.

@ewanmellor
Copy link
Owner

Smaller PRs would be best, thanks. They're easier to review that way.

I don't mind adding extra features. The principles I'll be sticking to are:

  • git diff / git diff-image must keep working (I presume most users find the package because they're looking specifically for that).
  • Any new dependencies should be optional (I'm sure people appreciate the low dependency count right now).
  • Both macOS and Linux must keep working.

After that, I don't mind adding extra flags or output options or whatever.

@xyzzy-plugh-plover
Copy link
Author

Thanks. I'll open PRs one at a time starting with simple cleanup ones, then non-output changes, then output changes.

I don't have Linux to test, but I can test on macOS.

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

No branches or pull requests

2 participants