Re: [PATCH 2/4] merge-base: "--is-ancestor A B"

From: Martin von Zweigbergk <martinvonz@gmail.com>
Date: Aug 31, 2012, 18:03 UTC
Message-ID: <CANiSa6i5tY_V_dhK0wSaKzy0xwvN9uTqRJUJ7ZvgqCG7Ta5rsQ@mail.gmail.com>
In-Reply-To: <7vtxvjnfcu.fsf@alter.siamese.dyndns.org>

On Fri, Aug 31, 2012 at 10:25 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 10 quoted lines

> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>>> +       if (argc != 2)
>>> +               die("--is-ancestor takes exactly two commits");
>>
>> I think git merge-base shows the usage message regardless if argc < 2,
>> so this only happens when more than two arguments are given.
>
> Yes, but it is a good discipline not to depend too much on what
> other parts of the code may have done, when you do not have to.

Yes, I definitely agree. It was more that when I saw this line and then tried it out, I was a little surprised that it didn't tell med "--is-ancestor takes exactly two commits" when I gave it only one. But since few users read the code before running the command, it will probably not be too surprising :-)

>> Maybe
>> include --is-ancestor in the usage message?
>
> I think "merge-base -h" will get this for free thanks to parseopt.

It seems not, or maybe I'm using the wrong terminology; I meant to include it in the first part of the below.

usage: git merge-base [-a|--all] <commit> <commit>...
   or: git merge-base [-a|--all] --octopus <commit>...
   or: git merge-base --independent <commit>...

    -a, --all             output all common ancestors
    --octopus             find ancestors for a single n-way merge
    --independent         list revs not reachable from others
    --is-ancestor         is the first one ancestor of the other?

>  'git merge-base' [-a|--all] --octopus <commit>...
> +'git merge-base' --is-ancestor <commit> <commit>
>  'git merge-base' --independent <commit>...

Ah, I guess this is what I was looking for. The remainder of the patch looks good too. Thanks.