Re: [PATCH v3] backfill: handle unexpected arguments
- From
Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
- Date
- Mar 23, 2026, 06:17 UTC
- Message-ID
- <CAGWgyh8E-=A+NKEAOz3xHPq96rf3j0tt7++3xvg7NoMCKq_Www@mail.gmail.com>
- In-Reply-To
- <6460601f-ff72-4683-abd1-2ae4c8352a27@gmail.com>
On Mon, 23 Mar 2026 at 07:12, Derrick Stolee <stolee@gmail.com> wrote:
Show 22 quoted lines
>
> On 3/22/26 9:01 PM, Junio C Hamano wrote:
> > Derrick Stolee <stolee@gmail.com> writes:
> >
> >>> + if (argc) {
> >>> + error(_("unknown argument '%s'"), argv[0]);
> >>> + usage(builtin_backfill_usage[0]);
> >>> + }
> >>
> >> Before we get too far into this: How does this interact with
> >> the ongoing change to introduce revision arguments to 'git
> >> backfill' [1]?
> >
> > Ahh, that one completely slipped my mind.
> >
> > Thanks for a doze of sanity. This patch becomes completely
> > irrelevant if we are taking command line arguments.
> >
> > It will become the responsibility of the other topic to detect and
> > complain about excess command line parameters (unless the feature it
> > adds absorbs all of them, which may be the case).
>Thank you for pointing this out, and apologies for missing the in-flight series. I agree that this patch becomes irrelevant given the revision arguments work, and it should be dropped.
Show 6 quoted lines
> At the end of my series, the error output for an unknown argument now > looks like this: > > fatal: ambiguous argument 'unexpected-arg': unknown revision or > path not in the working tree. >
That makes sense. Since backfill will now be passing arguments down to the revision walking machinery, falling back to the standard revision parsing error is exactly the right behavior.
> I'm not sure it's worth updating this,
I will drop this patch so it does not get in the way of your work.
Show 5 quoted lines
> but I can incorporate a test > that shows that this is handled. > > Thanks, > -Stolee
Regarding the test, I think it would be worth adding one to your series to explicitly verify the behavior for unexpected arguments, since the error message from the revision walk machinery is less obvious to users than a direct "unknown argument" message. But I will leave that decision to you.
Thanks also to Phillip Wood for the test_grep suggestion, and to Junio for the guidance throughout this thread.
Thanks, Siddharth