public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
To: git@vger•kernel.org
Cc: "Taylor Blau" <me@ttaylorr•com>,
	"Teng Long" <dyroneteng@gmail•com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
Subject: [RFC PATCH 1/4] ls-tree: don't use "show_tree_data" for "fast" callbacks
Date: Thu, 17 Nov 2022 14:48:14 +0100	[thread overview]
Message-ID: <RFC-patch-1.4-2d8bcfe2cab-20221117T134528Z-avarab@gmail.com> (raw)
In-Reply-To: <RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com>

As noted in [1] the code that made it in as part of
9c4d58ff2c3 (ls-tree: split up "fast path" callbacks, 2022-03-23) was
a "maybe a good idea, maybe not" RFC-quality patch. I hadn't looked
very carefully at the resulting patterns.

The implementation shared the "struct show_tree_data data", which was
introduced in e81517155e0 (ls-tree: introduce struct "show_tree_data",
2022-03-23) both for use in 455923e0a15 (ls-tree: introduce "--format"
option, 2022-03-23), and because the "fat" callback hadn't been split
up as 9c4d58ff2c3 did.

Now that that's been done we can see that most of what
show_tree_common() was doing could be done lazily by the callbacks
themselves, who in the pre-image were often using an odd mis-match of
their own arguments and those same arguments stuck into the "data"
structure. Let's also have the callers initialize the "type", rather
than grabbing it from the "data" structure afterwards.

1. https://lore.kernel.org/git/cover-0.7-00000000000-20220310T134811Z-avarab@gmail.com/
---
 builtin/ls-tree.c | 44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index c3ea09281af..cbb6782f9a5 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -173,19 +173,11 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 	return recurse;
 }
 
-static int show_tree_common(struct show_tree_data *data, int *recurse,
-			    const struct object_id *oid, struct strbuf *base,
-			    const char *pathname, unsigned mode)
+static int show_tree_common(int *recurse, struct strbuf *base,
+			    const char *pathname, enum object_type type)
 {
-	enum object_type type = object_type(mode);
 	int ret = -1;
-
 	*recurse = 0;
-	data->mode = mode;
-	data->type = type;
-	data->oid = oid;
-	data->pathname = pathname;
-	data->base = base;
 
 	if (type == OBJ_BLOB) {
 		if (ls_options & LS_TREE_ONLY)
@@ -217,15 +209,15 @@ static int show_tree_default(const struct object_id *oid, struct strbuf *base,
 {
 	int early;
 	int recurse;
-	struct show_tree_data data = { 0 };
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
-	printf("%06o %s %s\t", data.mode, type_name(data.type),
-	       find_unique_abbrev(data.oid, abbrev));
-	show_tree_common_default_long(base, pathname, data.base->len);
+	printf("%06o %s %s\t", mode, type_name(object_type(mode)),
+	       find_unique_abbrev(oid, abbrev));
+	show_tree_common_default_long(base, pathname, base->len);
 	return recurse;
 }
 
@@ -235,16 +227,16 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 {
 	int early;
 	int recurse;
-	struct show_tree_data data = { 0 };
 	char size_text[24];
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
-	if (data.type == OBJ_BLOB) {
+	if (type == OBJ_BLOB) {
 		unsigned long size;
-		if (oid_object_info(the_repository, data.oid, &size) == OBJ_BAD)
+		if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
 			xsnprintf(size_text, sizeof(size_text), "BAD");
 		else
 			xsnprintf(size_text, sizeof(size_text),
@@ -253,9 +245,9 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 		xsnprintf(size_text, sizeof(size_text), "-");
 	}
 
-	printf("%06o %s %s %7s\t", data.mode, type_name(data.type),
-	       find_unique_abbrev(data.oid, abbrev), size_text);
-	show_tree_common_default_long(base, pathname, data.base->len);
+	printf("%06o %s %s %7s\t", mode, type_name(type),
+	       find_unique_abbrev(oid, abbrev), size_text);
+	show_tree_common_default_long(base, pathname, base->len);
 	return recurse;
 }
 
@@ -266,9 +258,9 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
 	int early;
 	int recurse;
 	const size_t baselen = base->len;
-	struct show_tree_data data = { 0 };
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
@@ -286,9 +278,9 @@ static int show_tree_object(const struct object_id *oid, struct strbuf *base,
 {
 	int early;
 	int recurse;
-	struct show_tree_data data = { 0 };
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
-- 
2.38.0.1473.g172bcc0511c


  reply	other threads:[~2022-11-17 13:48 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-17 11:30 [RFC PATCH 0/6] ls-tree: introduce '--pattern' option Teng Long
2022-11-17 11:30 ` [RFC PATCH 1/6] ls-tree: cleanup the redundant SPACE Teng Long
2022-11-17 11:30 ` [RFC PATCH 2/6] t3104: remove shift code in 'test_ls_tree_format' Teng Long
2022-11-17 11:30 ` [RFC PATCH 3/6] ls-tree: optimize params of 'show_tree_common_default_long()' Teng Long
2022-11-17 11:30 ` [RFC PATCH 4/6] ls-tree: improving cohension in the print code Teng Long
2022-11-17 13:53   ` Ævar Arnfjörð Bjarmason
2022-11-17 11:30 ` [RFC PATCH 5/6] ls-tree: introduce 'match_pattern()' function Teng Long
2022-11-17 14:02   ` Ævar Arnfjörð Bjarmason
2022-11-30  9:39   ` Ævar Arnfjörð Bjarmason
2022-11-17 11:30 ` [RFC PATCH 6/6] ls-tree: introduce '--pattern' option Teng Long
2022-11-17 14:03   ` Ævar Arnfjörð Bjarmason
2022-12-12  8:32   ` Johannes Schindelin
2022-12-12 23:57     ` Junio C Hamano
2022-12-14  5:27       ` Junio C Hamano
2022-12-14 10:03         ` Ævar Arnfjörð Bjarmason
2022-12-14 10:38           ` Junio C Hamano
2023-03-27 10:37       ` win-test: unknown terminal "xterm-256color", was " Johannes Schindelin
2023-03-27 20:42         ` Junio C Hamano
2023-03-28 18:08           ` Jeff King
2023-03-28 19:31             ` Junio C Hamano
2023-03-28 19:59               ` Jeff King
2023-03-28 20:43                 ` Jeff King
2023-03-28 21:05                   ` Junio C Hamano
2022-11-17 13:22 ` [RFC PATCH 0/6] " Ævar Arnfjörð Bjarmason
2022-11-17 22:02   ` Taylor Blau
2022-11-21 11:41     ` Teng Long
2022-11-21 12:12       ` Ævar Arnfjörð Bjarmason
2022-11-17 13:48 ` [RFC PATCH 0/4] ls-tree: pass state in struct, not globals Ævar Arnfjörð Bjarmason
2022-11-17 13:48   ` Ævar Arnfjörð Bjarmason [this message]
2022-12-21 11:47     ` [RFC PATCH 1/4] ls-tree: don't use "show_tree_data" for "fast" callbacks Teng Long
2022-11-17 13:48   ` [RFC PATCH 2/4] ls-tree: use a "struct options" Ævar Arnfjörð Bjarmason
2022-11-17 13:48   ` [RFC PATCH 3/4] ls-tree: fold "show_tree_data" into "cb" struct Ævar Arnfjörð Bjarmason
2022-11-17 13:48   ` [RFC PATCH 4/4] ls-tree: make "line_termination" less generic Ævar Arnfjörð Bjarmason
2022-11-21 12:00   ` [RFC PATCH 0/4] ls-tree: pass state in struct, not globals Teng Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=RFC-patch-1.4-2d8bcfe2cab-20221117T134528Z-avarab@gmail.com \
    --to=avarab@gmail$(echo .)com \
    --cc=dyroneteng@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=me@ttaylorr$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox