Junio C Hamano <gitster@xxxxxxxxx> writes:
> Sergey Organov <sorganov@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>
> 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.
Fine with me, thanks! I'll re-roll soon.
Thanks,
-- Sergey
