{"thread":{"id":"65320","subject":"[PATCH] backfill: handle unexpected arguments","startedAt":"2026-03-21T03:16:51Z","lastAt":"2026-03-23T06:18:30Z","messageCount":12,"participants":["Siddharth Shrimali","Junio C Hamano","Phillip Wood","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"539584","messageId":"20260321031643.5185-1-r.siddharth.shrimali@gmail.com","threadId":"65320","inReplyTo":null,"subject":"[PATCH] backfill: handle unexpected arguments","fromName":"Siddharth Shrimali","fromEmail":"r.siddharth.shrimali@gmail.com","sentAt":"2026-03-21T03:16:43Z","receivedAt":"2026-03-21T03:16:51Z","isPatch":true,"sender":{"key":"r.siddharth.shrimali@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183274193?v=4"},"body":"git backfill takes no non-option arguments. However, if extra\narguments are passed with git backfill, parse_options() leaves\nthem in argc and the command currently ignores them silently,\ngiving the user no indication that something is wrong.\n\nAdd a check after parse_options() to call usage_with_options()\nif any unexpected arguments remain. This prints the correct usage\nand exits with an error, consistent with how other Git commands\nsuch as git-gc and git-repack handle this situation.\n\nSigned-off-by: Siddharth Shrimali \n---\n builtin/backfill.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex e9a33e81be..0eb171478a 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -135,6 +135,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \n \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n \t\t\t 0);\n+\t\n+\tif (argc)\n+\t\tusage_with_options(builtin_backfill_usage, options);\n \n \trepo_config(repo, git_default_config, NULL);\n \n-- \n2.51.2\n\n"},{"id":"539588","messageId":"xmqqtsu9dc9m.fsf@gitster.g","threadId":"65320","inReplyTo":"20260321031643.5185-1-r.siddharth.shrimali@gmail.com","subject":"Re: [PATCH] backfill: handle unexpected arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-21T04:42:13Z","receivedAt":"2026-03-21T04:42:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Shrimali writes:\n\n> git backfill takes no non-option arguments. However, if extra\n> arguments are passed with git backfill, parse_options() leaves\n> them in argc and the command currently ignores them silently,\n> giving the user no indication that something is wrong.\n\nWell written. If you drop unnecessary \"currently\", it would be\nperfect ;-).\n\n> Add a check after parse_options() to call usage_with_options()\n> if any unexpected arguments remain. This prints the correct usage\n> and exits with an error, consistent with how other Git commands\n> such as git-gc and git-repack handle this situation.\n\nI am not sure if this is a good idea.\n\nWhen parse_options() finds an unrecognised option, you would get\nusage-with-options help, so without explicitly telling the user\n\"Hey, you have an extra argument that I do not expect at the end of\nthe command line\" and giving only the same usage-with-options help,\nthe user would not know why they are seeing the help message, as it\nis totally unclear what mistake they made in their command line.\n\n\"git bugreport\" is also a command that does not take any positional\narguments on its command line. Study how it complains about an\nunwanted argument, and follow its example, perhaps?\n\nThanks.\n\n> Signed-off-by: Siddharth Shrimali \n> ---\n> builtin/backfill.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/builtin/backfill.c b/builtin/backfill.c\n> index e9a33e81be..0eb171478a 100644\n> --- a/builtin/backfill.c\n> +++ b/builtin/backfill.c\n> @@ -135,6 +135,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> \n> \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n> \t\t\t 0);\n> +\t\n> +\tif (argc)\n> +\t\tusage_with_options(builtin_backfill_usage, options);\n> \n> \trepo_config(repo, git_default_config, NULL);\n"},{"id":"539592","messageId":"xmqq341tdbal.fsf@gitster.g","threadId":"65320","inReplyTo":"xmqqtsu9dc9m.fsf@gitster.g","subject":"Re: [PATCH] backfill: handle unexpected arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-21T05:03:14Z","receivedAt":"2026-03-21T05:03:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I am not sure if this is a good idea.\n>\n> When parse_options() finds an unrecognised option, you would get\n> usage-with-options help, so without explicitly telling the user\n> \"Hey, you have an extra argument that I do not expect at the end of\n> the command line\" and giving only the same usage-with-options help,\n> the user would not know why they are seeing the help message, as it\n> is totally unclear what mistake they made in their command line.\n>\n> \"git bugreport\" is also a command that does not take any positional\n> arguments on its command line. Study how it complains about an\n> unwanted argument, and follow its example, perhaps?\n>\n> Thanks.\n>\n>> Signed-off-by: Siddharth Shrimali \n>> ---\n>> builtin/backfill.c | 3 +++\n>> 1 file changed, 3 insertions(+)\n\nOne thing I forgot. You may want to add a test for this.\n"},{"id":"539608","messageId":"20260321174730.34762-1-r.siddharth.shrimali@gmail.com","threadId":"65320","inReplyTo":"xmqq341tdbal.fsf@gitster.g","subject":"[PATCH v2] backfill: handle unexpected arguments","fromName":"Siddharth Shrimali","fromEmail":"r.siddharth.shrimali@gmail.com","sentAt":"2026-03-21T17:47:30Z","receivedAt":"2026-03-21T17:47:39Z","isPatch":true,"sender":{"key":"r.siddharth.shrimali@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183274193?v=4"},"body":"git backfill takes no non-option arguments. However, if extra\narguments are passed with git backfill, parse_options() leaves\nthem in argc and the command ignores them silently, giving the\nuser no indication that something is wrong.\n\nAdd a check after parse_options() to call usage_with_options()\nif any unexpected arguments remain. To ensure the user understands\nwhy the command failed, print an error message specifying the unknown\nargument before showing the usage string. This is consistent with how\nother Git commands such as git-bugreport handle this situation.\n\nAlso, add a test in t5620 to ensure the unexpected arguments are\nrejected with the correct error message.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Siddharth Shrimali \n---\nChanges in v2:\n- Dropped the word \"currently\" from the commit message as per \n Junio's feedback.\n- Added an `error()` call before `usage_with_options()` to state which \n argument was unknown, following the pattern in `git bugreport`.\n- Added a test case in `t5620-backfill.sh` to verify the new error output.\n\n builtin/backfill.c | 5 +++++\n t/t5620-backfill.sh | 5 +++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex e9a33e81be..5a333afde0 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -135,6 +135,11 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \n \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n \t\t\t 0);\n+\t\n+\tif (argc) {\n+\t\terror(_(\"unknown argument `%s'\"), argv[0]);\n+\t\tusage_with_options(builtin_backfill_usage, options);\n+\t}\n \n \trepo_config(repo, git_default_config, NULL);\n \ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 58c81556e7..d74e1be74b 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -176,6 +176,11 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '\n \ttest_line_count = 12 missing\n '\n \n+test_expect_success 'backfill rejects unexpected arguments' '\n+\ttest_must_fail git -C backfill1 backfill unexpected-arg 2>err &&\n+\tgrep \"unknown argument .*unexpected-arg\" err\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n \n-- \n2.51.2\n\n"},{"id":"539622","messageId":"xmqqfr5sacps.fsf@gitster.g","threadId":"65320","inReplyTo":"20260321174730.34762-1-r.siddharth.shrimali@gmail.com","subject":"Re: [PATCH v2] backfill: handle unexpected arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-22T01:13:03Z","receivedAt":"2026-03-22T01:13:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Shrimali writes:\n\n> git backfill takes no non-option arguments. However, if extra\n> arguments are passed with git backfill, parse_options() leaves\n> them in argc and the command ignores them silently, giving the\n> user no indication that something is wrong.\n>\n> Add a check after parse_options() to call usage_with_options()\n> if any unexpected arguments remain. To ensure the user understands\n> why the command failed, print an error message specifying the unknown\n> argument before showing the usage string. This is consistent with how\n> other Git commands such as git-bugreport handle this situation.\n\nWhile it is a thing to aim for, I doubt the updated behaviour is\nconsistent with how bugreport behaves.\n\n\t$ git bugreport extra-arg\n error: unknown argument `extra-arg'\n usage: git bugreport [(-o | --output-directory) ]\n [(-s | --suffix) | --no-suffix]\n [--diagnose[=]]\n\nI.e. the command does not show the list of options with descriptions\nto the user, who did not make any mistake in specifying any dashed\noption. On the other hand,\n\n\t$ git bugreport --mistyped-option\n\nwould give usage_with_options(), because unlike the case where the\nuser gave only an extra argument, it is clear in this case that the\nuser failed to choose the right option, which might be helped if we\ntell them what each option does. Also with\n\n\t$ git bugreport --suffix\n\nthe user will get a more targetted help that is specific to the\noption given incorrectly.\n\nCompared to that, the way the updated code would react to\n\n\t$ git backfill extra-arg\n\ncannot be called \"consistent with how\" bugreport handles this\nsituation, I would have to say. I didn't check how the command\nbehaves in the other two cases (i.e., option that does not exist,\nand an option that needs a value but the user forgot to give one)\n\n\n> diff --git a/builtin/backfill.c b/builtin/backfill.c\n> index e9a33e81be..5a333afde0 100644\n> --- a/builtin/backfill.c\n> +++ b/builtin/backfill.c\n> @@ -135,6 +135,11 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> \n> \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n> \t\t\t 0);\n> +\t\n> +\tif (argc) {\n> +\t\terror(_(\"unknown argument `%s'\"), argv[0]);\n\nYikes. Please do not add more uses of cute quotes.\n\nPerhaps we found a microproject target for the next intern season?\nLet's not spread cute quotes any more than what we already have, and\nclean existing up ones before it becomes too late.\n\n $ git grep -e \"'%s'\" \\*.c | wc -l\n 2012\n $ git grep -e \"\\`%s'\" \\*.c | wc -l\n 16\n\n> +\t\tusage_with_options(builtin_backfill_usage, options);\n> +\t}\n> \n> \trepo_config(repo, git_default_config, NULL);\n> \n> diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\n> index 58c81556e7..d74e1be74b 100755\n> --- a/t/t5620-backfill.sh\n> +++ b/t/t5620-backfill.sh\n> @@ -176,6 +176,11 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '\n> \ttest_line_count = 12 missing\n> '\n> \n> +test_expect_success 'backfill rejects unexpected arguments' '\n> +\ttest_must_fail git -C backfill1 backfill unexpected-arg 2>err &&\n> +\tgrep \"unknown argument .*unexpected-arg\" err\n\nThe test is a bit underspecified, isn't it?\n\nThis test will still pass if the code called usage_with_options() to\nshow the list of option descriptions after the short usage, instead\nof showing just the short usage by calling usage().\n\nAs with any test, it is important to test that the output has what\nwe expect to see, but at the same time we need to ensure that the\noutput does not have what we do not want to see.\n\n> +'\n> +\n> . \"$TEST_DIRECTORY\"/lib-httpd.sh\n> start_httpd\n"},{"id":"539633","messageId":"20260322053207.60992-1-r.siddharth.shrimali@gmail.com","threadId":"65320","inReplyTo":"xmqqfr5sacps.fsf@gitster.g","subject":"[PATCH v3] backfill: handle unexpected arguments","fromName":"Siddharth Shrimali","fromEmail":"r.siddharth.shrimali@gmail.com","sentAt":"2026-03-22T05:32:07Z","receivedAt":"2026-03-22T05:32:16Z","isPatch":true,"sender":{"key":"r.siddharth.shrimali@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183274193?v=4"},"body":"git backfill takes no non-option arguments. However, if extra\narguments are passed with git backfill, parse_options() leaves\nthem in argc and the command ignores them silently, giving the\nuser no indication that something is wrong.\n\nAdd a check after parse_options() to report an error if any unexpected\narguments remain. To ensure the user understands why the command\nfailed, print an error message specifying the unknown argument\nfollowed by the short usage string. This matches the behavior of\nother Git commands such as git bugreport.\n\nAlso, add a test in t5620 to ensure the unexpected arguments are\nrejected with the correct error message and that the full option\ndescriptions are not printed.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Siddharth Shrimali \n---\nChanges since v2:\n- Replaced the backtick (`%s') with standard single quotes ('%s').\n- Swapped `usage_with_options()` for `usage(builtin_backfill_usage[0])` \n so the user only sees the short usage string instead of the full \n option descriptions.\n- Updated the test to also verify that the full option descriptions \n are not printed.\n\n builtin/backfill.c | 5 +++++\n t/t5620-backfill.sh | 6 ++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex e9a33e81be..6d90db3da0 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -135,6 +135,11 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \n \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n \t\t\t 0);\n+\t\n+\tif (argc) {\n+\t\terror(_(\"unknown argument '%s'\"), argv[0]);\n+\t\tusage(builtin_backfill_usage[0]);\n+\t}\n \n \trepo_config(repo, git_default_config, NULL);\n \ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 58c81556e7..3f1eeb67e8 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -176,6 +176,12 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '\n \ttest_line_count = 12 missing\n '\n \n+test_expect_success 'backfill rejects unexpected arguments' '\n+\ttest_must_fail git -C backfill1 backfill unexpected-arg >err 2>&1 &&\n+\tgrep \"unknown argument .*unexpected-arg\" err &&\n+\t! grep \"Minimum number of objects\" err\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n \n-- \n2.51.2\n\n"},{"id":"539653","messageId":"45a949f3-8b90-4046-995f-da1df265abfe@gmail.com","threadId":"65320","inReplyTo":"20260322053207.60992-1-r.siddharth.shrimali@gmail.com","subject":"Re: [PATCH v3] backfill: handle unexpected arguments","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-22T16:38:03Z","receivedAt":"2026-03-22T16:38:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 22/03/2026 05:32, Siddharth Shrimali wrote:\n> git backfill takes no non-option arguments. However, if extra\n> arguments are passed with git backfill, parse_options() leaves\n> them in argc and the command ignores them silently, giving the\n> user no indication that something is wrong.\n> \n> Add a check after parse_options() to report an error if any unexpected\n> arguments remain. To ensure the user understands why the command\n> failed, print an error message specifying the unknown argument\n> followed by the short usage string. This matches the behavior of\n> other Git commands such as git bugreport.\n> \n> Also, add a test in t5620 to ensure the unexpected arguments are\n> rejected with the correct error message and that the full option\n> descriptions are not printed.\n\nNicely explained\n\n> diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\n> index 58c81556e7..3f1eeb67e8 100755\n> --- a/t/t5620-backfill.sh\n> +++ b/t/t5620-backfill.sh\n> @@ -176,6 +176,12 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '\n> \ttest_line_count = 12 missing\n> '\n> \n> +test_expect_success 'backfill rejects unexpected arguments' '\n> +\ttest_must_fail git -C backfill1 backfill unexpected-arg >err 2>&1 &&\n> +\tgrep \"unknown argument .*unexpected-arg\" err &&\n> +\t! grep \"Minimum number of objects\" err\n\nUsing test_grep would make test failures easier to debug as it prints a \ndiagnostic message if it fails. Note that \"! grep\" should become \n\"test_grep !\" to ensure the diagnostic message is printed when the \nexpression matches.\n\nThanks\n\nPhillip\n\n\n> +'\n> +\n> . \"$TEST_DIRECTORY\"/lib-httpd.sh\n> start_httpd\n> \n\n"},{"id":"539660","messageId":"xmqqa4vz91sk.fsf@gitster.g","threadId":"65320","inReplyTo":"45a949f3-8b90-4046-995f-da1df265abfe@gmail.com","subject":"Re: [PATCH v3] backfill: handle unexpected arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-22T18:06:35Z","receivedAt":"2026-03-22T18:06:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>> +test_expect_success 'backfill rejects unexpected arguments' '\n>> +\ttest_must_fail git -C backfill1 backfill unexpected-arg >err 2>&1 &&\n>> +\tgrep \"unknown argument .*unexpected-arg\" err &&\n>> +\t! grep \"Minimum number of objects\" err\n>\n> Using test_grep would make test failures easier to debug as it prints a \n> diagnostic message if it fails. Note that \"! grep\" should become \n> \"test_grep !\" to ensure the diagnostic message is printed when the \n> expression matches.\n>\n> Thanks\n\nGreat suggestion. Thanks.\n"},{"id":"539678","messageId":"d8e6f854-e838-439f-bc5a-27cbb4091e4f@gmail.com","threadId":"65320","inReplyTo":"20260322053207.60992-1-r.siddharth.shrimali@gmail.com","subject":"Re: [PATCH v3] backfill: handle unexpected arguments","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-22T23:01:33Z","receivedAt":"2026-03-22T23:01:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/22/26 1:32 AM, Siddharth Shrimali wrote:\n> git backfill takes no non-option arguments. However, if extra\n> arguments are passed with git backfill, parse_options() leaves\n> them in argc and the command ignores them silently, giving the\n> user no indication that something is wrong.\n\n> @@ -135,6 +135,11 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> \n> \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n> \t\t\t 0);\n> +\t\n> +\tif (argc) {\n> +\t\terror(_(\"unknown argument '%s'\"), argv[0]);\n> +\t\tusage(builtin_backfill_usage[0]);\n> +\t}\n\nBefore we get too far into this: How does this interact with\nthe ongoing change to introduce revision arguments to 'git\nbackfill' [1]? I suppose that the important bit would be that\nwe still parse arguments using the revision walk machinery at\nsome point, but it would be difficult to guarantee that when\nworking on a patch disconnected from that series.\n\n[1] https://lore.kernel.org/git/pull.2070.git.1773707361.gitgitgadget@gmail.com/\n\nThanks,\n-Stolee\n\n"},{"id":"539682","messageId":"xmqqa4vz7400.fsf@gitster.g","threadId":"65320","inReplyTo":"d8e6f854-e838-439f-bc5a-27cbb4091e4f@gmail.com","subject":"Re: [PATCH v3] backfill: handle unexpected arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-23T01:01:51Z","receivedAt":"2026-03-23T01:01:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n>> +\tif (argc) {\n>> +\t\terror(_(\"unknown argument '%s'\"), argv[0]);\n>> +\t\tusage(builtin_backfill_usage[0]);\n>> +\t}\n>\n> Before we get too far into this: How does this interact with\n> the ongoing change to introduce revision arguments to 'git\n> backfill' [1]?\n\nAhh, that one completely slipped my mind.\n\nThanks for a doze of sanity. This patch becomes completely\nirrelevant if we are taking command line arguments.\n\nIt will become the responsibility of the other topic to detect and\ncomplain about excess command line parameters (unless the feature it\nadds absorbs all of them, which may be the case).\n\n> [1] https://lore.kernel.org/git/pull.2070.git.1773707361.gitgitgadget@gmail.com/\n"},{"id":"539684","messageId":"6460601f-ff72-4683-abd1-2ae4c8352a27@gmail.com","threadId":"65320","inReplyTo":"xmqqa4vz7400.fsf@gitster.g","subject":"Re: [PATCH v3] backfill: handle unexpected arguments","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-23T01:42:21Z","receivedAt":"2026-03-23T01:42:24Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/22/26 9:01 PM, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n>>> +\tif (argc) {\n>>> +\t\terror(_(\"unknown argument '%s'\"), argv[0]);\n>>> +\t\tusage(builtin_backfill_usage[0]);\n>>> +\t}\n>>\n>> Before we get too far into this: How does this interact with\n>> the ongoing change to introduce revision arguments to 'git\n>> backfill' [1]?\n> \n> Ahh, that one completely slipped my mind.\n> \n> Thanks for a doze of sanity. This patch becomes completely\n> irrelevant if we are taking command line arguments.\n> \n> It will become the responsibility of the other topic to detect and\n> complain about excess command line parameters (unless the feature it\n> adds absorbs all of them, which may be the case).\n\nAt the end of my series, the error output for an unknown argument now\nlooks like this:\n\n fatal: ambiguous argument 'unexpected-arg': unknown revision or\n path not in the working tree.\n\nI'm not sure it's worth updating this, but I can incorporate a test\nthat shows that this is handled.\n\nThanks,\n-Stolee\n"},{"id":"539699","messageId":"CAGWgyh8E-=A+NKEAOz3xHPq96rf3j0tt7++3xvg7NoMCKq_Www@mail.gmail.com","threadId":"65320","inReplyTo":"6460601f-ff72-4683-abd1-2ae4c8352a27@gmail.com","subject":"Re: [PATCH v3] backfill: handle unexpected arguments","fromName":"Siddharth Shrimali","fromEmail":"r.siddharth.shrimali@gmail.com","sentAt":"2026-03-23T06:17:52Z","receivedAt":"2026-03-23T06:18:30Z","isPatch":true,"sender":{"key":"r.siddharth.shrimali@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183274193?v=4"},"body":"On Mon, 23 Mar 2026 at 07:12, Derrick Stolee wrote:\n>\n> On 3/22/26 9:01 PM, Junio C Hamano wrote:\n> > Derrick Stolee writes:\n> >\n> >>> + if (argc) {\n> >>> + error(_(\"unknown argument '%s'\"), argv[0]);\n> >>> + usage(builtin_backfill_usage[0]);\n> >>> + }\n> >>\n> >> Before we get too far into this: How does this interact with\n> >> the ongoing change to introduce revision arguments to 'git\n> >> backfill' [1]?\n> >\n> > Ahh, that one completely slipped my mind.\n> >\n> > Thanks for a doze of sanity. This patch becomes completely\n> > irrelevant if we are taking command line arguments.\n> >\n> > It will become the responsibility of the other topic to detect and\n> > complain about excess command line parameters (unless the feature it\n> > adds absorbs all of them, which may be the case).\n>\n\nThank you for pointing this out, and apologies for missing the in-flight series.\nI agree that this patch becomes irrelevant given the revision arguments work,\nand it should be dropped.\n\n> At the end of my series, the error output for an unknown argument now\n> looks like this:\n>\n> fatal: ambiguous argument 'unexpected-arg': unknown revision or\n> path not in the working tree.\n>\nThat makes sense. Since backfill will now be passing arguments down to\nthe revision walking machinery, falling back to the standard revision parsing\nerror is exactly the right behavior.\n\n> I'm not sure it's worth updating this,\n\nI will drop this patch so it does not get in the way of your work.\n\n> but I can incorporate a test\n> that shows that this is handled.\n>\n> Thanks,\n> -Stolee\n\nRegarding the test, I think it would be worth adding one to your series to\nexplicitly verify the behavior for unexpected arguments, since the error message\nfrom the revision walk machinery is less obvious to users than a direct\n\"unknown argument\" message. But I will leave that decision to you.\n\n\nThanks also to Phillip Wood for the test_grep suggestion, and to Junio for the\nguidance throughout this thread.\n\nThanks,\nSiddharth\n"}]}