Jeff King <peff@xxxxxxxx> writes:
> We already have the diff_from_contents flag which is used for
> --exit-code. We should be able to see where that logic is applied and do
> something similar. It looks like it happens in diff_flush(), which makes
> sense:
>
> if (output_format & DIFF_FORMAT_NO_OUTPUT &&
> options->flags.exit_with_status &&
> options->flags.diff_from_contents) {
> /*
> * run diff_flush_patch for the exit status. setting
> * options->file to /dev/null should be safe, because we
> * aren't supposed to produce any output anyway.
> */
> diff_free_file(options);
> options->file = xfopen("/dev/null", "w");
> options->close_file = 1;
> options->color_moved = 0;
> for (i = 0; i < q->nr; i++) {
> struct diff_filepair *p = q->queue[i];
> if (check_pair_status(p))
> diff_flush_patch(p, options);
> if (options->found_changes)
> break;
> }
> }
>
> So here's a naive application of the same technique:
>
> diff --git a/diff.c b/diff.c
> index 76291e238c..0fe6eb7443 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -6845,8 +6845,28 @@ void diff_flush(struct diff_options *options)
> DIFF_FORMAT_CHECKDIFF)) {
> for (i = 0; i < q->nr; i++) {
> struct diff_filepair *p = q->queue[i];
> - if (check_pair_status(p))
> - flush_one_pair(p, options);
> +
> + if (!check_pair_status(p))
> + continue;
> +
> + if (options->flags.diff_from_contents) {
> + FILE *orig_out = options->file;
> + int orig_changes = options->found_changes;
> + int skip;
> +
> + options->file = xfopen("/dev/null", "w");
> + diff_flush_patch(p, options);
> + skip = !options->found_changes;
> +
> + fclose(options->file);
> + options->file = orig_out;
> + options->found_changes = orig_changes;
> +
> + if (skip)
> + continue;
> + }
> +
> + flush_one_pair(p, options);
> }
> separator++;
> }
>
> which works on a trivial example. It affects all of raw, name-only,
> name-status, and checkdiff. I know Junio said that --raw should not be
> affected, but I'm not sure I agree. Anyway, it should be possible to
> split the logic by output type.
This is much more palatable to avoid code duplication and
repetition, compared to what was posted before. A new code that
loops over q->queue[] one more time (which is yucky but cannot be
helped due to output order as you noted earlier) and runs
diff_flush_patch(), like the one we see above, is probably the best
we can do. The beauty of the above approach is that the new code
only needs to know about a single .diff_from_contents bit, and does
not have to care what features cause the .diff_from_contents bit to
be flipped on. Doing anything more would be maintenance nightmare.
Thanks.
