Siddharth Kannan <kannan.siddharth12@xxxxxxxxx> writes:
> @@ -2234,11 +2235,18 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
> }
> if (opts < 0)
> exit(128);
> - continue;
> +
> + args = handle_revision_arg(arg, revs, flags, revarg_opt);
> + handle_rev_arg_called = 1;
> + if (args)
> + continue;
> + else
> + --left;
> }
>
>
> - if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
> + if ((handle_rev_arg_called && args) ||
> + handle_revision_arg(arg, revs, flags, revarg_opt)) {
Naively I would have expected that removing the "continue" at the
end and letting the control go to the existing
if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
would be all that is needed. The latter half of the patch is an
artifact of having ane xtra "handle_revision_arg() calls inside the
"if it begins with dash" block to avoid calling it twice.
So the difference is just "--left" (by the way, our codebase seem to
prefer "left--" when there is no difference between pre- or post-
decrement/increment) that adjusts the slot in argv[] where the next
unknown argument is stuffed to.
The adjustment is needed as the call to handle_revision_opt() that
is before the pre-context of this hunk stuffed the unknown thing
that begins with "-" into argv[left++]; if that thing turns out to
be a valid rev, then you would need to take it back, because after
all, that is not an unknown command line argument.
I am wondering if writing it like the following is easier to
understand. I had a hard time figuring out what you are trying to
do, partly because "args" is quite a misnomer---implying "how many
arguments did we see" that is similar to opts that does mean "how
many options did handle_revision_opts() see?" The variable means
means "yes we saw a valid rev" when it is zero. The rewrite
below may avoid such a confusion. I dunno.
revision.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/revision.c b/revision.c
index b37dbec378..e238430948 100644
--- a/revision.c
+++ b/revision.c
@@ -2204,6 +2204,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
revarg_opt |= REVARG_CANNOT_BE_FILENAME;
read_from_stdin = 0;
for (left = i = 1; i < argc; i++) {
+ int maybe_rev = 0;
const char *arg = argv[i];
if (*arg == '-') {
int opts;
@@ -2234,11 +2235,16 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
}
if (opts < 0)
exit(128);
- continue;
+ maybe_rev = 1;
+ left--; /* tentatively cancel "unknown opt" */
}
-
- if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
+ if (!handle_revision_arg(arg, revs, flags, revarg_opt)) {
+ got_rev_arg = 1;
+ } else if (maybe_rev) {
+ left++; /* it turns out that it was "unknown opt" */
+ continue;
+ } else {
int j;
if (seen_dashdash || *arg == '^')
die("bad revision '%s'", arg);
@@ -2255,8 +2261,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
append_prune_data(&prune_data, argv + i);
break;
}
- else
- got_rev_arg = 1;
}
if (prune_data.nr) {