From: Junio C Hamano <gitster@pobox.com>
To: Sergey Organov <sorganov@gmail.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH v2] revision: add separate field for "-m" of "diff-index -m"
Date: Mon, 31 Aug 2020 10:00:45 -0700 [thread overview]
Message-ID: <xmqqd036hhs2.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <87a6ybugpe.fsf@osv.gnss.ru> (Sergey Organov's message of "Mon, 31 Aug 2020 15:45:33 +0300")
Sergey Organov <sorganov@gmail.com> writes:
> $ git diff-index -m --no-diff-merges HEAD
> :100644 000000 4aec621a6d1a9a5892f0b4b6feb2ed329fd04bf2 0000000000000000000000000000000000000000 D main/main.cc
At the first glance, this looked like a good justification for this
patch.
> If you say "compatibility wart" is not a trouble by itself, -- I'm fine
> with it, -- then "more" in my commit message is misplaced indeed.
Yeah, when I wrote the "compatibility wart" comment originally, I
was describing "this needs a tricky code because two independent
options happen to share the command line parser" and nothing more.
I was not reacting to "more", by the way. I was reacting the lack
of concrete problem description. "A '-m' option given to the
'diff-index' command can be defeated by giving '--no-diff-merges'
later" you showed above can be a good replacement for "causes more
troubles".
But in the ideal world, "--[no-]diff-merges" should be rejected as
an irrelevant/unrecognised option to the "diff" family of commands
(as I said in the message you are responding to, it is only relevant
to the "log" family of commands where the diff machinery is solely
to compare between (some of) its parents and in that context, what,
if anything, kind of special treatment is made for merge commits
makes sense as an optional instruction to the command). Splitting
the field into two fields, setting both fields upon "-m" but
toggling only one with longhand "--[no-]diff-merges" would allow the
code to notice and make the above command line silently turn the
"--[no-]diff-merges" into a no-op, so in that sense it would be a
good first step, but an ideal solution would probably need to know
if we are parsing for the "log" family or for the "diff" family and
error out upon seeing a "log"-only option like "--[no-]diff-merges"
when checking the command line option for "diff".
> This change has nothing to do with defaults. It rather about correct and
> clear code.
OK, I misread your intention. Sorry about that.
Thanks.
next prev parent reply other threads:[~2020-08-31 17:00 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-29 19:46 [PATCH] revision: add separate field for "-m" of "diff-index -m" Sergey Organov
2020-08-29 20:11 ` [PATCH v2] " Sergey Organov
2020-08-31 4:49 ` Junio C Hamano
2020-08-31 12:45 ` Sergey Organov
2020-08-31 17:00 ` Junio C Hamano [this message]
2020-08-31 12:53 ` [PATCH v3] " Sergey Organov
2020-08-31 17:23 ` Junio C Hamano
2020-08-31 19:41 ` Sergey Organov
2020-08-31 20:14 ` [PATCH v4] " Sergey Organov
2020-08-31 20:42 ` Junio C Hamano
2020-08-31 21:01 ` Sergey Organov
2020-10-23 4:03 ` [PATCH] revision: wording tweak in comment for parsing "-m" Junio C Hamano
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=xmqqd036hhs2.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=sorganov@gmail.com \
/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 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 NNTP newsgroup(s).