Re: [PATCH] diff: report modified binary files as changes in builtin_diff()
Am 21.09.2024 um 17:09 schrieb René Scharfe:
Hi René,
Show 12 quoted lines
> diff --git a/diff.c b/diff.c
> index 3be927b073..84a6bb0868 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3675,6 +3675,7 @@ static void builtin_diff(const char *name_a,
> emit_diff_symbol(o, DIFF_SYMBOL_BINARY_FILES,
> sb.buf, sb.len, 0);
> strbuf_release(&sb);
> + o->found_changes = 1;
> goto free_ab_and_return;
> }
> if (fill_mmfile(o->repo, &mf1, one) < 0 ||
I poked at the same issue in parallel and had the same fix, but ...
Show 19 quoted lines
> diff --git a/t/t4017-diff-retval.sh b/t/t4017-diff-retval.sh
> index d644310e22..1cea73ef5a 100755
> --- a/t/t4017-diff-retval.sh
> +++ b/t/t4017-diff-retval.sh
> @@ -145,6 +145,14 @@ test_expect_success 'option errors are not confused by --exit-code' '
>
> for option in --exit-code --quiet
> do
> + test_expect_success "git diff $option returns 1 for changed binary file" "
> + test_when_finished 'rm -f .gitattributes' &&
> + git reset --hard &&
> + echo a binary >.gitattributes &&
> + echo 2 >>a &&
> + test_expect_code 1 git diff $option
> + "
> +
> test_expect_success "git diff $option returns 1 for copied file" "
> git reset --hard &&
> cp a copy &&
your test is nicer.
The patch works here locally.
For what it's worth:
Reviewed-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
Thomas