[v3,2/3] cat-file: introduce batch_command enum to replace print_contents

Commit Message

John Cai Feb. 8, 2022, 8:58 p.m. UTC

From: John Cai <[e-mail]>

The next patch introduces a new --batch-command flag. Including --batch
and --batch-check, we will have a total of three batch modes. Currently,
from the batch_options struct's perspective, print_options is the only
member used to distinguish between the different modes. This makes the
code harder to read.

To reduce potential confusion, replace print_contents with an enum to
help readability and clarity.

Helped-by: Junio C Hamano <[e-mail]>
Signed-off-by: John Cai <[e-mail]>
---
 builtin/cat-file.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Junio C Hamano Feb. 8, 2022, 11:43 p.m. UTC | #1

"John Cai via GitGitGadget" <[e-mail]> writes:

> +enum batch_command {
> +	BATCH_COMMAND_CONTENTS,
> +	BATCH_COMMAND_INFO,
> +};
> +
>  struct batch_options {
>  	int enabled;
>  	int follow_symlinks;
> -	int print_contents;
> +	enum batch_command command_mode;
>  	int buffer_output;
>  	int all_objects;
>  	int unordered;
> @@ -386,7 +391,7 @@ static void batch_object_write(const char *obj_name,
>  	strbuf_addch(scratch, '\n');
>  	batch_write(opt, scratch->buf, scratch->len);
>  
> -	if (opt->print_contents) {
> +	if (opt->command_mode == BATCH_COMMAND_CONTENTS) {
>  		print_object_or_die(opt, data);
>  		batch_write(opt, "\n", 1);
>  	}

Nice.

> @@ -536,7 +541,7 @@ static int batch_objects(struct batch_options *opt)
>  	 * If we are printing out the object, then always fill in the type,
>  	 * since we will want to decide whether or not to stream.
>  	 */
> -	if (opt->print_contents)
> +	if (opt->command_mode == BATCH_COMMAND_CONTENTS)
>  		data.info.typep = &data.type;
>  
>  	if (opt->all_objects) {
> @@ -635,7 +640,12 @@ static int batch_option_callback(const struct option *opt,
>  	}
>  
>  	bo->enabled = 1;
> -	bo->print_contents = !strcmp(opt->long_name, "batch");
> +
> +	if (!strcmp(opt->long_name, "batch"))
> +		bo->command_mode = BATCH_COMMAND_CONTENTS;
> +	if (!strcmp(opt->long_name, "batch-check"))
> +		bo->command_mode = BATCH_COMMAND_INFO;

This may want to become if / else if / else cascade, whose last
"else" clause would say

	BUG("%s given to batch-option-callback", opt->long_name);

perhaps, but it is so minor that there is no need to reroll only for
this.

Looking good.

Patch

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5f015e71096..1c673385868 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -17,10 +17,15 @@ 
 #include "object-store.h"
 #include "promisor-remote.h"
 
+enum batch_command {
+	BATCH_COMMAND_CONTENTS,
+	BATCH_COMMAND_INFO,
+};
+
 struct batch_options {
 	int enabled;
 	int follow_symlinks;
-	int print_contents;
+	enum batch_command command_mode;
 	int buffer_output;
 	int all_objects;
 	int unordered;
@@ -386,7 +391,7 @@  static void batch_object_write(const char *obj_name,
 	strbuf_addch(scratch, '\n');
 	batch_write(opt, scratch->buf, scratch->len);
 
-	if (opt->print_contents) {
+	if (opt->command_mode == BATCH_COMMAND_CONTENTS) {
 		print_object_or_die(opt, data);
 		batch_write(opt, "\n", 1);
 	}
@@ -536,7 +541,7 @@  static int batch_objects(struct batch_options *opt)
 	 * If we are printing out the object, then always fill in the type,
 	 * since we will want to decide whether or not to stream.
 	 */
-	if (opt->print_contents)
+	if (opt->command_mode == BATCH_COMMAND_CONTENTS)
 		data.info.typep = &data.type;
 
 	if (opt->all_objects) {
@@ -635,7 +640,12 @@  static int batch_option_callback(const struct option *opt,
 	}
 
 	bo->enabled = 1;
-	bo->print_contents = !strcmp(opt->long_name, "batch");
+
+	if (!strcmp(opt->long_name, "batch"))
+		bo->command_mode = BATCH_COMMAND_CONTENTS;
+	if (!strcmp(opt->long_name, "batch-check"))
+		bo->command_mode = BATCH_COMMAND_INFO;
+
 	bo->format = arg;
 
 	return 0;