Re: [PATCH 3/7] revision: use die_for_incompatible_opt3() for --graph/--reverse/--walk-reflogs
- From
René Scharfe <l.s.r@web.de>
- Date
- Dec 6, 2023, 13:47 UTC
- Message-ID
- <382fac73-1afc-4b89-ad0f-3320c7a64838@web.de>
- In-Reply-To
- <ZXByNGuQTaOQ3sKW@tanuki>
Am 06.12.23 um 14:08 schrieb Patrick Steinhardt:
Show 41 quoted lines
> On Wed, Dec 06, 2023 at 12:51:57PM +0100, René Scharfe wrote:
>> The revision options --reverse is incompatibel with --walk-reflogs and
>> --graph is incompatible with both --reverse and --walk-reflogs. So they
>> are all incompatible with each other.
>>
>> Use the function for checking three mutually incompatible options,
>> die_for_incompatible_opt3(), to perform this check in one place and
>> without repetition. This is shorter and clearer.
>>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>> revision.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/revision.c b/revision.c
>> index b2861474b1..1b7e1af6c6 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -3036,8 +3036,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>> revs->grep_filter.ignore_locale = 1;
>> compile_grep_patterns(&revs->grep_filter);
>>
>> - if (revs->reverse && revs->reflog_info)
>> - die(_("options '%s' and '%s' cannot be used together"), "--reverse", "--walk-reflogs");
>> if (revs->reflog_info && revs->limited)
>> die("cannot combine --walk-reflogs with history-limiting options");
>> if (revs->rewrite_parents && revs->children.name)
>> @@ -3048,11 +3046,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>> /*
>> * Limitations on the graph functionality
>> */
>> - if (revs->reverse && revs->graph)
>> - die(_("options '%s' and '%s' cannot be used together"), "--reverse", "--graph");
>> + die_for_incompatible_opt3(!!revs->graph, "--graph",
>> + !!revs->reverse, "--reverse",
>> + !!revs->reflog_info, "--walk-reflogs");
>
> I've been wondering why we use `!!` here, as `die_for_incompatible_*()`
> doesn't care for the actual value but only checks that it is non-zero.
> Is it because of the type mismatch, where these flags here use unsigned
> ints whereas `die_for_incompatible_*()` expect ints?->graph and ->reflog_info are pointers and clang reports an int-conversion warning without the double negation. ->reverse is an unsigned:1 and so doesn't need it, but I gave it one anyway for aesthetic reasons.
René