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

Junio C Hamano [off-list ref] writes:

Sergey Organov [off-list ref] writes:

quoted

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

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.

quoted

 	} 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