Patrick Steinhardt <ps@xxxxxx> writes:
> + } else if (!strcmp(arg, "--unsorted-input")) {
> + if (revs->no_walk && !revs->unsorted_input)
> + die(_("--unsorted-input is incompatible with --no-walk and --no-walk=sorted"));
> + revs->unsorted_input = 1;
So this can be used with --no-walk=unsorted, even though doing
so would be redundant and meaningless. OK.
> @@ -2651,6 +2655,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
> } else if (!strcmp(arg, "--not")) {
> *flags ^= UNINTERESTING | BOTTOM;
> } else if (!strcmp(arg, "--no-walk")) {
> + if (revs->unsorted_input)
> + die(_("--no-walk is incompatible with --no-walk=unsorted and --unsorted-input"));
And likewise, --no-walk is --no-walk=sorted so we do not allow it to
be used with --unsorted-input or --no=walk=unsorted. OK.
> revs->no_walk = 1;
> } else if (skip_prefix(arg, "--no-walk=", &optarg)) {
> /*
> @@ -2658,9 +2664,12 @@ static int handle_revision_pseudo_opt(const char *submodule,
> * not allowed, since the argument is optional.
> */
> revs->no_walk = 1;
> - if (!strcmp(optarg, "sorted"))
> + if (!strcmp(optarg, "sorted")) {
> + if (revs->unsorted_input)
> + die(_("--no-walk=sorted is incompatible with --no-walk=unsorted "
> + "and --unsorted-input"));
OK.
> revs->unsorted_input = 0;
> - else if (!strcmp(optarg, "unsorted"))
> + } else if (!strcmp(optarg, "unsorted"))
> revs->unsorted_input = 1;
This is --no-walk=unsorted; could it have been given after --no-walk
or --no-walk=unsorted?
The application of the incompatibility rules seems a bit uneven. An
earlier piece of code will reject "--no-walk=unsorted --no-walk" given
in this order (see "And likewise" above). But here, this part of
the code will happily take "--no-walk --no-walk=unsorted".
Of course these details can be fixed with more careful code design,
but I wonder if it may be result in the code and behaviour that is
far simpler to explain (and probably implement) if we declare that
* --no-walk is not a synonym to --no-walk=sorted; it just flips
.no_walk member on.
* --no-walk=sorted and --no-walk=unsorted flip .no_walk member on,
and then flips .unsorted_input member off or on, respectively.
and define that the usual last-one-wins rule would apply?
Thanks.
