| Message ID | 86df0c9e4df34566c10870e06865af536504a6af.1643915286.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add cat-file --batch-command flag | expand |
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: John Cai <johncai86@gmail.com> > > To prepare for a new flag --batch-command, we will add a flag that > indicates whether or not an interactive command mode will be used > that reads commands and arguments off of stdin. > > An intuitive name for this flag would be "command", which can get > confusing with the already existing cmdmode. I am moderately against this change. "mode" is too vague a word (i.e. it does not answer "mode of what?" question with the name), and renaming it to "mode" is probably a regression in readability. The original name "cmdmode" is not all that better, either, as I am sure that it was named after the irrelevant implementation detail that it is originally read using the OPT_CMDMODE() feature of the parse-options machinery, and did not mean to express mode of doing "what" with the name. So, let's think aloud what this "mode" is about. The feature lets the batch operation to choose how the blob objects are mangled [*], so "mangle-mode", "filter-mode", etc. (or find and use a better verb than these) would be an improvement over the original "cmdmode". [Footnote] * 'w' to apply the smudge filter to make them as if they were written to the working tree, 'c' to apply the textconv filter, or 0 to stream them as-is.
On Thu, Feb 03 2022, John Cai via GitGitGadget wrote: > From: John Cai <johncai86@gmail.com> > > To prepare for a new flag --batch-command, we will add a flag that > indicates whether or not an interactive command mode will be used > that reads commands and arguments off of stdin. > > An intuitive name for this flag would be "command", which can get > confusing with the already existing cmdmode. > > Rename cmdmode->mode to prepare for this change. > > Signed-off-by: John Cai <johncai86@gmail.com> > --- > builtin/cat-file.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/builtin/cat-file.c b/builtin/cat-file.c > index d94050e6c18..858bca208ff 100644 > --- a/builtin/cat-file.c > +++ b/builtin/cat-file.c > @@ -24,7 +24,7 @@ struct batch_options { > int buffer_output; > int all_objects; > int unordered; > - int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */ > + int mode; /* may be 'w' or 'c' for --filters or --textconv */ > const char *format; > }; > > @@ -306,19 +306,19 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d > if (data->type == OBJ_BLOB) { > if (opt->buffer_output) > fflush(stdout); > - if (opt->cmdmode) { > + if (opt->mode) { > char *contents; > unsigned long size; > > if (!data->rest) > die("missing path for '%s'", oid_to_hex(oid)); > > - if (opt->cmdmode == 'w') { > + if (opt->mode == 'w') { > if (filter_object(data->rest, 0100644, oid, > &contents, &size)) > die("could not convert '%s' %s", > oid_to_hex(oid), data->rest); > - } else if (opt->cmdmode == 'c') { > + } else if (opt->mode == 'c') { > enum object_type type; > if (!textconv_object(the_repository, > data->rest, 0100644, oid, > @@ -330,7 +330,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d > die("could not convert '%s' %s", > oid_to_hex(oid), data->rest); > } else > - BUG("invalid cmdmode: %c", opt->cmdmode); > + BUG("invalid mode: %c", opt->mode); > batch_write(opt, contents, size); > free(contents); > } else { > @@ -533,7 +533,7 @@ static int batch_objects(struct batch_options *opt) > strbuf_expand(&output, opt->format, expand_format, &data); > data.mark_query = 0; > strbuf_release(&output); > - if (opt->cmdmode) > + if (opt->mode) > data.split_on_whitespace = 1; > > /* > @@ -695,10 +695,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) > > batch.buffer_output = -1; > argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0); > - > if (opt) { > if (batch.enabled && (opt == 'c' || opt == 'w')) > - batch.cmdmode = opt; > + batch.mode = opt; > else if (argc == 1) > obj_name = argv[0]; > else > @@ -712,9 +711,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) > usage_with_options(cat_file_usage, options); > } > if (batch.enabled) { > - if (batch.cmdmode != opt || argc) > + if (batch.mode != opt || argc) > usage_with_options(cat_file_usage, options); > - if (batch.cmdmode && batch.all_objects) > + if (batch.mode && batch.all_objects) > die("--batch-all-objects cannot be combined with " > "--textconv nor with --filters"); > } There's a rather major rewrite in-flight to this codepath in ab/cat-file. I think it would be better to base this on top of that. Per Junio's latest "What's Cooking" that may be getting merged down today or tomorrow.
diff --git a/builtin/cat-file.c b/builtin/cat-file.c index d94050e6c18..858bca208ff 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -24,7 +24,7 @@ struct batch_options { int buffer_output; int all_objects; int unordered; - int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */ + int mode; /* may be 'w' or 'c' for --filters or --textconv */ const char *format; }; @@ -306,19 +306,19 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d if (data->type == OBJ_BLOB) { if (opt->buffer_output) fflush(stdout); - if (opt->cmdmode) { + if (opt->mode) { char *contents; unsigned long size; if (!data->rest) die("missing path for '%s'", oid_to_hex(oid)); - if (opt->cmdmode == 'w') { + if (opt->mode == 'w') { if (filter_object(data->rest, 0100644, oid, &contents, &size)) die("could not convert '%s' %s", oid_to_hex(oid), data->rest); - } else if (opt->cmdmode == 'c') { + } else if (opt->mode == 'c') { enum object_type type; if (!textconv_object(the_repository, data->rest, 0100644, oid, @@ -330,7 +330,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d die("could not convert '%s' %s", oid_to_hex(oid), data->rest); } else - BUG("invalid cmdmode: %c", opt->cmdmode); + BUG("invalid mode: %c", opt->mode); batch_write(opt, contents, size); free(contents); } else { @@ -533,7 +533,7 @@ static int batch_objects(struct batch_options *opt) strbuf_expand(&output, opt->format, expand_format, &data); data.mark_query = 0; strbuf_release(&output); - if (opt->cmdmode) + if (opt->mode) data.split_on_whitespace = 1; /* @@ -695,10 +695,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) batch.buffer_output = -1; argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0); - if (opt) { if (batch.enabled && (opt == 'c' || opt == 'w')) - batch.cmdmode = opt; + batch.mode = opt; else if (argc == 1) obj_name = argv[0]; else @@ -712,9 +711,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) usage_with_options(cat_file_usage, options); } if (batch.enabled) { - if (batch.cmdmode != opt || argc) + if (batch.mode != opt || argc) usage_with_options(cat_file_usage, options); - if (batch.cmdmode && batch.all_objects) + if (batch.mode && batch.all_objects) die("--batch-all-objects cannot be combined with " "--textconv nor with --filters"); }