From: Sergey Organov <sorganov@gmail.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
Paul Mackerras <paulus@ozlabs.org>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: diff-index --cc no longer permitted, gitk is now broken (slightly)
Date: Wed, 08 Sep 2021 22:04:46 +0300 [thread overview]
Message-ID: <87pmtiopm9.fsf@osv.gnss.ru> (raw)
In-Reply-To: <cbd0d173-ef17-576b-ab7a-465d42c82265@kdbg.org> (Johannes Sixt's message of "Wed, 8 Sep 2021 19:23:52 +0200")
Johannes Sixt <j6t@kdbg.org> writes:
> Am 08.09.21 um 15:43 schrieb Sergey Organov:
>> 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*?
>
> No, it is not a left-over. The thing is,
>
> - there is one point in the code where gitk adds options -p -C --cc (and
> more) to the command line (around line 8034),
>
> - and there is a totally different point in the code where it is decided
> whether diff-index, diff-tree, or diff-files is invoked (proc diffcmd
> around line 7871).
>
> IOW, Gitk expects that these option combinations can always be passed to
> all three commands.
I see, but the problem here is that while diff-files and diff-tree both
accept --cc according to their documentation, diff-index does not. This
means that, strictly speaking, gitk makes a mistake treating all 3
commands universally with respect to command-line arguments when it uses
--cc.
>
> Gitk does not want to look at a commit and then decide which incarnation
> of the command it wants to use (--cc vs. -p) depending on whether it is
> a merge commit or not. This decision is delegated to command that is
> invoked.
The problem is not in the kind of commit, the problem is in the command
being invoked. diff-index doesn't support --cc according to its
documentation, and thus gitk relies on undocumented behavior of
diff-index. It might well be the case that it just happened to be
"working", thus nobody cared.
> Therefore, silent fall-back from --cc to -p in case of non-merge
> commits or non-conflicted index is absolutely necessary.
I didn't change semantics of --cc, so this thing was not broken at all.
I just disabled the --cc option in diff-index command, to match the
documentation.
As a side note, in fact Git does no "silent fall-back from --cc to -p in
case of non-merge commits", even though the behavior could be indeed
seen like this. Instead, --cc implies -p, and, as --cc does not
otherwise affect treating of non-merge commits, only -p is left active
for them. Once again, this has not been recently changed, so does not
need to be fixed.
Thanks,
-- Sergey Organov
next prev parent reply other threads:[~2021-09-08 19:04 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-30 8:03 diff-index --cc no longer permitted, gitk is now broken (slightly) Johannes Sixt
2021-08-30 13:05 ` Sergey Organov
2021-08-30 18:13 ` Jeff King
2021-08-30 20:01 ` Sergey Organov
2021-08-30 20:26 ` Johannes Sixt
2021-08-30 20:45 ` Sergey Organov
2021-08-30 17:12 ` Junio C Hamano
2021-08-30 17:40 ` Sergey Organov
2021-08-30 18:09 ` Junio C Hamano
2021-08-30 20:03 ` Sergey Organov
2021-09-01 16:52 ` Sergey Organov
2021-09-07 18:19 ` Junio C Hamano
2021-09-07 19:53 ` Sergey Organov
2021-09-08 13:43 ` Sergey Organov
2021-09-08 17:23 ` Johannes Sixt
2021-09-08 19:04 ` Sergey Organov [this message]
2021-09-09 17:07 ` Junio C Hamano
2021-09-09 20:07 ` Sergey Organov
2021-09-16 9:50 ` Sergey Organov
2021-09-16 21:15 ` Junio C Hamano
2021-09-16 22:41 ` Sergey Organov
2021-09-16 22:50 ` Junio C Hamano
2021-09-17 7:08 ` Sergey Organov
2021-09-17 16:32 ` Junio C Hamano
2021-09-17 18:41 ` Sergey Organov
2021-09-17 16:58 ` Philip Oakley
2021-09-17 17:34 ` Sergey Organov
2021-09-18 17:56 ` Sergey Organov
2021-09-07 20:32 ` Johannes Sixt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pmtiopm9.fsf@osv.gnss.ru \
--to=sorganov@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=paulus@ozlabs.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.