diff mbox series

[v10,7/9] ls-tree.c: introduce struct "show_tree_data"

Message ID 3ddffa10278b5814123a5bcd2fd3a531def3ecdc.1641978175.git.dyroneteng@gmail.com (mailing list archive)
State Superseded
Headers show
Series ls-tree: "--object-only" and "--format" opts | expand

Commit Message

Teng Long Jan. 13, 2022, 3:42 a.m. UTC
"show_tree_data" is a struct that packages the necessary fields for
"show_tree()". This commit is a pre-prepared commit for supporting
"--format" option and it does not affect any existing functionality.

Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
 builtin/ls-tree.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

Comments

Ævar Arnfjörð Bjarmason Jan. 13, 2022, 7:03 a.m. UTC | #1
On Thu, Jan 13 2022, Teng Long wrote:

> "show_tree_data" is a struct that packages the necessary fields for
> "show_tree()". This commit is a pre-prepared commit for supporting
> "--format" option and it does not affect any existing functionality.

Is the only reason this is split off from 9/9 because you're injecting a
8/9 commit for the coccinelle rule change, and wanted to find some
logical cut-off between the two?

> Signed-off-by: Teng Long <dyroneteng@gmail.com>

For both this & 9/9 this seems to mostly/substantially be code I wrote
and submitted as part of
https://lore.kernel.org/git/RFC-patch-6.7-eac299f06ff-20211217T131635Z-avarab@gmail.com;

The convention we use in such cases is to retain the "Author" header and
just add your own Signed-off-by to patches you're modifying/splitting
up.
Teng Long Jan. 14, 2022, 9:12 a.m. UTC | #2
On Thu, Jan 13, 2022 at 3:07 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:

> > "show_tree_data" is a struct that packages the necessary fields for
> > "show_tree()". This commit is a pre-prepared commit for supporting
> > "--format" option and it does not affect any existing functionality.
>
> Is the only reason this is split off from 9/9 because you're injecting a
> 8/9 commit for the coccinelle rule change, and wanted to find some
> logical cut-off between the two?

I hope "show_tree()" and "the show_tree_format()" to share this structure,
so I made this a pre-prepared and non-functional commit.

After that, in the 9/9, the structure can be used directly and focus on the
functionality changes. If we merge this commit with 9/9, 9/9 will contain a part
of the changes that let "show_tree()" use the new structure, which has nothing
to do with "show_tree_format()" actually, because we designed them to go
through different execution logic. So, personally, I prefer not to mix them
together.

So, the commit of "show_tree_data()" originally was not for "coccinelle".
The only thing that is certain is that coccinelle also should go before 9/9
I think. With regard to 8/9 and 7/9, I think the current order is OK because
they're not related.
>
> For both this & 9/9 this seems to mostly/substantially be code I wrote
> and submitted as part of
> https://lore.kernel.org/git/RFC-patch-6.7-eac299f06ff-20211217T131635Z-avarab@gmail.com;
>
> The convention we use in such cases is to retain the "Author" header and
> just add your own Signed-off-by to patches you're modifying/splitting
> up.

Oops. Sorry for that, I misunderstood it before and I'll be fixed in
the next path.

Thanks.
diff mbox series

Patch

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index e1a2f8225b..56cc166adb 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -35,6 +35,14 @@  static unsigned int shown_fields;
 #define FIELD_DEFAULT 29 /* 11101 size is not shown to output by default */
 #define FIELD_LONG_DEFAULT  (FIELD_DEFAULT | FIELD_SIZE)
 
+struct show_tree_data {
+	unsigned mode;
+	enum object_type type;
+	const struct object_id *oid;
+	const char *pathname;
+	struct strbuf *base;
+};
+
 static const  char * const ls_tree_usage[] = {
 	N_("git ls-tree [<options>] <tree-ish> [<path>...]"),
 	NULL
@@ -108,17 +116,15 @@  static enum object_type get_type(unsigned int mode)
 	        : OBJ_BLOB);
 }
 
-static int show_default(const struct object_id *oid, enum object_type type,
-			const char *pathname, unsigned mode,
-			struct strbuf *base)
+static int show_default(struct show_tree_data *data)
 {
-	size_t baselen = base->len;
+	size_t baselen = data->base->len;
 
 	if (shown_fields & FIELD_SIZE) {
 		char size_text[24];
-		if (type == OBJ_BLOB) {
+		if (data->type == OBJ_BLOB) {
 			unsigned long size;
-			if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
+			if (oid_object_info(the_repository, data->oid, &size) == OBJ_BAD)
 				xsnprintf(size_text, sizeof(size_text), "BAD");
 			else
 				xsnprintf(size_text, sizeof(size_text),
@@ -126,18 +132,18 @@  static int show_default(const struct object_id *oid, enum object_type type,
 		} else {
 			xsnprintf(size_text, sizeof(size_text), "-");
 		}
-		printf("%06o %s %s %7s\t", mode, type_name(type),
-		find_unique_abbrev(oid, abbrev), size_text);
+		printf("%06o %s %s %7s\t", data->mode, type_name(data->type),
+		find_unique_abbrev(data->oid, abbrev), size_text);
 	} else {
-		printf("%06o %s %s\t", mode, type_name(type),
-		find_unique_abbrev(oid, abbrev));
+		printf("%06o %s %s\t", data->mode, type_name(data->type),
+		find_unique_abbrev(data->oid, abbrev));
 	}
-	baselen = base->len;
-	strbuf_addstr(base, pathname);
-	write_name_quoted_relative(base->buf,
+	baselen = data->base->len;
+	strbuf_addstr(data->base, data->pathname);
+	write_name_quoted_relative(data->base->buf,
 				   chomp_prefix ? ls_tree_prefix : NULL, stdout,
 				   line_termination);
-	strbuf_setlen(base, baselen);
+	strbuf_setlen(data->base, baselen);
 	return 1;
 }
 
@@ -148,6 +154,14 @@  static int show_tree(const struct object_id *oid, struct strbuf *base,
 	size_t baselen;
 	enum object_type type = get_type(mode);
 
+	struct show_tree_data data = {
+		.mode = mode,
+		.type = type,
+		.oid = oid,
+		.pathname = pathname,
+		.base = base,
+	};
+
 	if (type == OBJ_TREE && show_recursive(base->buf, base->len, pathname))
 		recurse = READ_TREE_RECURSIVE;
 	if (type == OBJ_TREE && recurse && !(ls_options & LS_SHOW_TREES))
@@ -171,7 +185,7 @@  static int show_tree(const struct object_id *oid, struct strbuf *base,
 	}
 
 	if (shown_fields >= FIELD_DEFAULT)
-		show_default(oid, type, pathname, mode, base);
+		show_default(&data);
 
 	return recurse;
 }