diff mbox series

[RFC,06/20] cat-file: remove mark_query from expand_data

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

Commit Message

Olga Telezhnaya Feb. 22, 2019, 4:05 p.m. UTC
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

Comments

Jeff King Feb. 28, 2019, 9:25 p.m. UTC | #1
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
Christian Couder March 3, 2019, 9:41 a.m. UTC | #2
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 mbox series

Patch

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) {