{"thread":{"id":"56401","subject":"diff-index --cc no longer permitted, gitk is now broken (slightly)","startedAt":"2021-08-30T08:03:25Z","lastAt":"2021-09-18T17:56:25Z","messageCount":29,"participants":["Johannes Sixt","Sergey Organov","Junio C Hamano","Jeff King","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"434055","messageId":"e6bd4cf7-ec8b-5d22-70f6-07089794df0c@kdbg.org","threadId":"56401","inReplyTo":null,"subject":"diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-08-30T08:03:18Z","receivedAt":"2021-08-30T08:03:25Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\nto diff-index, 2021-05-21) git diff-index no longer accepts --cc. This\nbreaks gitk: it invokes\n\n   git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n\nto show the staged changes (when the line \"Local changes checked in to\nindex but not committed\" is selected).\n\nThe man page of git diff-index does not mention --cc as an option. I\nhaven't fully grokked the meaning of --cc, so I cannot tell whether this\nabsence has any significance (is deliberate or an omission).\n\nIs gitk wrong to add --cc unconditionally? Should it do so only when\nthere are conflicts? Or not at all?\n\n-- Hannes\n"},{"id":"434070","messageId":"875yvn9j69.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"e6bd4cf7-ec8b-5d22-70f6-07089794df0c@kdbg.org","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-08-30T13:05:50Z","receivedAt":"2021-08-30T13:05:56Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\n> to diff-index, 2021-05-21) git diff-index no longer accepts --cc.\n\nYep, this is expected, and I even put corresponding comment in the\nsource:\n\n\t/*\n\t * We need no diff for merges options, and we need to avoid conflict\n\t * with our own meaning of \"-m\".\n\t */\n\n\n> This breaks gitk: it invokes\n>\n>    git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n>\n> to show the staged changes (when the line \"Local changes checked in to\n> index but not committed\" is selected).\n>\n> The man page of git diff-index does not mention --cc as an option. I\n> haven't fully grokked the meaning of --cc, so I cannot tell whether this\n> absence has any significance (is deliberate or an omission).\n>\n> Is gitk wrong to add --cc unconditionally? Should it do so only when\n> there are conflicts? Or not at all?\n\nAs far as I can tell, --cc had no effect on diff-index, it was just\nsilently consumed. If I'm right, this line in gitk never needed --cc.\nThen either gitk is to be fixed, or we can \"fix\" diff-index to silently\nconsume --cc/-c again, for backward compatibility.\n\nIf --cc did affect diff-index, then my commit in question is wrong and\nshould be fixed.\n\nThanks,\n-- Sergey Organov\n"},{"id":"434123","messageId":"xmqqsfyq501v.fsf@gitster.g","threadId":"56401","inReplyTo":"e6bd4cf7-ec8b-5d22-70f6-07089794df0c@kdbg.org","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-30T17:12:28Z","receivedAt":"2021-08-30T17:12:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\n> to diff-index, 2021-05-21) git diff-index no longer accepts --cc. This\n> breaks gitk: it invokes\n>\n>    git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n>\n> to show the staged changes (when the line \"Local changes checked in to\n> index but not committed\" is selected).\n>\n> The man page of git diff-index does not mention --cc as an option. I\n> haven't fully grokked the meaning of --cc, so I cannot tell whether this\n> absence has any significance (is deliberate or an omission).\n>\n> Is gitk wrong to add --cc unconditionally? Should it do so only when\n> there are conflicts? Or not at all?\n\nI think --cc is designed to naturally fall back to -p when there is\nonly one parent.  Use of both -p and --cc has also long been an\nacceptable combination, and even if we say the later --cc overrides\n-p, there is no reason not to show single parent patch here with\n--cc.\n\n\n"},{"id":"434129","messageId":"87o89e3k6s.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqqsfyq501v.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-08-30T17:40:27Z","receivedAt":"2021-08-30T17:40:36Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\n>> to diff-index, 2021-05-21) git diff-index no longer accepts --cc. This\n>> breaks gitk: it invokes\n>>\n>>    git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n>>\n>> to show the staged changes (when the line \"Local changes checked in to\n>> index but not committed\" is selected).\n>>\n>> The man page of git diff-index does not mention --cc as an option. I\n>> haven't fully grokked the meaning of --cc, so I cannot tell whether this\n>> absence has any significance (is deliberate or an omission).\n>>\n>> Is gitk wrong to add --cc unconditionally? Should it do so only when\n>> there are conflicts? Or not at all?\n>\n> I think --cc is designed to naturally fall back to -p when there is\n> only one parent.  Use of both -p and --cc has also long been an\n> acceptable combination, and even if we say the later --cc overrides\n> -p, there is no reason not to show single parent patch here with\n> --cc.\n\nI'm pretty sure I've checked diff-index doesn't use the flag that --cc\nsets when I wrote the patch, so the only incompatibility this patch\nintroduced is denying the command when --cc is given, i.e., it now\nbehaves as if diff-index doesn't support --cc *option*, that makes sense\nto me and matches diff-index documentation.\n\nIrrespective to chosen solution, it still looks to me like gitk\nshouldn't had --cc in that command in the first place. I that correct,\nor do I miss something essential?\n\nThat said, if you think that diff-index should silently accept --cc (and\n-c ?), for whatever reason, it's fine with me, provided it's properly\ndocumented and there are proper test-cases in place.\n\nThanks,\n-- Sergey Organov\n"},{"id":"434137","messageId":"xmqq7dg24xff.fsf@gitster.g","threadId":"56401","inReplyTo":"xmqqsfyq501v.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-30T18:09:08Z","receivedAt":"2021-08-30T18:09:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\n>> to diff-index, 2021-05-21) git diff-index no longer accepts --cc. This\n>> breaks gitk: it invokes\n>>\n>>    git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n>>\n>> to show the staged changes (when the line \"Local changes checked in to\n>> index but not committed\" is selected).\n>>\n>> The man page of git diff-index does not mention --cc as an option. I\n>> haven't fully grokked the meaning of --cc, so I cannot tell whether this\n>> absence has any significance (is deliberate or an omission).\n>>\n>> Is gitk wrong to add --cc unconditionally? Should it do so only when\n>> there are conflicts? Or not at all?\n>\n> I think --cc is designed to naturally fall back to -p when there is\n> only one parent.  Use of both -p and --cc has also long been an\n> acceptable combination, and even if we say the later --cc overrides\n> -p, there is no reason not to show single parent patch here with\n> --cc.\n\nAnother tangent.\n\nI think the use of --cc with diff-index can make sense in another\nway.\n\n    $ echo \"# both\" >>COPYING\n    $ git add COPYING\n    $ echo \"# work\" >>COPYING\n\nNow we have one extra line at the end in both the index and the\nworking tree file, with yet another at the end of the latter.\n\n    $ git diff-index --cc HEAD\n\nis a way to show combined diff to go to the working tree version\nstarting from HEAD and starting from the index (I needed to use an\nold version because the 'maint' and upwards are broken as reported).\n\n    $ rungit v1.5.3 diff-index --cc HEAD\n    diff --cc COPYING\n    index 8b9c100,536e555..0000000\n    --- a/COPYING\n    +++ b/COPYING\n    @@@ -358,4 -358,3 +358,5 @@@ proprietary programs.  If your program \n      consider it more useful to permit linking proprietary applications with the\n      library.  If this is what you want to do, use the GNU Lesser General\n      Public License instead of this License.\n     +# both\n    ++# work\n\nNow the way \"gitk\" used is with \"--cached\", so there is no multi-way\ncomparisons to be combined, and it is natural to fall back to \"-p\",\nso it is a different issue, but since we invented \"--cc\" to\noriginally emulate, and to later improve, the output from gitk,\nI am reasonably sure that its use of \"--cc\" should be supported.\n"},{"id":"434138","messageId":"YS0f37xPKOHCDHr1@coredump.intra.peff.net","threadId":"56401","inReplyTo":"875yvn9j69.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-30T18:13:51Z","receivedAt":"2021-08-30T18:13:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 30, 2021 at 04:05:50PM +0300, Sergey Organov wrote:\n\n> > Is gitk wrong to add --cc unconditionally? Should it do so only when\n> > there are conflicts? Or not at all?\n> \n> As far as I can tell, --cc had no effect on diff-index, it was just\n> silently consumed. If I'm right, this line in gitk never needed --cc.\n> Then either gitk is to be fixed, or we can \"fix\" diff-index to silently\n> consume --cc/-c again, for backward compatibility.\n\nI think it does have an effect. Try this to generate a simple merge\nconflict:\n\n  git init repo\n  cd repo\n\n  echo base >file\n  git add file\n  git commit -m base\n\n  echo main >file\n  git commit -am main\n\n  git checkout -b side HEAD^\n  echo side >file\n  git commit -am side\n\n  git merge main ;# maybe master here depending on your config\n\nAnd then with pre-v2.33 Git, --cc does a combined diff:\n\n  $ git.v2.32.0 diff-index -p  HEAD\n  diff --git a/file b/file\n  index 2299c37..81768df 100644\n  --- a/file\n  +++ b/file\n  @@ -1 +1,5 @@\n  +<<<<<<< HEAD\n   side\n  +=======\n  +main\n  +>>>>>>> main\n\n  $ git.v2.32.0 diff-index --cc HEAD\n  diff --cc file\n  index df967b9,2299c37..0000000\n  --- a/file\n  +++ b/file\n  @@@ -1,1 -1,1 +1,5 @@@\n  - base\n  ++<<<<<<< HEAD\n  + side\n  ++=======\n  ++main\n  ++>>>>>>> main\n\nLikewise, --raw will show a combined diff, though it's a bit less useful\nbecause the unmerged entry has a null sha1:\n\n  $ git.v2.32.0 diff-index HEAD\n  :100644 100644 2299c37978265a95cbe835a4b0f0bbf15aad5549 0000000000000000000000000000000000000000 M\tfile\n\n  $ git.v2.32.0 diff-index --cc --raw HEAD\n  ::100644 100644 100644 df967b96a579e45a18b8251732d16804b2e56a55 2299c37978265a95cbe835a4b0f0bbf15aad5549 0000000000000000000000000000000000000000 MM\tfile\n\n-Peff\n"},{"id":"434152","messageId":"87pmtuzoql.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"YS0f37xPKOHCDHr1@coredump.intra.peff.net","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-08-30T20:01:06Z","receivedAt":"2021-08-30T20:01:13Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Aug 30, 2021 at 04:05:50PM +0300, Sergey Organov wrote:\n>\n>> > Is gitk wrong to add --cc unconditionally? Should it do so only when\n>> > there are conflicts? Or not at all?\n>> \n>> As far as I can tell, --cc had no effect on diff-index, it was just\n>> silently consumed. If I'm right, this line in gitk never needed --cc.\n>> Then either gitk is to be fixed, or we can \"fix\" diff-index to silently\n>> consume --cc/-c again, for backward compatibility.\n>\n> I think it does have an effect. Try this to generate a simple merge\n> conflict:\n>\n>   git init repo\n>   cd repo\n>\n>   echo base >file\n>   git add file\n>   git commit -m base\n>\n>   echo main >file\n>   git commit -am main\n>\n>   git checkout -b side HEAD^\n>   echo side >file\n>   git commit -am side\n>\n>   git merge main ;# maybe master here depending on your config\n>\n> And then with pre-v2.33 Git, --cc does a combined diff:\n>\n>   $ git.v2.32.0 diff-index -p  HEAD\n>   diff --git a/file b/file\n>   index 2299c37..81768df 100644\n>   --- a/file\n>   +++ b/file\n>   @@ -1 +1,5 @@\n>   +<<<<<<< HEAD\n>    side\n>   +=======\n>   +main\n>   +>>>>>>> main\n>\n>   $ git.v2.32.0 diff-index --cc HEAD\n>   diff --cc file\n>   index df967b9,2299c37..0000000\n>   --- a/file\n>   +++ b/file\n>   @@@ -1,1 -1,1 +1,5 @@@\n>   - base\n>   ++<<<<<<< HEAD\n>   + side\n>   ++=======\n>   ++main\n>   ++>>>>>>> main\n>\n> Likewise, --raw will show a combined diff, though it's a bit less useful\n> because the unmerged entry has a null sha1:\n>\n>   $ git.v2.32.0 diff-index HEAD\n>   :100644 100644 2299c37978265a95cbe835a4b0f0bbf15aad5549 0000000000000000000000000000000000000000 M\tfile\n>\n>   $ git.v2.32.0 diff-index --cc --raw HEAD\n>   ::100644 100644 100644 df967b96a579e45a18b8251732d16804b2e56a55 2299c37978265a95cbe835a4b0f0bbf15aad5549 0000000000000000000000000000000000000000 MM\tfile\n>\n> -Peff\n\nWell, thanks! As it does make a difference, it's a pity it's neither\ndocumented nor being tested.\n\nI need to review the issue more closely to give a fix then.\n\nThanks,\n-- Sergey Organov\n"},{"id":"434153","messageId":"87lf4izon9.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqq7dg24xff.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-08-30T20:03:06Z","receivedAt":"2021-08-30T20:03:13Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Johannes Sixt <j6t@kdbg.org> writes:\n>>\n>>> Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\n>>> to diff-index, 2021-05-21) git diff-index no longer accepts --cc. This\n>>> breaks gitk: it invokes\n>>>\n>>>    git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n>>>\n>>> to show the staged changes (when the line \"Local changes checked in to\n>>> index but not committed\" is selected).\n>>>\n>>> The man page of git diff-index does not mention --cc as an option. I\n>>> haven't fully grokked the meaning of --cc, so I cannot tell whether this\n>>> absence has any significance (is deliberate or an omission).\n>>>\n>>> Is gitk wrong to add --cc unconditionally? Should it do so only when\n>>> there are conflicts? Or not at all?\n>>\n>> I think --cc is designed to naturally fall back to -p when there is\n>> only one parent.  Use of both -p and --cc has also long been an\n>> acceptable combination, and even if we say the later --cc overrides\n>> -p, there is no reason not to show single parent patch here with\n>> --cc.\n>\n> Another tangent.\n>\n> I think the use of --cc with diff-index can make sense in another\n> way.\n>\n>     $ echo \"# both\" >>COPYING\n>     $ git add COPYING\n>     $ echo \"# work\" >>COPYING\n>\n> Now we have one extra line at the end in both the index and the\n> working tree file, with yet another at the end of the latter.\n>\n>     $ git diff-index --cc HEAD\n>\n> is a way to show combined diff to go to the working tree version\n> starting from HEAD and starting from the index (I needed to use an\n> old version because the 'maint' and upwards are broken as reported).\n>\n>     $ rungit v1.5.3 diff-index --cc HEAD\n>     diff --cc COPYING\n>     index 8b9c100,536e555..0000000\n>     --- a/COPYING\n>     +++ b/COPYING\n>     @@@ -358,4 -358,3 +358,5 @@@ proprietary programs.  If your program \n>       consider it more useful to permit linking proprietary applications with the\n>       library.  If this is what you want to do, use the GNU Lesser General\n>       Public License instead of this License.\n>      +# both\n>     ++# work\n>\n> Now the way \"gitk\" used is with \"--cached\", so there is no multi-way\n> comparisons to be combined, and it is natural to fall back to \"-p\",\n> so it is a different issue, but since we invented \"--cc\" to\n> originally emulate, and to later improve, the output from gitk,\n> I am reasonably sure that its use of \"--cc\" should be supported.\n\nIf the patch breaks essential (even if undocumented and untested)\nbehavior, as Jeff pointed, it should obviously be fixed. I'll look at it\nmore closely to suggest a fix.\n\nThanks,\n-- Sergey Organov\n"},{"id":"434159","messageId":"3ce08986-a402-0b07-de87-2d0f01aef7dc@kdbg.org","threadId":"56401","inReplyTo":"875yvn9j69.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-08-30T20:26:32Z","receivedAt":"2021-08-30T20:26:37Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 30.08.21 um 15:05 schrieb Sergey Organov:\n> As far as I can tell, --cc had no effect on diff-index, it was just\n> silently consumed. If I'm right, this line in gitk never needed --cc.\n> Then either gitk is to be fixed, or we can \"fix\" diff-index to silently\n> consume --cc/-c again, for backward compatibility.\n\nThat latter would be much preferable. Gitk uses the combination -p --cc\nfor *all* diff commands when it requests patch output regardless of the\nnumber of parents. It would be tedious to special-case diff-index to not\npass --cc.\n\n-- Hannes\n"},{"id":"434161","messageId":"87h7f6zmo0.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"3ce08986-a402-0b07-de87-2d0f01aef7dc@kdbg.org","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-08-30T20:45:51Z","receivedAt":"2021-08-30T20:45:56Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 30.08.21 um 15:05 schrieb Sergey Organov:\n>> As far as I can tell, --cc had no effect on diff-index, it was just\n>> silently consumed. If I'm right, this line in gitk never needed --cc.\n>> Then either gitk is to be fixed, or we can \"fix\" diff-index to silently\n>> consume --cc/-c again, for backward compatibility.\n>\n> That latter would be much preferable. Gitk uses the combination -p --cc\n> for *all* diff commands when it requests patch output regardless of the\n> number of parents. It would be tedious to special-case diff-index to not\n> pass --cc.\n\nIt's already noticed by Jeff that --cc was not in fact a no-op, so\ndiff-index is to be fixed. Sorry for the breakage.\n\nThanks,\n-- Sergey Organov\n"},{"id":"434454","messageId":"87h7f4tf0b.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"e6bd4cf7-ec8b-5d22-70f6-07089794df0c@kdbg.org","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-01T16:52:20Z","receivedAt":"2021-09-01T16:52:31Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\n> to diff-index, 2021-05-21) git diff-index no longer accepts --cc. This\n> breaks gitk: it invokes\n>\n>    git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n>\n> to show the staged changes (when the line \"Local changes checked in to\n> index but not committed\" is selected).\n>\n> The man page of git diff-index does not mention --cc as an option. I\n> haven't fully grokked the meaning of --cc, so I cannot tell whether this\n> absence has any significance (is deliberate or an omission).\n>\n> Is gitk wrong to add --cc unconditionally? Should it do so only when\n> there are conflicts? Or not at all?\n>\n\nHere is a patch that fixes diff-index to accept --cc again:\n\nSubject: [PATCH] diff-index: restore -c/--cc options handling\n\nThis fixes git:19b2517f95a0a908a8ada7417cf0717299e7e1aa\n\"diff-merges: move specific diff-index \"-m\" handling to diff-index\"\n\nThat commit disabled handling of all diff for merges options in\ndiff-index on an assumption that they are unused. However, it later\nappeared that -c and --cc, even though undocumented and not being\ncovered by tests, happen to have had particular effect on diff-index\noutput.\n\nRestore original -c/--cc options handling by diff-index.\n\nSigned-off-by: Sergey Organov <sorganov@gmail.com>\n---\n builtin/diff-index.c |  6 +++---\n diff-merges.c        | 14 ++++----------\n diff-merges.h        |  2 +-\n 3 files changed, 8 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/diff-index.c b/builtin/diff-index.c\nindex cf09559e422d..5fd23ab5b6c5 100644\n--- a/builtin/diff-index.c\n+++ b/builtin/diff-index.c\n@@ -29,10 +29,10 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n \tprefix = precompose_argv_prefix(argc, argv, prefix);\n \n \t/*\n-\t * We need no diff for merges options, and we need to avoid conflict\n-\t * with our own meaning of \"-m\".\n+\t * We need (some of) diff for merges options (e.g., --cc), and we need\n+\t * to avoid conflict with our own meaning of \"-m\".\n \t */\n-\tdiff_merges_suppress_options_parsing();\n+\tdiff_merges_suppress_m_parsing();\n \n \targc = setup_revisions(argc, argv, &rev, NULL);\n \tfor (i = 1; i < argc; i++) {\ndiff --git a/diff-merges.c b/diff-merges.c\nindex d897fd8a2933..5060ccd890bd 100644\n--- a/diff-merges.c\n+++ b/diff-merges.c\n@@ -6,7 +6,7 @@ typedef void (*diff_merges_setup_func_t)(struct rev_info *);\n static void set_separate(struct rev_info *revs);\n \n static diff_merges_setup_func_t set_to_default = set_separate;\n-static int suppress_parsing;\n+static int suppress_m_parsing;\n \n static void suppress(struct rev_info *revs)\n {\n@@ -91,9 +91,9 @@ int diff_merges_config(const char *value)\n \treturn 0;\n }\n \n-void diff_merges_suppress_options_parsing(void)\n+void diff_merges_suppress_m_parsing(void)\n {\n-\tsuppress_parsing = 1;\n+\tsuppress_m_parsing = 1;\n }\n \n int diff_merges_parse_opts(struct rev_info *revs, const char **argv)\n@@ -102,10 +102,7 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)\n \tconst char *optarg;\n \tconst char *arg = argv[0];\n \n-\tif (suppress_parsing)\n-\t\treturn 0;\n-\n-\tif (!strcmp(arg, \"-m\")) {\n+\tif (!suppress_m_parsing && !strcmp(arg, \"-m\")) {\n \t\tset_to_default(revs);\n \t} else if (!strcmp(arg, \"-c\")) {\n \t\tset_combined(revs);\n@@ -153,9 +150,6 @@ void diff_merges_set_dense_combined_if_unset(struct rev_info *revs)\n \n void diff_merges_setup_revs(struct rev_info *revs)\n {\n-\tif (suppress_parsing)\n-\t\treturn;\n-\n \tif (revs->combine_merges == 0)\n \t\trevs->dense_combined_merges = 0;\n \tif (revs->separate_merges == 0)\ndiff --git a/diff-merges.h b/diff-merges.h\nindex b5d57f6563e3..19639689bb05 100644\n--- a/diff-merges.h\n+++ b/diff-merges.h\n@@ -11,7 +11,7 @@ struct rev_info;\n \n int diff_merges_config(const char *value);\n \n-void diff_merges_suppress_options_parsing(void);\n+void diff_merges_suppress_m_parsing(void);\n \n int diff_merges_parse_opts(struct rev_info *revs, const char **argv);\n \n-- \n2.33.0.114.g9123bcff51bf\n\n"},{"id":"434905","messageId":"xmqqy288b64q.fsf@gitster.g","threadId":"56401","inReplyTo":"87h7f4tf0b.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-07T18:19:33Z","receivedAt":"2021-09-07T18:19:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n> Here is a patch that fixes diff-index to accept --cc again:\n\nSorry for the delay; I did not notice there was a patch buried in a\ndiscussion thread.\n\nWe might later need to do this suppression in more codepaths if we\nfind more regressions, but let's have one fix at a time.  \n\nWill queue.\n\n>  builtin/diff-index.c |  6 +++---\n>  diff-merges.c        | 14 ++++----------\n>  diff-merges.h        |  2 +-\n\nThis would deserve new tests that cover the existing use cases,\ngiven that both of us (and other reviewers in the original thread)\ndid not notice how big a regression we are causing.\n\nWe care about --cc naturally falling back to -p when there is only\none other thing to compare with, and also we care about --cc that\nallows us to compare during conflict resolution, at least, I think.\n\nIt can and should come as a separate step, of course.  Unbreaking\ngitk for an already known breakage would be more urgent than hunting\nfor other breakages, even though the latter might result in a more\nthorough fix in the end.\n\nThanks.\n"},{"id":"434922","messageId":"878s089n79.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqqy288b64q.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-07T19:53:46Z","receivedAt":"2021-09-07T19:53:51Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>> Here is a patch that fixes diff-index to accept --cc again:\n>\n> Sorry for the delay; I did not notice there was a patch buried in a\n> discussion thread.\n\nSorry from my side as well. I was already about to re-submit the patch\nproperly, but you happened to be faster with this )\n\n>\n> We might later need to do this suppression in more codepaths if we\n> find more regressions, but let's have one fix at a time.  \n>\n> Will queue.\n>\n>>  builtin/diff-index.c |  6 +++---\n>>  diff-merges.c        | 14 ++++----------\n>>  diff-merges.h        |  2 +-\n>\n> This would deserve new tests that cover the existing use cases,\n> given that both of us (and other reviewers in the original thread)\n> did not notice how big a regression we are causing.\n\nYep, it's too easy to break undocumented and untested behavior.\n\n> We care about --cc naturally falling back to -p when there is only\n> one other thing to compare with, and also we care about --cc that\n> allows us to compare during conflict resolution, at least, I think.\n\nI'm all for more tests, but I'm afraid I'm not in a good position to\nwrite them, especially for an undocumented behavior. I think somebody\nshould first document what --cc/-c does in diff-index, and only then\nit makes sense to write some tests.\n\n> It can and should come as a separate step, of course.  Unbreaking\n> gitk for an already known breakage would be more urgent than hunting\n> for other breakages, even though the latter might result in a more\n> thorough fix in the end.\n\nMakes sense.\n\nThanks,\n-- Sergey Organov\n"},{"id":"434933","messageId":"efcba5ef-d79d-ebc0-3bbb-f86eee08db05@kdbg.org","threadId":"56401","inReplyTo":"87h7f4tf0b.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-09-07T20:32:53Z","receivedAt":"2021-09-07T20:33:02Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 01.09.21 um 18:52 schrieb Sergey Organov:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n>> Since 19b2517f95a0 (diff-merges: move specific diff-index \"-m\" handling\n>> to diff-index, 2021-05-21) git diff-index no longer accepts --cc. This\n>> breaks gitk: it invokes\n>>\n>>    git diff-index --cached -p -C --cc --no-commit-id -U3 HEAD\n>>\n>> to show the staged changes (when the line \"Local changes checked in to\n>> index but not committed\" is selected).\n>>\n>> The man page of git diff-index does not mention --cc as an option. I\n>> haven't fully grokked the meaning of --cc, so I cannot tell whether this\n>> absence has any significance (is deliberate or an omission).\n>>\n>> Is gitk wrong to add --cc unconditionally? Should it do so only when\n>> there are conflicts? Or not at all?\n>>\n> \n> Here is a patch that fixes diff-index to accept --cc again:\n> \n> Subject: [PATCH] diff-index: restore -c/--cc options handling\n> \n> This fixes git:19b2517f95a0a908a8ada7417cf0717299e7e1aa\n> \"diff-merges: move specific diff-index \"-m\" handling to diff-index\"\n> \n> That commit disabled handling of all diff for merges options in\n> diff-index on an assumption that they are unused. However, it later\n> appeared that -c and --cc, even though undocumented and not being\n> covered by tests, happen to have had particular effect on diff-index\n> output.\n> \n> Restore original -c/--cc options handling by diff-index.\n\nThank you! I can confirm that gitk works again as expected with this patch.\n\n-- Hannes\n\n> \n> Signed-off-by: Sergey Organov <sorganov@gmail.com>\n> ---\n>  builtin/diff-index.c |  6 +++---\n>  diff-merges.c        | 14 ++++----------\n>  diff-merges.h        |  2 +-\n>  3 files changed, 8 insertions(+), 14 deletions(-)\n> \n> diff --git a/builtin/diff-index.c b/builtin/diff-index.c\n> index cf09559e422d..5fd23ab5b6c5 100644\n> --- a/builtin/diff-index.c\n> +++ b/builtin/diff-index.c\n> @@ -29,10 +29,10 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n>  \tprefix = precompose_argv_prefix(argc, argv, prefix);\n>  \n>  \t/*\n> -\t * We need no diff for merges options, and we need to avoid conflict\n> -\t * with our own meaning of \"-m\".\n> +\t * We need (some of) diff for merges options (e.g., --cc), and we need\n> +\t * to avoid conflict with our own meaning of \"-m\".\n>  \t */\n> -\tdiff_merges_suppress_options_parsing();\n> +\tdiff_merges_suppress_m_parsing();\n>  \n>  \targc = setup_revisions(argc, argv, &rev, NULL);\n>  \tfor (i = 1; i < argc; i++) {\n> diff --git a/diff-merges.c b/diff-merges.c\n> index d897fd8a2933..5060ccd890bd 100644\n> --- a/diff-merges.c\n> +++ b/diff-merges.c\n> @@ -6,7 +6,7 @@ typedef void (*diff_merges_setup_func_t)(struct rev_info *);\n>  static void set_separate(struct rev_info *revs);\n>  \n>  static diff_merges_setup_func_t set_to_default = set_separate;\n> -static int suppress_parsing;\n> +static int suppress_m_parsing;\n>  \n>  static void suppress(struct rev_info *revs)\n>  {\n> @@ -91,9 +91,9 @@ int diff_merges_config(const char *value)\n>  \treturn 0;\n>  }\n>  \n> -void diff_merges_suppress_options_parsing(void)\n> +void diff_merges_suppress_m_parsing(void)\n>  {\n> -\tsuppress_parsing = 1;\n> +\tsuppress_m_parsing = 1;\n>  }\n>  \n>  int diff_merges_parse_opts(struct rev_info *revs, const char **argv)\n> @@ -102,10 +102,7 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)\n>  \tconst char *optarg;\n>  \tconst char *arg = argv[0];\n>  \n> -\tif (suppress_parsing)\n> -\t\treturn 0;\n> -\n> -\tif (!strcmp(arg, \"-m\")) {\n> +\tif (!suppress_m_parsing && !strcmp(arg, \"-m\")) {\n>  \t\tset_to_default(revs);\n>  \t} else if (!strcmp(arg, \"-c\")) {\n>  \t\tset_combined(revs);\n> @@ -153,9 +150,6 @@ void diff_merges_set_dense_combined_if_unset(struct rev_info *revs)\n>  \n>  void diff_merges_setup_revs(struct rev_info *revs)\n>  {\n> -\tif (suppress_parsing)\n> -\t\treturn;\n> -\n>  \tif (revs->combine_merges == 0)\n>  \t\trevs->dense_combined_merges = 0;\n>  \tif (revs->separate_merges == 0)\n> diff --git a/diff-merges.h b/diff-merges.h\n> index b5d57f6563e3..19639689bb05 100644\n> --- a/diff-merges.h\n> +++ b/diff-merges.h\n> @@ -11,7 +11,7 @@ struct rev_info;\n>  \n>  int diff_merges_config(const char *value);\n>  \n> -void diff_merges_suppress_options_parsing(void);\n> +void diff_merges_suppress_m_parsing(void);\n>  \n>  int diff_merges_parse_opts(struct rev_info *revs, const char **argv);\n>  \n> \n"},{"id":"435090","messageId":"87pmtjkwsj.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqqy288b64q.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-08T13:43:24Z","receivedAt":"2021-09-08T13:43:31Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>> Here is a patch that fixes diff-index to accept --cc again:\n>\n> Sorry for the delay; I did not notice there was a patch buried in a\n> discussion thread.\n>\n> We might later need to do this suppression in more codepaths if we\n> find more regressions, but let's have one fix at a time.\n\nI'm pretty positive there should be nothing left. This commit was\ndiff-index specific, and doesn't affect anything else. Nowhere in entire\nseries the semantics of --cc itself has been changed, it has been only\ndisabled as particular option in diff-index command-line parsing.\nOverall, this is pretty local change.\n\n>\n> Will queue.\n>\n>>  builtin/diff-index.c |  6 +++---\n>>  diff-merges.c        | 14 ++++----------\n>>  diff-merges.h        |  2 +-\n>\n> This would deserve new tests that cover the existing use cases,\n> given that both of us (and other reviewers in the original thread)\n> did not notice how big a regression we are causing.\n\nI don't see it as a \"big regression\", but no wonder the breakage was\nentirely unexpected, see below.\n\n>\n> We care about --cc naturally falling back to -p when there is only\n> one other thing to compare with, and also we care about --cc that\n> allows us to compare during conflict resolution, at least, I think.\n\nThe problem here is not with -c/--cc itself, it is rather with\ndiff-index. It's neither documented nor tested nor obvious what -c/--cc\nshould mean in diff-index, given -c/--cc description (e.g., in \"git help\nlog\"):\n\n       -c\n           With this option, diff output for a merge commit shows the\n           differences [...]\n\nHow an option to deal with merge commits is applicable to diff-index,\nthat:\n\n    git-diff-index - Compare a tree to the working tree or index\n\n???\n\nBesides, nobody yet told us why gitk uses --cc option in invocation of\n'diff-index' in the first place. Does it actually *rely* on particular\nundocumented behavior of \"diff-index --cc\", or is it just a copy-paste\n*leftover*?\n\nOverall, the original commit had a mistake, as the commit that was meant\nto be pure refactoring had changed observable behavior, even if\nundocumented. However, the essence of the commit, disabling of \"diff for\nmerge /commits/\" options in diff-index that does not deal with /commits/,\ncould still be the right thing to do long term.\n\nThanks,\n-- Sergey Organov\n"},{"id":"435125","messageId":"cbd0d173-ef17-576b-ab7a-465d42c82265@kdbg.org","threadId":"56401","inReplyTo":"87pmtjkwsj.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-09-08T17:23:52Z","receivedAt":"2021-09-08T17:24:02Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.09.21 um 15:43 schrieb Sergey Organov:\n> Besides, nobody yet told us why gitk uses --cc option in invocation of\n> 'diff-index' in the first place. Does it actually *rely* on particular\n> undocumented behavior of \"diff-index --cc\", or is it just a copy-paste\n> *leftover*?\n\nNo, it is not a left-over. The thing is,\n\n- there is one point in the code where gitk adds options -p -C --cc (and\nmore) to the command line (around line 8034),\n\n- and there is a totally different point in the code where it is decided\nwhether diff-index, diff-tree, or diff-files is invoked (proc diffcmd\naround line 7871).\n\nIOW, Gitk expects that these option combinations can always be passed to\nall three commands.\n\nGitk does not want to look at a commit and then decide which incarnation\nof the command it wants to use (--cc vs. -p) depending on whether it is\na merge commit or not. This decision is delegated to command that is\ninvoked. Therefore, silent fall-back from --cc to -p in case of\nnon-merge commits or non-conflicted index is absolutely necessary.\n\n-- Hannes\n"},{"id":"435141","messageId":"87pmtiopm9.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"cbd0d173-ef17-576b-ab7a-465d42c82265@kdbg.org","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-08T19:04:46Z","receivedAt":"2021-09-08T19:04:52Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 08.09.21 um 15:43 schrieb Sergey Organov:\n>> Besides, nobody yet told us why gitk uses --cc option in invocation of\n>> 'diff-index' in the first place. Does it actually *rely* on particular\n>> undocumented behavior of \"diff-index --cc\", or is it just a copy-paste\n>> *leftover*?\n>\n> No, it is not a left-over. The thing is,\n>\n> - there is one point in the code where gitk adds options -p -C --cc (and\n> more) to the command line (around line 8034),\n>\n> - and there is a totally different point in the code where it is decided\n> whether diff-index, diff-tree, or diff-files is invoked (proc diffcmd\n> around line 7871).\n>\n> IOW, Gitk expects that these option combinations can always be passed to\n> all three commands.\n\nI see, but the problem here is that while diff-files and diff-tree both\naccept --cc according to their documentation, diff-index does not. This\nmeans that, strictly speaking, gitk makes a mistake treating all 3\ncommands universally with respect to command-line arguments when it uses\n--cc.\n\n>\n> Gitk does not want to look at a commit and then decide which incarnation\n> of the command it wants to use (--cc vs. -p) depending on whether it is\n> a merge commit or not. This decision is delegated to command that is\n> invoked.\n\nThe problem is not in the kind of commit, the problem is in the command\nbeing invoked. diff-index doesn't support --cc according to its\ndocumentation, and thus gitk relies on undocumented behavior of\ndiff-index. It might well be the case that it just happened to be\n\"working\", thus nobody cared.\n\n> Therefore, silent fall-back from --cc to -p in case of non-merge\n> commits or non-conflicted index is absolutely necessary.\n\nI didn't change semantics of --cc, so this thing was not broken at all.\nI just disabled the --cc option in diff-index command, to match the\ndocumentation.\n\nAs a side note, in fact Git does no \"silent fall-back from --cc to -p in\ncase of non-merge commits\", even though the behavior could be indeed\nseen like this. Instead, --cc implies -p, and, as --cc does not\notherwise affect treating of non-merge commits, only -p is left active\nfor them. Once again, this has not been recently changed, so does not\nneed to be fixed.\n\nThanks,\n-- Sergey Organov\n"},{"id":"435280","messageId":"xmqqilz91xvq.fsf@gitster.g","threadId":"56401","inReplyTo":"cbd0d173-ef17-576b-ab7a-465d42c82265@kdbg.org","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-09T17:07:05Z","receivedAt":"2021-09-09T17:07:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Gitk does not want to look at a commit and then decide which incarnation\n> of the command it wants to use (--cc vs. -p) depending on whether it is\n> a merge commit or not. This decision is delegated to command that is\n> invoked. Therefore, silent fall-back from --cc to -p in case of\n> non-merge commits or non-conflicted index is absolutely necessary.\n\nWell explained.\n\n\"-p\" in general is an instruction to show some form of textual\npatch, and \"--cc\" and \"-c\" are the variants (i.e. compare with each\nparent and combine the comparison results) of it that naturally\ndegenerates to the normal patch output when there is only one\nparent.\n\n\"--cc\" also flips the \"m\" bit, which controls if there is any tree\ncomparison should be made for merge commits, which matters for \"log\"\nfamily of commands, so in that sense \"--cc\" was made to imply \"-m\",\nbut \"--cc\" inherently means \"-p\" for non-merge commits without any\nneed to say X implies Y.\n\nThanks.\n\n"},{"id":"435329","messageId":"87pmthldhp.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqqilz91xvq.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-09T20:07:14Z","receivedAt":"2021-09-09T20:07:20Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> Gitk does not want to look at a commit and then decide which incarnation\n>> of the command it wants to use (--cc vs. -p) depending on whether it is\n>> a merge commit or not. This decision is delegated to command that is\n>> invoked. Therefore, silent fall-back from --cc to -p in case of\n>> non-merge commits or non-conflicted index is absolutely necessary.\n>\n> Well explained.\n>\n> \"-p\" in general is an instruction to show some form of textual\n> patch, and \"--cc\" and \"-c\" are the variants (i.e. compare with each\n> parent and combine the comparison results) of it that naturally\n> degenerates to the normal patch output when there is only one\n> parent.\n>\n> \"--cc\" also flips the \"m\" bit,  which controls if there is any tree\n> comparison should be made for merge commits, which matters for \"log\"\n> family of commands, so in that sense \"--cc\" was made to imply \"-m\",\n> but \"--cc\" inherently means \"-p\" for non-merge commits without any\n> need to say X implies Y.\n\nExcept both Git documentation and implementation are in some\ncontradiction with the explanations above, as far as I can see, so this\nlooks to me like a kind of wishful thinking, sorry.\n\n[One can read, say, year old Git documentation, prior to recent patches\n(to get rid of possible bias), to see that it describes different\npicture and explicitly contradicts some of the statements above, and\nthen I definitely did no steps to move in the direction described above\neither, at least intentionally.]\n\nAnyway, here is the current reality the way I see it (this description\nassumes \"-m imply -p\" patch by me has been reverted due to introduction\nof slight backward incompatibility):\n\n-p enables diff for non-merge commits only\n--diff-merges=XXXX enables diff for merge commits only\n\n--cc => -p --diff-merges=cc\n-c   => -p --diff-merges=c\n-m   =>    --diff-merges=m\n\nwhere \"=>\" means \"is shortcut for\".\n\nSimple and clear, except -m is an outlier, as it does not imply -p.\nFurther, -m produces inconvenient output unless --first-parent is also\nin use, and then --first-parent already implies -m, rendering explicit\n-m virtually useless.\n\nDue to this outlier, all the recent changes in this area were targeted\nto make -m useful, similar to --cc/-c, not to change --cc/-c, or -p\nsemantics at all, so that finally we get perfect:\n\n--cc => -p --diff-merges=cc\n-c   => -p --diff-merges=c\n-m   => -p --diff-merges=m\n\nPlease also notice that with this system there is no need for every\n(new) way of representing of merge commits to support \"fall back to -p\nin case of non-merge commits\" at options level, as it does not have to\ndeal with non-merge commits at all.\n\nThanks,\n-- Sergey Organov\n"},{"id":"436094","messageId":"87sfy497ed.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"87pmtjkwsj.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-16T09:50:02Z","receivedAt":"2021-09-16T09:50:15Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Sergey Organov <sorganov@gmail.com> writes:\n>>\n>>> Here is a patch that fixes diff-index to accept --cc again:\n>>\n>> Sorry for the delay; I did not notice there was a patch buried in a\n>> discussion thread.\n>>\n>> We might later need to do this suppression in more codepaths if we\n>> find more regressions, but let's have one fix at a time.\n>\n> I'm pretty positive there should be nothing left. This commit was\n> diff-index specific, and doesn't affect anything else. Nowhere in entire\n> series the semantics of --cc itself has been changed, it has been only\n> disabled as particular option in diff-index command-line parsing.\n> Overall, this is pretty local change.\n\nI'm afraid this issue is left up in the air after application of the\nfix-up patch, as usage of --cc in the diff-index is still undocumented.\nI.e., the fix-up just restores the historical status quo that has a\nproblem by itself.\n\nAs current documentation of --cc elsewhere does not seem to be even\nsuitable for diff-index, I don't feel like providing corresponding patch\nfor the documentation, even less so as I still have no idea if current\nbehavior is intended or accidental, and if it's the latter, do we need\nto keep or somehow fix it? Anybody?\n\nThanks,\n-- Sergey Organov\n"},{"id":"436163","messageId":"xmqqy27wrzmj.fsf@gitster.g","threadId":"56401","inReplyTo":"87sfy497ed.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-16T21:15:16Z","receivedAt":"2021-09-16T21:15:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n> I'm afraid this issue is left up in the air after application of the\n> fix-up patch, as usage of --cc in the diff-index is still undocumented.\n\nYeah, I do think documentation update is needed, but being buried by\nother topics I haven't had a chance to revisit the way how --cc is\ndescribed in the wider context in order to make an intellgent\nsuggestion on how to present it in the context of \"diff-index\".\n\n> I.e., the fix-up just restores the historical status quo that has a\n> problem by itself.\n\nI do agree \"show -p\" on merge is an oddball that trips new people,\nbecause it does not imply the \"do present the changes for merges\"\nbit unlike \"show -c/--cc\" do, and from that point of view, the\ngeneralization --diff-merges tried to bring us was a good thing.\n\nBut \"-c/--cc\" are explicit enough in what they want to do.  It does\nwant to present the changes to compare a single end state with\npossibly more than one starting state (e.g. a merge) and not\nrequiring an explicit \"-m\" is quite natural.  Even more, when it\ncompares the end state with only one starting state (e.g. showing a\nsingle parent commit), there is only one pairwise result to\n\"combine\", so it is also natural that it ends up showing the same\noutput as \"-p\".  So I do not quite see the behaviour of \"diff*/show\n--cc\" as a problem, though.  IOW, the use pattern in gitk is more\nthan just \"historical status quo\", but is quite sensible, I would\nhave to say.\n\nThanks.\n"},{"id":"436182","messageId":"874kaki1nb.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqqy27wrzmj.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-16T22:41:44Z","receivedAt":"2021-09-16T22:41:51Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>> I'm afraid this issue is left up in the air after application of the\n>> fix-up patch, as usage of --cc in the diff-index is still undocumented.\n>\n> Yeah, I do think documentation update is needed, but being buried by\n> other topics I haven't had a chance to revisit the way how --cc is\n> described in the wider context in order to make an intellgent\n> suggestion on how to present it in the context of \"diff-index\".\n\nI tend to believe yet another description of --cc is needed for\ndiff-index, separate from the current one. Just saying.\n\n>\n>> I.e., the fix-up just restores the historical status quo that has a\n>> problem by itself.\n>\n> I do agree \"show -p\" on merge is an oddball that trips new people,\n> because it does not imply the \"do present the changes for merges\"\n> bit unlike \"show -c/--cc\" do, and from that point of view, the\n> generalization --diff-merges tried to bring us was a good thing.\n\nI'm not sure I follow. What \"show -p\" has to do with \"diff-index --cc\"?\n\nMy only point here is that usage of *--cc* in *diff-index* is entirely\nundocumneted, and that needs to be somehow resolved.\n\n>\n> But \"-c/--cc\" are explicit enough in what they want to do.  It does\n> want to present the changes to compare a single end state with\n> possibly more than one starting state (e.g. a merge) and not\n> requiring an explicit \"-m\" is quite natural.\n\nDoesn't seem to be relevant for \"diff-index --cc\" lacking documentation,\nbut -m and -c and --cc are rather *mutually exclusive*. I.e., they all\nset different formats for output of diffs for merges, so \"--cc -m\" ==\n\"-m\", and \"-m --cc\" == \"--cc\", i.e., the latest overrides the format to\nbe used. Therefore \"requiring explicit -m for -cc\" simply doesn't make\nsense.\n\n> Even more, when it compares the end state with only one starting state\n> (e.g. showing a single parent commit), there is only one pairwise\n> result to \"combine\", so it is also natural that it ends up showing the\n> same output as \"-p\". So I do not quite see the behaviour of\n> \"diff*/show --cc\" as a problem, though.\n\nI don't see it as a problem as well, so whom you are arguing with?\n\nThe only problem in this particular case I see is that \"diff-index --cc\"\nis undocumented (and untested), and this has nothing to do with\nlog/diff/show, unless I miss your point.\n\n> IOW, the use pattern in gitk is more than just \"historical status\n> quo\", but is quite sensible, I would have to say.\n\n\"diff-index --cc\" in gitk is a bug, as according to Git documentation\n\"diff-index\" does not accept \"--cc\", period.\n\ngitk trying to make sense of what is neither documented, nor tested, nor\nguaranteed is the problem, but I was talking even not about that\nproblem, but rather about the cause of this: some undocumented\nprocessing of \"git diff-index --cc\".\n\nThanks,\n-- Sergey Organov\n"},{"id":"436189","messageId":"xmqqzgscqgmv.fsf@gitster.g","threadId":"56401","inReplyTo":"874kaki1nb.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-16T22:50:48Z","receivedAt":"2021-09-16T22:50:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n> I'm not sure I follow. What \"show -p\" has to do with \"diff-index --cc\"?\n>\n> My only point here is that usage of *--cc* in *diff-index* is entirely\n> undocumneted, and that needs to be somehow resolved.\n\nIt was a response to your \"historical status quo that is a problem.\"\nI do not think there is any problem with \"diff-index --cc\" (except\nfor it wants a better documentation---but that we already agree) but\nI wanted to give you some credit for having worked on \"--diff-merges\",\nan effort to generalize things in a related area.\n"},{"id":"436213","messageId":"87bl4rk7bh.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqqzgscqgmv.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-17T07:08:34Z","receivedAt":"2021-09-17T07:08:42Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>> I'm not sure I follow. What \"show -p\" has to do with \"diff-index --cc\"?\n>>\n>> My only point here is that usage of *--cc* in *diff-index* is entirely\n>> undocumneted, and that needs to be somehow resolved.\n>\n> It was a response to your \"historical status quo that is a problem.\"\n> I do not think there is any problem with \"diff-index --cc\" (except\n> for it wants a better documentation---but that we already agree) but\n\nAh, now I see, but it's exactly lack of documentation (and tests) that I\nwas referring to as the \"problem of the historical status quo\" on the\nGit side, so I was somewhat confused by your original response.\n\nAlso, it's still unclear, even if not very essential, what exactly that\n\"status quo\" is when seen from the point of view of gitk. Does gitk\nactually utilize *particular output* of \"diff-index --cc\" for better, or\ngitk would be just as happy if it were synonym for \"diff-index -p\", or\neven if it'd be just as happy if --cc were silently consumed by\ndiff-index?\n\n> I wanted to give you some credit for having worked on \"--diff-merges\",\n> an effort to generalize things in a related area.\n\nThanks for that! More to follow )\n\nThanks,\n-- Sergey Organov\n"},{"id":"436231","messageId":"xmqqbl4ri2mq.fsf@gitster.g","threadId":"56401","inReplyTo":"87bl4rk7bh.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-17T16:32:45Z","receivedAt":"2021-09-17T16:32:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n> Ah, now I see, but it's exactly lack of documentation (and tests) that I\n> was referring to as the \"problem of the historical status quo\" on the\n> Git side, so I was somewhat confused by your original response.\n\nWell, you said the fixup \"restores\" the status quo, but in fact,\nwith or without the fixup, before or after it, the lack of\ndocumentation was there.  So I thought you were talking about\nsomething else.\n\n>> I wanted to give you some credit for having worked on \"--diff-merges\",\n>> an effort to generalize things in a related area.\n>\n> Thanks for that! More to follow )\n\nI somehow expect there was need for no further work in this area,\nbut there are also many other areas in Git where your talent is\napplicable and appreciated, I am sure ;-)\n\nThanks.\n"},{"id":"436237","messageId":"61ffe5ee-dcc9-4308-7ced-430c3891e898@iee.email","threadId":"56401","inReplyTo":"87bl4rk7bh.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-09-17T16:58:55Z","receivedAt":"2021-09-17T16:59:00Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 17/09/2021 08:08, Sergey Organov wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Sergey Organov <sorganov@gmail.com> writes:\n>>\n>>> I'm not sure I follow. What \"show -p\" has to do with \"diff-index --cc\"?\n>>>\n>>> My only point here is that usage of *--cc* in *diff-index* is entirely\n>>> undocumneted, and that needs to be somehow resolved.\n>> It was a response to your \"historical status quo that is a problem.\"\n>> I do not think there is any problem with \"diff-index --cc\" (except\n>> for it wants a better documentation---but that we already agree) but\n> Ah, now I see, but it's exactly lack of documentation (and tests) that I\n> was referring to as the \"problem of the historical status quo\" on the\n> Git side, so I was somewhat confused by your original response.\n>\n> Also, it's still unclear, even if not very essential, what exactly that\n> \"status quo\" is when seen from the point of view of gitk. Does gitk\n> actually utilize *particular output* of \"diff-index --cc\" for better, or\n> gitk would be just as happy if it were synonym for \"diff-index -p\", or\n> even if it'd be just as happy if --cc were silently consumed by\n> diff-index?\n\nDid Johannes Sixt's earlier answer\nhttps://lore.kernel.org/git/cbd0d173-ef17-576b-ab7a-465d42c82265@kdbg.org/\nhelp clarify the choices?\n>> I wanted to give you some credit for having worked on \"--diff-merges\",\n>> an effort to generalize things in a related area.\n> Thanks for that! More to follow )\n>\n> Thanks,\n> -- Sergey Organov\n\n"},{"id":"436241","messageId":"87ee9n5cob.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"61ffe5ee-dcc9-4308-7ced-430c3891e898@iee.email","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-17T17:34:12Z","receivedAt":"2021-09-17T17:34:19Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Philip Oakley <philipoakley@iee.email> writes:\n\n> On 17/09/2021 08:08, Sergey Organov wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Sergey Organov <sorganov@gmail.com> writes:\n>>>\n>>>> I'm not sure I follow. What \"show -p\" has to do with \"diff-index --cc\"?\n>>>>\n>>>> My only point here is that usage of *--cc* in *diff-index* is entirely\n>>>> undocumneted, and that needs to be somehow resolved.\n>>> It was a response to your \"historical status quo that is a problem.\"\n>>> I do not think there is any problem with \"diff-index --cc\" (except\n>>> for it wants a better documentation---but that we already agree) but\n>> Ah, now I see, but it's exactly lack of documentation (and tests) that I\n>> was referring to as the \"problem of the historical status quo\" on the\n>> Git side, so I was somewhat confused by your original response.\n>>\n>> Also, it's still unclear, even if not very essential, what exactly that\n>> \"status quo\" is when seen from the point of view of gitk. Does gitk\n>> actually utilize *particular output* of \"diff-index --cc\" for better, or\n>> gitk would be just as happy if it were synonym for \"diff-index -p\", or\n>> even if it'd be just as happy if --cc were silently consumed by\n>> diff-index?\n>\n> Did Johannes Sixt's earlier answer\n> https://lore.kernel.org/git/cbd0d173-ef17-576b-ab7a-465d42c82265@kdbg.org/\n> help clarify the choices?\n\nSorry, no. I did read that carefully when it has been posted. Further\nexplanations by Johannes also only tell that gitk expects --cc to be\naccepted by diff-index as it likes to treat multiple commands\nuniversally, but don't specify what output git expects from --cc when it\npasses it exactly to diff-index. Maybe it just shows the output and have\nno other expectations, dunno.\n\n\"silent fall-back from --cc to -p in case of non-merge commits or\nnon-conflicted index is absolutely necessary\" that is stated there is a\nnon-issue as it was always the case, and there are no questions about\nit, as --cc in fact implies -p, and the latter is applied to non-merge\ncommits. So, how Git treats \"non-conflicted index\" and non-merge commits\ndoes not depend on --cc. It's how it treats \"conflicting index\" that is\nstill unspecified.\n\nThanks,\n-- Sergey Organov\n"},{"id":"436244","messageId":"878rzv59k9.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"xmqqbl4ri2mq.fsf@gitster.g","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-17T18:41:26Z","receivedAt":"2021-09-17T18:41:37Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>> Ah, now I see, but it's exactly lack of documentation (and tests) that I\n>> was referring to as the \"problem of the historical status quo\" on the\n>> Git side, so I was somewhat confused by your original response.\n>\n> Well, you said the fixup \"restores\" the status quo, but in fact,\n> with or without the fixup, before or after it, the lack of\n> documentation was there.\n\nNo, what problematic patch did, it changed behavior of diff-index\nexactly in accordance with its *current* documentation that doesn't\nmention --cc as accepted command-line option for diff-index. So, with\nthat patch applied, there were no this problem with documentation\nanymore. Implementation now actually matched the docs.\n\nUnfortunately, that brought worse problem: it unexpectedly broke gitk,\nthat, as it appeared, depends on undocumented diff-index behavior.\n\nSo, I re-enabled --cc in diff-index, lesser of two evils, that brought\nback the problem of lack of documentation and test cases for \"diff-index\n--cc\". This way, the status quo has been restored indeed.\n\n> So I thought you were talking about something else.\n>\n>>> I wanted to give you some credit for having worked on \"--diff-merges\",\n>>> an effort to generalize things in a related area.\n>>\n>> Thanks for that! More to follow )\n>\n> I somehow expect there was need for no further work in this area,\n> but there are also many other areas in Git where your talent is\n> applicable and appreciated, I am sure ;-)\n\nI'm afraid we still didn't reach one of the ultimate goals of all this:\nletting -m be useful again, specifically, as suitable *user* option.\n\nAlso, current --diff-merges options are incapable of providing current\n-m behavior, as has been noticed by Jonathan Nieder in another thread on\nreverting \"-m implies -p\" commit:\n\n  \"When I try it locally, -m shows no diff by default,\n   whereas --diff-merges=separate shows a diff for merges.\"\n\nand I'm going to fix this by adding yet another feature for\n--diff-merges. This is to be pure addition, thus causing no backward\ncompatibility problems.\n\nThanks,\n-- Sergey Organov\n"},{"id":"436317","messageId":"87mto99398.fsf@osv.gnss.ru","threadId":"56401","inReplyTo":"87ee9n5cob.fsf@osv.gnss.ru","subject":"Re: diff-index --cc no longer permitted, gitk is now broken (slightly)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-09-18T17:56:19Z","receivedAt":"2021-09-18T17:56:25Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n> Philip Oakley <philipoakley@iee.email> writes:\n>\n>> On 17/09/2021 08:08, Sergey Organov wrote:\n>>> Junio C Hamano <gitster@pobox.com> writes:\n>>>\n>>>> Sergey Organov <sorganov@gmail.com> writes:\n>>>>\n>>>>> I'm not sure I follow. What \"show -p\" has to do with \"diff-index --cc\"?\n>>>>>\n>>>>> My only point here is that usage of *--cc* in *diff-index* is entirely\n>>>>> undocumneted, and that needs to be somehow resolved.\n>>>> It was a response to your \"historical status quo that is a problem.\"\n>>>> I do not think there is any problem with \"diff-index --cc\" (except\n>>>> for it wants a better documentation---but that we already agree) but\n>>> Ah, now I see, but it's exactly lack of documentation (and tests) that I\n>>> was referring to as the \"problem of the historical status quo\" on the\n>>> Git side, so I was somewhat confused by your original response.\n>>>\n>>> Also, it's still unclear, even if not very essential, what exactly that\n>>> \"status quo\" is when seen from the point of view of gitk. Does gitk\n>>> actually utilize *particular output* of \"diff-index --cc\" for better, or\n>>> gitk would be just as happy if it were synonym for \"diff-index -p\", or\n>>> even if it'd be just as happy if --cc were silently consumed by\n>>> diff-index?\n>>\n>> Did Johannes Sixt's earlier answer\n>> https://lore.kernel.org/git/cbd0d173-ef17-576b-ab7a-465d42c82265@kdbg.org/\n>> help clarify the choices?\n>\n> Sorry, no. I did read that carefully when it has been posted. Further\n> explanations by Johannes also only tell that gitk expects --cc to be\n> accepted by diff-index as it likes to treat multiple commands\n> universally, but don't specify what output git expects from --cc when it\n> passes it exactly to diff-index. Maybe it just shows the output and have\n> no other expectations, dunno.\n\nAnd, even more importantly, if gitk uses \"git diff-index --cc\" for its\nspecific output, is there another, documented way to achieve the same\ngoal?\n\nThanks,\n-- Sergey Organov\n"}]}