git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Sergey Organov <sorganov@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j6t@kdbg.org>, 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: Thu, 09 Sep 2021 23:07:14 +0300	[thread overview]
Message-ID: <87pmthldhp.fsf@osv.gnss.ru> (raw)
In-Reply-To: <xmqqilz91xvq.fsf@gitster.g> (Junio C. Hamano's message of "Thu, 09 Sep 2021 10:07:05 -0700")

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

> Johannes Sixt <j6t@kdbg.org> writes:
>
>> 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. Therefore, silent fall-back from --cc to -p in case of
>> non-merge commits or non-conflicted index is absolutely necessary.
>
> Well explained.
>
> "-p" in general is an instruction to show some form of textual
> patch, and "--cc" and "-c" are the variants (i.e. compare with each
> parent and combine the comparison results) of it that naturally
> degenerates to the normal patch output when there is only one
> parent.
>
> "--cc" also flips the "m" bit,  which controls if there is any tree
> comparison should be made for merge commits, which matters for "log"
> family of commands, so in that sense "--cc" was made to imply "-m",
> but "--cc" inherently means "-p" for non-merge commits without any
> need to say X implies Y.

Except both Git documentation and implementation are in some
contradiction with the explanations above, as far as I can see, so this
looks to me like a kind of wishful thinking, sorry.

[One can read, say, year old Git documentation, prior to recent patches
(to get rid of possible bias), to see that it describes different
picture and explicitly contradicts some of the statements above, and
then I definitely did no steps to move in the direction described above
either, at least intentionally.]

Anyway, here is the current reality the way I see it (this description
assumes "-m imply -p" patch by me has been reverted due to introduction
of slight backward incompatibility):

-p enables diff for non-merge commits only
--diff-merges=XXXX enables diff for merge commits only

--cc => -p --diff-merges=cc
-c   => -p --diff-merges=c
-m   =>    --diff-merges=m

where "=>" means "is shortcut for".

Simple and clear, except -m is an outlier, as it does not imply -p.
Further, -m produces inconvenient output unless --first-parent is also
in use, and then --first-parent already implies -m, rendering explicit
-m virtually useless.

Due to this outlier, all the recent changes in this area were targeted
to make -m useful, similar to --cc/-c, not to change --cc/-c, or -p
semantics at all, so that finally we get perfect:

--cc => -p --diff-merges=cc
-c   => -p --diff-merges=c
-m   => -p --diff-merges=m

Please also notice that with this system there is no need for every
(new) way of representing of merge commits to support "fall back to -p
in case of non-merge commits" at options level, as it does not have to
deal with non-merge commits at all.

Thanks,
-- Sergey Organov

  reply	other threads:[~2021-09-09 20:07 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
2021-09-09 17:07         ` Junio C Hamano
2021-09-09 20:07           ` Sergey Organov [this message]
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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pmthldhp.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).