Am 06.12.23 um 14:08 schrieb Patrick Steinhardt: > 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@xxxxxx> >> --- >> 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é