From f31abb421ddd37e9a13e5ffc2e5c5f10f0f0f9b9 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 21 Jul 2025 15:46:42 -0700 Subject: [PATCH] rev-list: update a NEEDSWORK comment The comment is poorly phrased and it in't clear what it wanted to say. Strongly discourage this broken pattern to be copied and pasted to other code paths. Signed-off-by: Junio C Hamano --- builtin/rev-list.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 0984b607bf..baa9ba17d4 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -650,17 +650,21 @@ int cmd_rev_list(int argc, * * Let "--missing" to conditionally set fetch_if_missing. */ + /* - * NEEDSWORK: These loops that attempt to find presence of - * options without understanding that the options they are - * skipping are broken (e.g., it would not know "--grep + * NEEDSWORK: The next loop is utterly broken. It tries to + * notice an option is used, but without understanding if each + * option takes an argument, which fundamentally would not + * work. It would not know "--grep * --exclude-promisor-objects" is not triggering - * "--exclude-promisor-objects" option). We really need - * setup_revisions() to have a mechanism to allow and disallow - * some sets of options for different commands (like rev-list, - * replay, etc). Such a mechanism should do an early parsing - * of options and be able to manage the `--missing=...` and - * `--exclude-promisor-objects` options below. + * "--exclude-promisor-objects" option, for example. + * + * We really need setup_revisions() to have a mechanism to + * allow and disallow some sets of options for different + * commands (like rev-list, replay, etc). Such a mechanism + * should do an early parsing of options and be able to manage + * the `--missing=...` and `--exclude-promisor-objects` + * options below. */ for (i = 1; i < argc; i++) { const char *arg = argv[i]; -- 2.47.2