Re: [PATCH 1/5] Add a new option 'merges' to revision.c

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 22, 2015, 23:31 UTC
Message-ID: <xmqqoankbodw.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1427048921-28677-1-git-send-email-koosha@posteo.de>

Koosha Khajehmoogahi <koosha@posteo.de> writes:

Show 7 quoted lines

> @@ -1800,9 +1817,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  		revs->show_all = 1;
>  	} else if (!strcmp(arg, "--remove-empty")) {
>  		revs->remove_empty_trees = 1;
> +	} else if (starts_with(arg, "--merges=")) {
> +		if (parse_merges_opt(revs, arg + 9))
> +			die("unknown option: %s", arg);

This one makes sense to me (so does what the parse_merges_opt() does).

>  	} else if (!strcmp(arg, "--merges")) {
> +		revs->max_parents = -1;
>  		revs->min_parents = 2;

But is this change warranted? An existing user who is not at all interested in the new --merges= option may be relying on the fact that "--merges" does not affect the value of max_parents and she can say "log --max-parents=2 --merges" to see only the two-way merges, for example. This change just broke her, and I do not see why it is a good thing.

>  	} else if (!strcmp(arg, "--no-merges")) {
> +		revs->min_parents = 0;
>  		revs->max_parents = 1;

Likewise.