diff mbox series

[2/4] rev-list: refactor early option parsing

Message ID 20250310192829.661692-3-jltobler@gmail.com (mailing list archive)
State New
Headers show
Series rev-list: introduce NUL-delimited output mode | expand

Commit Message

Justin Tobler March 10, 2025, 7:28 p.m. UTC
Before invoking `setup_revisions()`, the `--missing` and
`--exclude-promisor-objects` options are parsed early. In a subsequent
commit, another option is added that must be parsed early.

Refactor the code to parse both options in a single early pass.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 builtin/rev-list.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

Comments

Junio C Hamano March 10, 2025, 8:54 p.m. UTC | #1
Justin Tobler <jltobler@gmail.com> writes:

> @@ -639,19 +640,15 @@ int cmd_rev_list(int argc,
>  		if (!strcmp(arg, "--exclude-promisor-objects")) {
>  			fetch_if_missing = 0;
>  			revs.exclude_promisor_objects = 1;
> -			break;
> -		}
> -	}
> -	for (i = 1; i < argc; i++) {
> -		const char *arg = argv[i];
> -		if (skip_prefix(arg, "--missing=", &arg)) {
> -			if (revs.exclude_promisor_objects)
> -				die(_("options '%s' and '%s' cannot be used together"), "--exclude-promisor-objects", "--missing");
> -			if (parse_missing_action_value(arg))
> -				break;
> +		} else if (skip_prefix(arg, "--missing=", &arg)) {
> +			parse_missing_action_value(arg);
>  		}
>  	}

There is a huge NEEDSWORK comment that essentially says that the
above two loops that this patch combines into one is fundamentally
broken.  I suspect that the remaining two patches in this series
would punt and not improve them, but offhand I am not sure if this
change is making it harder to fix them properly easier or harder.
diff mbox series

Patch

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index dcd079c16c..04d9c893b5 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -16,6 +16,7 @@ 
 #include "object-file.h"
 #include "object-store-ll.h"
 #include "pack-bitmap.h"
+#include "parse-options.h"
 #include "log-tree.h"
 #include "graph.h"
 #include "bisect.h"
@@ -639,19 +640,15 @@  int cmd_rev_list(int argc,
 		if (!strcmp(arg, "--exclude-promisor-objects")) {
 			fetch_if_missing = 0;
 			revs.exclude_promisor_objects = 1;
-			break;
-		}
-	}
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (skip_prefix(arg, "--missing=", &arg)) {
-			if (revs.exclude_promisor_objects)
-				die(_("options '%s' and '%s' cannot be used together"), "--exclude-promisor-objects", "--missing");
-			if (parse_missing_action_value(arg))
-				break;
+		} else if (skip_prefix(arg, "--missing=", &arg)) {
+			parse_missing_action_value(arg);
 		}
 	}
 
+	die_for_incompatible_opt2(revs.exclude_promisor_objects,
+				  "--exclude_promisor_objects",
+				  arg_missing_action, "--missing");
+
 	if (arg_missing_action)
 		revs.do_not_die_on_missing_objects = 1;