Message ID | 0102016915f49a4f-f02a6509-a3ba-41b0-b768-3d8ba116f526-000000@eu-west-1.amazonses.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [RFC,01/20] cat-file: reuse struct ref_format | expand |
On Fri, Feb 22, 2019 at 04:05:45PM +0000, Olga Telezhnaya wrote: > Get rid of mark_query field in struct expand_data. > expand_data may be global further as we use it in ref-filter also, > so we need to remove cat-file specific fields from it. > > All globals that I add through this patch will be deleted in the end, > so treat it just as the middle step. So this is a similar situation to the split_on_whitespace thing we have in the previous patch. I think many of my comments there could apply here. I.e., do we need to be removing them from expand_data now, instead of just moving the bits from expand_data over to ref-filter? But if we assume for a moment that doing it that way isn't feasible (or at least isn't as easy as this way), then I think what this patch does is preferable to the previous one. By making it a global variable, we can still interact with it from the expand callback, even if it's not part of expand_data(). So the previous patch could make "split_on_whitespace" a global, and then continue to set it from expand_atom() as the current code does. -Peff
On Fri, Feb 22, 2019 at 5:07 PM Olga Telezhnaya <olyatelezhnaya@gmail.com> wrote: > > Get rid of mark_query field in struct expand_data. > expand_data may be global further as we use it in ref-filter also, > so we need to remove cat-file specific fields from it. > > All globals that I add through this patch will be deleted in the end, > so treat it just as the middle step. > > Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com> > --- > builtin/cat-file.c | 22 +++++++++------------- > 1 file changed, 9 insertions(+), 13 deletions(-) > > diff --git a/builtin/cat-file.c b/builtin/cat-file.c > index 60f3839b06f8c..9bcb02fad1f0d 100644 > --- a/builtin/cat-file.c > +++ b/builtin/cat-file.c > @@ -29,6 +29,8 @@ struct batch_options { > }; > > static const char *force_path; > +/* Will be deleted at the end of this patch */ When this patch is committed, there is no patch anymore, only a commit. And of course the variable will be deleted in a following commit. So instead I'd rather see something like: /* Will be deleted in a following commit */ or maybe: /* TODO: delete this in a following commit */
diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 60f3839b06f8c..9bcb02fad1f0d 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -29,6 +29,8 @@ struct batch_options { }; static const char *force_path; +/* Will be deleted at the end of this patch */ +static int mark_query; static int filter_object(const char *path, unsigned mode, const struct object_id *oid, @@ -197,12 +199,6 @@ struct expand_data { const char *rest; struct object_id delta_base_oid; - /* - * If mark_query is true, we do not expand anything, but rather - * just mark the object_info with items we wish to query. - */ - int mark_query; - /* * After a mark_query run, this object_info is set up to be * passed to oid_object_info_extended. It will point to the data @@ -230,20 +226,20 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, struct expand_data *data = vdata; if (is_atom("objectname", atom, len)) { - if (!data->mark_query) + if (!mark_query) strbuf_addstr(sb, oid_to_hex(&data->oid)); } else if (is_atom("objecttype", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.typep = &data->type; else strbuf_addstr(sb, type_name(data->type)); } else if (is_atom("objectsize", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.sizep = &data->size; else strbuf_addf(sb, "%"PRIuMAX , (uintmax_t)data->size); } else if (is_atom("objectsize:disk", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.disk_sizep = &data->disk_size; else strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size); @@ -251,7 +247,7 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, if (data->rest) strbuf_addstr(sb, data->rest); } else if (is_atom("deltabase", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.delta_base_sha1 = data->delta_base_oid.hash; else strbuf_addstr(sb, @@ -490,9 +486,9 @@ static int batch_objects(struct batch_options *opt) * object. */ memset(&data, 0, sizeof(data)); - data.mark_query = 1; + mark_query = 1; strbuf_expand(&output, opt->format.format, expand_format, &data); - data.mark_query = 0; + mark_query = 0; strbuf_release(&output); if (opt->all_objects) {
Get rid of mark_query field in struct expand_data. expand_data may be global further as we use it in ref-filter also, so we need to remove cat-file specific fields from it. All globals that I add through this patch will be deleted in the end, so treat it just as the middle step. Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com> --- builtin/cat-file.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) -- https://github.com/git/git/pull/568