Hi,

On Mon, 6 Nov 2023, Elijah Newren wrote:

> On Thu, Nov 2, 2023 at 6:52 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> >
> [...]
>
> >     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> >      -
> >         strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
> >
> >     ++  /*
> >     ++   * TODO: For now, let's warn when we see an option that we are
> >     ++   * going to override after setup_revisions() below. In the
> >     ++   * future we might want to either die() or allow them if we
> >     ++   * think they could be useful though.
> >     ++   */
> >     ++  for (i = 0; i < argc; i++) {
> >     ++          if (!strcmp(argv[i], "--reverse") || !strcmp(argv[i], "--date-order") ||
> >     ++              !strcmp(argv[i], "--topo-order") || !strcmp(argv[i], "--author-date-order") ||
> >     ++              !strcmp(argv[i], "--full-history"))
> >     ++                  warning(_("option '%s' will be overridden"), argv[i]);
> >     ++  }
> >     ++
>
> [...] This seems like an inefficient way to provide this warning;

Not only inefficient: think about the false positive when looking at a
command-line containing `--grep --reverse`. In this instance, `--reverse`
is an argument of the `--grep` option, but would be mistaken for an option
in its own right.

Ciao,
Johannes