Re: diff-index --cc no longer permitted, gitk is now broken (slightly)

[Sergey Organov <sorganov@gmail.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Here is a patch that fixes diff-index to accept --cc again:
>
> Sorry for the delay; I did not notice there was a patch buried in a
> discussion thread.
>
> We might later need to do this suppression in more codepaths if we
> find more regressions, but let's have one fix at a time.

I'm pretty positive there should be nothing left. This commit was
diff-index specific, and doesn't affect anything else. Nowhere in entire
series the semantics of --cc itself has been changed, it has been only
disabled as particular option in diff-index command-line parsing.
Overall, this is pretty local change.

>
> Will queue.
>
>>  builtin/diff-index.c |  6 +++---
>>  diff-merges.c        | 14 ++++----------
>>  diff-merges.h        |  2 +-
>
> This would deserve new tests that cover the existing use cases,
> given that both of us (and other reviewers in the original thread)
> did not notice how big a regression we are causing.

I don't see it as a "big regression", but no wonder the breakage was
entirely unexpected, see below.

>
> We care about --cc naturally falling back to -p when there is only
> one other thing to compare with, and also we care about --cc that
> allows us to compare during conflict resolution, at least, I think.

The problem here is not with -c/--cc itself, it is rather with
diff-index. It's neither documented nor tested nor obvious what -c/--cc
should mean in diff-index, given -c/--cc description (e.g., in "git help
log"):

       -c
           With this option, diff output for a merge commit shows the
           differences [...]

How an option to deal with merge commits is applicable to diff-index,
that:

    git-diff-index - Compare a tree to the working tree or index

???

Besides, nobody yet told us why gitk uses --cc option in invocation of
'diff-index' in the first place. Does it actually *rely* on particular
undocumented behavior of "diff-index --cc", or is it just a copy-paste
*leftover*?

Overall, the original commit had a mistake, as the commit that was meant
to be pure refactoring had changed observable behavior, even if
undocumented. However, the essence of the commit, disabling of "diff for
merge /commits/" options in diff-index that does not deal with /commits/,
could still be the right thing to do long term.

Thanks,
-- Sergey Organov