Re: [PATCH v3] revision: add separate field for "-m" of "diff-index -m"

[Junio C Hamano <gitster@pobox.com>]

Sergey Organov <sorganov@gmail.com> writes:

> Add separate 'diff_index_match_missing' field for diff-index to use and set it
> when we encounter "-m" option. This field won't then be cleared when another
> meaning of "-m" is reverted (e.g., by "--no-diff-merges"), nor it will be
> affected by future option(s) that might drive 'ignore_merges' field.
>
> Use this new field from diff-lib:do_oneway_diff() instead of reusing
> 'ignore_merges' field.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---

Much easier to reason about.  As I said, I think we would ideally
want to detect and diagnose --[no-]diff-merges on the command line
of "diff" or "diff-{files,index,trees}" as an error, but for now
this is a good first step.

>  	} else if (!strcmp(arg, "-m")) {
>  		revs->ignore_merges = 0;
> +		/*
> +		 * Backward compatibility wart - "diff-index -m" does
> +		 * not mean "do not ignore merges", but "match_missing",
> +		 * so set separate flag for it.
> +		 */
> +		revs->diff_index_match_missing = 1;

Half the wart has been removed thanks to this patch and the rest of
the code can look at the right field for their purpose.  The parsing,
unless we make a bigger change that allows us to detect and diagnose
"diff-index --no-diff-merges" as an error, still needs to be tricky
and may deserve a comment.

The comment should apply to and treat both fields equally, perhaps
like this:

	} else if (!strcmp(arg, "-m")) {
		/*
		 * To "diff-index", "-m" means "match missing", and to
		 * the "log" family of commands, it means "keep merges".
		 * Set both fields appropriately.
		 */
		revs->ignore_merges = 0;
		revs->match_missing = 1;
	}

By the way, let's drop diff_index_ prefix from the name of the new
field.  I do not see a strong reason to object to a possible update
to "diff-files" to match the behaviour of "diff-index".  

In a sparsely checked out working tree (e.g. start from "clone
--no-checkout"), you can check out only paths that you want to
modify, edit them, and then "git diff-files -m" would be able to
show useful result without having to show deletions to all other
files you are not interested in.  And that is exactly the same use
case as "git diff-index -m HEAD" was invented for.

Thanks.

> diff --git a/revision.h b/revision.h
> index c1e5bcf139d7..5ae8254ffaed 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -188,6 +188,7 @@ struct rev_info {
>  	unsigned int	diff:1,
>  			full_diff:1,
>  			show_root_diff:1,
> +			diff_index_match_missing:1,
>  			no_commit_id:1,
>  			verbose_header:1,
>  			combine_merges:1,