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

> 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.