{"thread":{"id":"59074","subject":"[PATCH 0/6] ls-tree.c: clean-up works","startedAt":"2023-01-12T09:20:13Z","lastAt":"2023-01-12T15:15:41Z","messageCount":10,"participants":["Teng Long","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"470144","messageId":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","threadId":"59074","inReplyTo":null,"subject":"[PATCH 0/6] ls-tree.c: clean-up works","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-01-12T09:11:29Z","receivedAt":"2023-01-12T09:20:13Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long <dyroneteng@gmail.com>\n\nThis patchset aims to do works on refactoring and cleanup\nof ls-tree.c.\n\nThe patches can be viewed as two parts:\n\n      * Part 1: from Ævar Arnfjörð Bjarmason which mentioned in\n      his RFC patchset[1] and Ævar suggests to submit a separate clean-up\n      series. After I read them, I accept them all.\n\n      * Part 2: fÆvarrom me which introduced in my RFC patchset[2] and after\n      I accepted Ævar's patches, I keep two patches from [2], the left\n      patches in [2] may no longer be needed.\n\n[1] https://public-inbox.org/git/RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com/\n[2] https://public-inbox.org/git/20221117113023.65865-1-tenglong.tl@alibaba-inc.com/\n\nThanks.\n\nTeng Long (2):\n  ls-tree: cleanup the redundant SPACE\n  t3104: remove shift code in 'test_ls_tree_format'\n\nÆvar Arnfjörð Bjarmason (4):\n  ls-tree: don't use \"show_tree_data\" for \"fast\" callbacks\n  ls-tree: use a \"struct options\"\n  ls-tree: fold \"show_tree_data\" into \"cb\" struct\n  ls-tree: make \"line_termination\" less generic\n\n builtin/ls-tree.c         | 257 ++++++++++++++++++++++----------------\n t/t3104-ls-tree-format.sh |   1 -\n 2 files changed, 150 insertions(+), 108 deletions(-)\n\n-- \n2.39.0.98.g174eaa53973.dirty\n\n"},{"id":"470145","messageId":"20230112091135.20050-2-tenglong.tl@alibaba-inc.com","threadId":"59074","inReplyTo":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","subject":"[PATCH 1/6] ls-tree: don't use \"show_tree_data\" for \"fast\" callbacks","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-01-12T09:11:30Z","receivedAt":"2023-01-12T09:20:14Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nAs noted in [1] the code that made it in as part of\n9c4d58ff2c3 (ls-tree: split up \"fast path\" callbacks, 2022-03-23) was\na \"maybe a good idea, maybe not\" RFC-quality patch. I hadn't looked\nvery carefully at the resulting patterns.\n\nThe implementation shared the \"struct show_tree_data data\", which was\nintroduced in e81517155e0 (ls-tree: introduce struct \"show_tree_data\",\n2022-03-23) both for use in 455923e0a15 (ls-tree: introduce \"--format\"\noption, 2022-03-23), and because the \"fat\" callback hadn't been split\nup as 9c4d58ff2c3 did.\n\nNow that that's been done we can see that most of what\nshow_tree_common() was doing could be done lazily by the callbacks\nthemselves, who in the pre-image were often using an odd mis-match of\ntheir own arguments and those same arguments stuck into the \"data\"\nstructure. Let's also have the callers initialize the \"type\", rather\nthan grabbing it from the \"data\" structure afterwards.\n\n1. https://lore.kernel.org/git/cover-0.7-00000000000-20220310T134811Z-avarab@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Teng Long <dyronteng@gmail.com>\n---\n builtin/ls-tree.c | 44 ++++++++++++++++++--------------------------\n 1 file changed, 18 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex c3ea09281af..cbb6782f9a5 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -173,19 +173,11 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \treturn recurse;\n }\n \n-static int show_tree_common(struct show_tree_data *data, int *recurse,\n-\t\t\t    const struct object_id *oid, struct strbuf *base,\n-\t\t\t    const char *pathname, unsigned mode)\n+static int show_tree_common(int *recurse, struct strbuf *base,\n+\t\t\t    const char *pathname, enum object_type type)\n {\n-\tenum object_type type = object_type(mode);\n \tint ret = -1;\n-\n \t*recurse = 0;\n-\tdata->mode = mode;\n-\tdata->type = type;\n-\tdata->oid = oid;\n-\tdata->pathname = pathname;\n-\tdata->base = base;\n \n \tif (type == OBJ_BLOB) {\n \t\tif (ls_options & LS_TREE_ONLY)\n@@ -217,15 +209,15 @@ static int show_tree_default(const struct object_id *oid, struct strbuf *base,\n {\n \tint early;\n \tint recurse;\n-\tstruct show_tree_data data = { 0 };\n+\tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&data, &recurse, oid, base, pathname, mode);\n+\tearly = show_tree_common(&recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n-\tprintf(\"%06o %s %s\\t\", data.mode, type_name(data.type),\n-\t       find_unique_abbrev(data.oid, abbrev));\n-\tshow_tree_common_default_long(base, pathname, data.base->len);\n+\tprintf(\"%06o %s %s\\t\", mode, type_name(object_type(mode)),\n+\t       find_unique_abbrev(oid, abbrev));\n+\tshow_tree_common_default_long(base, pathname, base->len);\n \treturn recurse;\n }\n \n@@ -235,16 +227,16 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,\n {\n \tint early;\n \tint recurse;\n-\tstruct show_tree_data data = { 0 };\n \tchar size_text[24];\n+\tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&data, &recurse, oid, base, pathname, mode);\n+\tearly = show_tree_common(&recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n-\tif (data.type == OBJ_BLOB) {\n+\tif (type == OBJ_BLOB) {\n \t\tunsigned long size;\n-\t\tif (oid_object_info(the_repository, data.oid, &size) == OBJ_BAD)\n+\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n \t\t\txsnprintf(size_text, sizeof(size_text), \"BAD\");\n \t\telse\n \t\t\txsnprintf(size_text, sizeof(size_text),\n@@ -253,9 +245,9 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,\n \t\txsnprintf(size_text, sizeof(size_text), \"-\");\n \t}\n \n-\tprintf(\"%06o %s %s %7s\\t\", data.mode, type_name(data.type),\n-\t       find_unique_abbrev(data.oid, abbrev), size_text);\n-\tshow_tree_common_default_long(base, pathname, data.base->len);\n+\tprintf(\"%06o %s %s %7s\\t\", mode, type_name(type),\n+\t       find_unique_abbrev(oid, abbrev), size_text);\n+\tshow_tree_common_default_long(base, pathname, base->len);\n \treturn recurse;\n }\n \n@@ -266,9 +258,9 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,\n \tint early;\n \tint recurse;\n \tconst size_t baselen = base->len;\n-\tstruct show_tree_data data = { 0 };\n+\tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&data, &recurse, oid, base, pathname, mode);\n+\tearly = show_tree_common(&recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n@@ -286,9 +278,9 @@ static int show_tree_object(const struct object_id *oid, struct strbuf *base,\n {\n \tint early;\n \tint recurse;\n-\tstruct show_tree_data data = { 0 };\n+\tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&data, &recurse, oid, base, pathname, mode);\n+\tearly = show_tree_common(&recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n-- \n2.39.0.98.g174eaa53973.dirty\n\n"},{"id":"470146","messageId":"20230112091135.20050-3-tenglong.tl@alibaba-inc.com","threadId":"59074","inReplyTo":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","subject":"[PATCH 2/6] ls-tree: use a \"struct options\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-01-12T09:11:31Z","receivedAt":"2023-01-12T09:20:35Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nAs a first step towards being able to turn this code into an API some\nday let's change the \"static\" options in builtin/ls-tree.c into a\n\"struct ls_tree_options\" that can be constructed dynamically without\nthe help of parse_options().\n\nBecause we're now using non-static variables for this we'll need to\nclear_pathspec() at the end of cmd_ls_tree(), least various tests\nstart failing under SANITIZE=leak. The memory leak was already there\nbefore, now it's just being brought to the surface.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Teng Long <dyroneteng@gmail.com>\n---\n builtin/ls-tree.c | 203 ++++++++++++++++++++++++++--------------------\n 1 file changed, 116 insertions(+), 87 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex cbb6782f9a5..54f7b604cb7 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -14,37 +14,11 @@\n #include \"parse-options.h\"\n #include \"pathspec.h\"\n \n-static int line_termination = '\\n';\n-#define LS_RECURSIVE 1\n-#define LS_TREE_ONLY (1 << 1)\n-#define LS_SHOW_TREES (1 << 2)\n-static int abbrev;\n-static int ls_options;\n-static struct pathspec pathspec;\n-static int chomp_prefix;\n-static const char *ls_tree_prefix;\n-static const char *format;\n-struct show_tree_data {\n-\tunsigned mode;\n-\tenum object_type type;\n-\tconst struct object_id *oid;\n-\tconst char *pathname;\n-\tstruct strbuf *base;\n-};\n-\n static const  char * const ls_tree_usage[] = {\n \tN_(\"git ls-tree [<options>] <tree-ish> [<path>...]\"),\n \tNULL\n };\n \n-static enum ls_tree_cmdmode {\n-\tMODE_DEFAULT = 0,\n-\tMODE_LONG,\n-\tMODE_NAME_ONLY,\n-\tMODE_NAME_STATUS,\n-\tMODE_OBJECT_ONLY,\n-} cmdmode;\n-\n static void expand_objectsize(struct strbuf *line, const struct object_id *oid,\n \t\t\t      const enum object_type type, unsigned int padded)\n {\n@@ -64,10 +38,39 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid,\n \t}\n }\n \n+struct ls_tree_options {\n+\tint line_termination;\n+\tint abbrev;\n+\tenum ls_tree_path_options {\n+\t\tLS_RECURSIVE = 1 << 0,\n+\t\tLS_TREE_ONLY = 1 << 1,\n+\t\tLS_SHOW_TREES = 1 << 2,\n+\t} ls_options;\n+\tstruct pathspec pathspec;\n+\tint chomp_prefix;\n+\tconst char *ls_tree_prefix;\n+\tconst char *format;\n+};\n+\n+struct show_tree_data {\n+\tunsigned mode;\n+\tenum object_type type;\n+\tconst struct object_id *oid;\n+\tconst char *pathname;\n+\tstruct strbuf *base;\n+};\n+\n+struct show_tree_data_cb {\n+\tstruct ls_tree_options *options;\n+\tstruct show_tree_data *data;\n+};\n+\n static size_t expand_show_tree(struct strbuf *sb, const char *start,\n \t\t\t       void *context)\n {\n-\tstruct show_tree_data *data = context;\n+\tstruct show_tree_data_cb *wrapper = context;\n+\tstruct ls_tree_options *options = wrapper->options;\n+\tstruct show_tree_data *data = wrapper->data;\n \tconst char *end;\n \tconst char *p;\n \tunsigned int errlen;\n@@ -92,10 +95,10 @@ static size_t expand_show_tree(struct strbuf *sb, const char *start,\n \t} else if (skip_prefix(start, \"(objectsize)\", &p)) {\n \t\texpand_objectsize(sb, data->oid, data->type, 0);\n \t} else if (skip_prefix(start, \"(objectname)\", &p)) {\n-\t\tstrbuf_add_unique_abbrev(sb, data->oid, abbrev);\n+\t\tstrbuf_add_unique_abbrev(sb, data->oid, options->abbrev);\n \t} else if (skip_prefix(start, \"(path)\", &p)) {\n \t\tconst char *name = data->base->buf;\n-\t\tconst char *prefix = chomp_prefix ? ls_tree_prefix : NULL;\n+\t\tconst char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;\n \t\tstruct strbuf quoted = STRBUF_INIT;\n \t\tstruct strbuf sbuf = STRBUF_INIT;\n \t\tstrbuf_addstr(data->base, data->pathname);\n@@ -111,18 +114,19 @@ static size_t expand_show_tree(struct strbuf *sb, const char *start,\n \treturn len;\n }\n \n-static int show_recursive(const char *base, size_t baselen, const char *pathname)\n+static int show_recursive(struct ls_tree_options *options, const char *base,\n+\t\t\t  size_t baselen, const char *pathname)\n {\n \tint i;\n \n-\tif (ls_options & LS_RECURSIVE)\n+\tif (options->ls_options & LS_RECURSIVE)\n \t\treturn 1;\n \n-\tif (!pathspec.nr)\n+\tif (!options->pathspec.nr)\n \t\treturn 0;\n \n-\tfor (i = 0; i < pathspec.nr; i++) {\n-\t\tconst char *spec = pathspec.items[i].match;\n+\tfor (i = 0; i < options->pathspec.nr; i++) {\n+\t\tconst char *spec = options->pathspec.items[i].match;\n \t\tsize_t len, speclen;\n \n \t\tif (strncmp(base, spec, baselen))\n@@ -142,13 +146,13 @@ static int show_recursive(const char *base, size_t baselen, const char *pathname\n }\n \n static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n-\t\t\t const char *pathname, unsigned mode, void *context UNUSED)\n+\t\t\t const char *pathname, unsigned mode, void *context)\n {\n+\tstruct ls_tree_options *options = context;\n \tsize_t baselen;\n \tint recurse = 0;\n \tstruct strbuf sb = STRBUF_INIT;\n \tenum object_type type = object_type(mode);\n-\n \tstruct show_tree_data data = {\n \t\t.mode = mode,\n \t\t.type = type,\n@@ -156,81 +160,89 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \t\t.pathname = pathname,\n \t\t.base = base,\n \t};\n+\tstruct show_tree_data_cb cb_data = {\n+\t\t.data = &data,\n+\t\t.options = options,\n+\t};\n \n-\tif (type == OBJ_TREE && show_recursive(base->buf, base->len, pathname))\n+\tif (type == OBJ_TREE && show_recursive(options, base->buf, base->len, pathname))\n \t\trecurse = READ_TREE_RECURSIVE;\n-\tif (type == OBJ_TREE && recurse && !(ls_options & LS_SHOW_TREES))\n+\tif (type == OBJ_TREE && recurse && !(options->ls_options & LS_SHOW_TREES))\n \t\treturn recurse;\n-\tif (type == OBJ_BLOB && (ls_options & LS_TREE_ONLY))\n+\tif (type == OBJ_BLOB && (options->ls_options & LS_TREE_ONLY))\n \t\treturn 0;\n \n \tbaselen = base->len;\n-\tstrbuf_expand(&sb, format, expand_show_tree, &data);\n-\tstrbuf_addch(&sb, line_termination);\n+\tstrbuf_expand(&sb, options->format, expand_show_tree, &cb_data);\n+\tstrbuf_addch(&sb, options->line_termination);\n \tfwrite(sb.buf, sb.len, 1, stdout);\n \tstrbuf_release(&sb);\n \tstrbuf_setlen(base, baselen);\n \treturn recurse;\n }\n \n-static int show_tree_common(int *recurse, struct strbuf *base,\n-\t\t\t    const char *pathname, enum object_type type)\n+static int show_tree_common(struct ls_tree_options *options, int *recurse,\n+\t\t\t    struct strbuf *base, const char *pathname,\n+\t\t\t    enum object_type type)\n {\n \tint ret = -1;\n \t*recurse = 0;\n \n \tif (type == OBJ_BLOB) {\n-\t\tif (ls_options & LS_TREE_ONLY)\n+\t\tif (options->ls_options & LS_TREE_ONLY)\n \t\t\tret = 0;\n \t} else if (type == OBJ_TREE &&\n-\t\t   show_recursive(base->buf, base->len, pathname)) {\n+\t\t   show_recursive(options, base->buf, base->len, pathname)) {\n \t\t*recurse = READ_TREE_RECURSIVE;\n-\t\tif (!(ls_options & LS_SHOW_TREES))\n+\t\tif (!(options->ls_options & LS_SHOW_TREES))\n \t\t\tret = *recurse;\n \t}\n \n \treturn ret;\n }\n \n-static void show_tree_common_default_long(struct strbuf *base,\n+static void show_tree_common_default_long(struct ls_tree_options *options,\n+\t\t\t\t\t  struct strbuf *base,\n \t\t\t\t\t  const char *pathname,\n \t\t\t\t\t  const size_t baselen)\n {\n \tstrbuf_addstr(base, pathname);\n \twrite_name_quoted_relative(base->buf,\n-\t\t\t\t   chomp_prefix ? ls_tree_prefix : NULL, stdout,\n-\t\t\t\t   line_termination);\n+\t\t\t\t   options->chomp_prefix ? options->ls_tree_prefix : NULL, stdout,\n+\t\t\t\t   options->line_termination);\n \tstrbuf_setlen(base, baselen);\n }\n \n static int show_tree_default(const struct object_id *oid, struct strbuf *base,\n \t\t\t     const char *pathname, unsigned mode,\n-\t\t\t     void *context UNUSED)\n+\t\t\t     void *context)\n {\n+\tstruct ls_tree_options *options = context;\n \tint early;\n \tint recurse;\n \tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&recurse, base, pathname, type);\n+\tearly = show_tree_common(options, &recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n \tprintf(\"%06o %s %s\\t\", mode, type_name(object_type(mode)),\n-\t       find_unique_abbrev(oid, abbrev));\n-\tshow_tree_common_default_long(base, pathname, base->len);\n+\t       find_unique_abbrev(oid, options->abbrev));\n+\tshow_tree_common_default_long(options, base, pathname, base->len);\n \treturn recurse;\n }\n \n static int show_tree_long(const struct object_id *oid, struct strbuf *base,\n \t\t\t  const char *pathname, unsigned mode,\n-\t\t\t  void *context UNUSED)\n+\t\t\t  void *context)\n {\n+\tstruct ls_tree_options *options = context;\n \tint early;\n \tint recurse;\n \tchar size_text[24];\n \tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&recurse, base, pathname, type);\n+\tearly = show_tree_common(options, &recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n@@ -246,48 +258,58 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,\n \t}\n \n \tprintf(\"%06o %s %s %7s\\t\", mode, type_name(type),\n-\t       find_unique_abbrev(oid, abbrev), size_text);\n-\tshow_tree_common_default_long(base, pathname, base->len);\n+\t       find_unique_abbrev(oid, options->abbrev), size_text);\n+\tshow_tree_common_default_long(options, base, pathname, base->len);\n \treturn recurse;\n }\n \n static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,\n \t\t\t       const char *pathname, unsigned mode,\n-\t\t\t       void *context UNUSED)\n+\t\t\t       void *context)\n {\n+\tstruct ls_tree_options *options = context;\n \tint early;\n \tint recurse;\n \tconst size_t baselen = base->len;\n \tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&recurse, base, pathname, type);\n+\tearly = show_tree_common(options, &recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n \tstrbuf_addstr(base, pathname);\n \twrite_name_quoted_relative(base->buf,\n-\t\t\t\t   chomp_prefix ? ls_tree_prefix : NULL,\n-\t\t\t\t   stdout, line_termination);\n+\t\t\t\t   options->chomp_prefix ? options->ls_tree_prefix : NULL,\n+\t\t\t\t   stdout, options->line_termination);\n \tstrbuf_setlen(base, baselen);\n \treturn recurse;\n }\n \n static int show_tree_object(const struct object_id *oid, struct strbuf *base,\n \t\t\t    const char *pathname, unsigned mode,\n-\t\t\t    void *context UNUSED)\n+\t\t\t    void *context)\n {\n+\tstruct ls_tree_options *options = context;\n \tint early;\n \tint recurse;\n \tenum object_type type = object_type(mode);\n \n-\tearly = show_tree_common(&recurse, base, pathname, type);\n+\tearly = show_tree_common(options, &recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n-\tprintf(\"%s%c\", find_unique_abbrev(oid, abbrev), line_termination);\n+\tprintf(\"%s%c\", find_unique_abbrev(oid, options->abbrev), options->line_termination);\n \treturn recurse;\n }\n \n+enum ls_tree_cmdmode {\n+\tMODE_DEFAULT = 0,\n+\tMODE_LONG,\n+\tMODE_NAME_ONLY,\n+\tMODE_NAME_STATUS,\n+\tMODE_OBJECT_ONLY,\n+};\n+\n struct ls_tree_cmdmode_to_fmt {\n \tenum ls_tree_cmdmode mode;\n \tconst char *const fmt;\n@@ -327,14 +349,18 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tstruct tree *tree;\n \tint i, full_tree = 0;\n \tread_tree_fn_t fn = NULL;\n+\tenum ls_tree_cmdmode cmdmode = MODE_DEFAULT;\n+\tstruct ls_tree_options options = {\n+\t\t.line_termination = '\\n',\n+\t};\n \tconst struct option ls_tree_options[] = {\n-\t\tOPT_BIT('d', NULL, &ls_options, N_(\"only show trees\"),\n+\t\tOPT_BIT('d', NULL, &options.ls_options, N_(\"only show trees\"),\n \t\t\tLS_TREE_ONLY),\n-\t\tOPT_BIT('r', NULL, &ls_options, N_(\"recurse into subtrees\"),\n+\t\tOPT_BIT('r', NULL, &options.ls_options, N_(\"recurse into subtrees\"),\n \t\t\tLS_RECURSIVE),\n-\t\tOPT_BIT('t', NULL, &ls_options, N_(\"show trees when recursing\"),\n+\t\tOPT_BIT('t', NULL, &options.ls_options, N_(\"show trees when recursing\"),\n \t\t\tLS_SHOW_TREES),\n-\t\tOPT_SET_INT('z', NULL, &line_termination,\n+\t\tOPT_SET_INT('z', NULL, &options.line_termination,\n \t\t\t    N_(\"terminate entries with NUL byte\"), 0),\n \t\tOPT_CMDMODE('l', \"long\", &cmdmode, N_(\"include object size\"),\n \t\t\t    MODE_LONG),\n@@ -344,29 +370,30 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\t    MODE_NAME_STATUS),\n \t\tOPT_CMDMODE(0, \"object-only\", &cmdmode, N_(\"list only objects\"),\n \t\t\t    MODE_OBJECT_ONLY),\n-\t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n+\t\tOPT_SET_INT(0, \"full-name\", &options.chomp_prefix,\n \t\t\t    N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n \t\t\t N_(\"list entire tree; not just current directory \"\n \t\t\t    \"(implies --full-name)\")),\n-\t\tOPT_STRING_F(0, \"format\", &format, N_(\"format\"),\n+\t\tOPT_STRING_F(0, \"format\", &options.format, N_(\"format\"),\n \t\t\t\t\t N_(\"format to use for the output\"),\n \t\t\t\t\t PARSE_OPT_NONEG),\n-\t\tOPT__ABBREV(&abbrev),\n+\t\tOPT__ABBREV(&options.abbrev),\n \t\tOPT_END()\n \t};\n \tstruct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format;\n+\tint ret;\n \n \tgit_config(git_default_config, NULL);\n-\tls_tree_prefix = prefix;\n+\toptions.ls_tree_prefix = prefix;\n \tif (prefix)\n-\t\tchomp_prefix = strlen(prefix);\n+\t\toptions.chomp_prefix = strlen(prefix);\n \n \targc = parse_options(argc, argv, prefix, ls_tree_options,\n \t\t\t     ls_tree_usage, 0);\n \tif (full_tree) {\n-\t\tls_tree_prefix = prefix = NULL;\n-\t\tchomp_prefix = 0;\n+\t\toptions.ls_tree_prefix = prefix = NULL;\n+\t\toptions.chomp_prefix = 0;\n \t}\n \t/*\n \t * We wanted to detect conflicts between --name-only and\n@@ -378,10 +405,10 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \n \t/* -d -r should imply -t, but -d by itself should not have to. */\n \tif ( (LS_TREE_ONLY|LS_RECURSIVE) ==\n-\t    ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options))\n-\t\tls_options |= LS_SHOW_TREES;\n+\t    ((LS_TREE_ONLY|LS_RECURSIVE) & options.ls_options))\n+\t\toptions.ls_options |= LS_SHOW_TREES;\n \n-\tif (format && cmdmode)\n+\tif (options.format && cmdmode)\n \t\tusage_msg_opt(\n \t\t\t_(\"--format can't be combined with other format-altering options\"),\n \t\t\tls_tree_usage, ls_tree_options);\n@@ -396,13 +423,13 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t * cannot be lifted until it is converted to use\n \t * match_pathspec() or tree_entry_interesting()\n \t */\n-\tparse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC &\n-\t\t\t\t  ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),\n+\tparse_pathspec(&options.pathspec, PATHSPEC_ALL_MAGIC &\n+\t\t       ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),\n \t\t       PATHSPEC_PREFER_CWD,\n \t\t       prefix, argv + 1);\n-\tfor (i = 0; i < pathspec.nr; i++)\n-\t\tpathspec.items[i].nowildcard_len = pathspec.items[i].len;\n-\tpathspec.has_wildcard = 0;\n+\tfor (i = 0; i < options.pathspec.nr; i++)\n+\t\toptions.pathspec.items[i].nowildcard_len = options.pathspec.items[i].len;\n+\toptions.pathspec.has_wildcard = 0;\n \ttree = parse_tree_indirect(&oid);\n \tif (!tree)\n \t\tdie(\"not a tree object\");\n@@ -412,11 +439,11 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t */\n \twhile (m2f) {\n \t\tif (!m2f->fmt) {\n-\t\t\tfn = format ? show_tree_fmt : show_tree_default;\n-\t\t} else if (format && !strcmp(format, m2f->fmt)) {\n+\t\t\tfn = options.format ? show_tree_fmt : show_tree_default;\n+\t\t} else if (options.format && !strcmp(options.format, m2f->fmt)) {\n \t\t\tcmdmode = m2f->mode;\n \t\t\tfn = m2f->fn;\n-\t\t} else if (!format && cmdmode == m2f->mode) {\n+\t\t} else if (!options.format && cmdmode == m2f->mode) {\n \t\t\tfn = m2f->fn;\n \t\t} else {\n \t\t\tm2f++;\n@@ -425,5 +452,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \t}\n \n-\treturn !!read_tree(the_repository, tree, &pathspec, fn, NULL);\n+\tret = !!read_tree(the_repository, tree, &options.pathspec, fn, &options);\n+\tclear_pathspec(&options.pathspec);\n+\treturn ret;\n }\n-- \n2.39.0.98.g174eaa53973.dirty\n\n"},{"id":"470147","messageId":"20230112091135.20050-4-tenglong.tl@alibaba-inc.com","threadId":"59074","inReplyTo":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","subject":"[PATCH 3/6] ls-tree: fold \"show_tree_data\" into \"cb\" struct","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-01-12T09:11:32Z","receivedAt":"2023-01-12T09:20:43Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nAfter the the preceding two commits the only user of the\n\"show_tree_data\" struct needed it along with the \"options\" member,\nlet's instead fold all of that into a \"show_tree_data\" struct that\nwe'll use only for that callback.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Teng Long <dyroneteng@gmail.com>\n---\n builtin/ls-tree.c | 18 +++++-------------\n 1 file changed, 5 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 54f7b604cb7..da664eecfb9 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -53,6 +53,7 @@ struct ls_tree_options {\n };\n \n struct show_tree_data {\n+\tstruct ls_tree_options *options;\n \tunsigned mode;\n \tenum object_type type;\n \tconst struct object_id *oid;\n@@ -60,17 +61,11 @@ struct show_tree_data {\n \tstruct strbuf *base;\n };\n \n-struct show_tree_data_cb {\n-\tstruct ls_tree_options *options;\n-\tstruct show_tree_data *data;\n-};\n-\n static size_t expand_show_tree(struct strbuf *sb, const char *start,\n \t\t\t       void *context)\n {\n-\tstruct show_tree_data_cb *wrapper = context;\n-\tstruct ls_tree_options *options = wrapper->options;\n-\tstruct show_tree_data *data = wrapper->data;\n+\tstruct show_tree_data *data = context;\n+\tstruct ls_tree_options *options = data->options;\n \tconst char *end;\n \tconst char *p;\n \tunsigned int errlen;\n@@ -153,17 +148,14 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \tint recurse = 0;\n \tstruct strbuf sb = STRBUF_INIT;\n \tenum object_type type = object_type(mode);\n-\tstruct show_tree_data data = {\n+\tstruct show_tree_data cb_data = {\n+\t\t.options = options,\n \t\t.mode = mode,\n \t\t.type = type,\n \t\t.oid = oid,\n \t\t.pathname = pathname,\n \t\t.base = base,\n \t};\n-\tstruct show_tree_data_cb cb_data = {\n-\t\t.data = &data,\n-\t\t.options = options,\n-\t};\n \n \tif (type == OBJ_TREE && show_recursive(options, base->buf, base->len, pathname))\n \t\trecurse = READ_TREE_RECURSIVE;\n-- \n2.39.0.98.g174eaa53973.dirty\n\n"},{"id":"470148","messageId":"20230112091135.20050-5-tenglong.tl@alibaba-inc.com","threadId":"59074","inReplyTo":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","subject":"[PATCH 4/6] ls-tree: make \"line_termination\" less generic","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-01-12T09:11:33Z","receivedAt":"2023-01-12T09:20:56Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nThe \"ls-tree\" command isn't capable of ending \"lines\" with anything\nexcept '\\n' or '\\0', and in the latter case we can avoid calling\nwrite_name_quoted_relative() entirely. Let's do that, less for\noptimization and more for clarity, the write_name_quoted_relative()\nAPI itself does much the same thing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Teng Long <dyroneteng@gmail.com>\n---\n builtin/ls-tree.c | 58 +++++++++++++++++++++++++++++++++++------------\n 1 file changed, 44 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex da664eecfb9..a743959f2b3 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -39,7 +39,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid,\n }\n \n struct ls_tree_options {\n-\tint line_termination;\n+\tunsigned null_termination:1;\n \tint abbrev;\n \tenum ls_tree_path_options {\n \t\tLS_RECURSIVE = 1 << 0,\n@@ -166,7 +166,7 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \n \tbaselen = base->len;\n \tstrbuf_expand(&sb, options->format, expand_show_tree, &cb_data);\n-\tstrbuf_addch(&sb, options->line_termination);\n+\tstrbuf_addch(&sb, options->null_termination ? '\\0' : '\\n');\n \tfwrite(sb.buf, sb.len, 1, stdout);\n \tstrbuf_release(&sb);\n \tstrbuf_setlen(base, baselen);\n@@ -198,10 +198,22 @@ static void show_tree_common_default_long(struct ls_tree_options *options,\n \t\t\t\t\t  const char *pathname,\n \t\t\t\t\t  const size_t baselen)\n {\n+\tconst char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;\n+\n \tstrbuf_addstr(base, pathname);\n-\twrite_name_quoted_relative(base->buf,\n-\t\t\t\t   options->chomp_prefix ? options->ls_tree_prefix : NULL, stdout,\n-\t\t\t\t   options->line_termination);\n+\n+\tif (options->null_termination) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *name = relative_path(base->buf, prefix, &sb);\n+\n+\t\tfputs(name, stdout);\n+\t\tfputc('\\0', stdout);\n+\n+\t\tstrbuf_release(&sb);\n+\t} else {\n+\t\twrite_name_quoted_relative(base->buf, prefix, stdout, '\\n');\n+\t}\n+\n \tstrbuf_setlen(base, baselen);\n }\n \n@@ -264,15 +276,25 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,\n \tint recurse;\n \tconst size_t baselen = base->len;\n \tenum object_type type = object_type(mode);\n+\tconst char *prefix;\n \n \tearly = show_tree_common(options, &recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n+\tprefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;\n \tstrbuf_addstr(base, pathname);\n-\twrite_name_quoted_relative(base->buf,\n-\t\t\t\t   options->chomp_prefix ? options->ls_tree_prefix : NULL,\n-\t\t\t\t   stdout, options->line_termination);\n+\tif (options->null_termination) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *name = relative_path(base->buf, prefix, &sb);\n+\n+\t\tfputs(name, stdout);\n+\t\tfputc('\\0', stdout);\n+\n+\t\tstrbuf_release(&sb);\n+\t} else {\n+\t\twrite_name_quoted_relative(base->buf, prefix, stdout, '\\n');\n+\t}\n \tstrbuf_setlen(base, baselen);\n \treturn recurse;\n }\n@@ -285,12 +307,19 @@ static int show_tree_object(const struct object_id *oid, struct strbuf *base,\n \tint early;\n \tint recurse;\n \tenum object_type type = object_type(mode);\n+\tconst char *str;\n \n \tearly = show_tree_common(options, &recurse, base, pathname, type);\n \tif (early >= 0)\n \t\treturn early;\n \n-\tprintf(\"%s%c\", find_unique_abbrev(oid, options->abbrev), options->line_termination);\n+\tstr = find_unique_abbrev(oid, options->abbrev);\n+\tif (options->null_termination) {\n+\t\tfputs(str, stdout);\n+\t\tfputc('\\0', stdout);\n+\t} else  {\n+\t\tputs(str);\n+\t}\n \treturn recurse;\n }\n \n@@ -342,9 +371,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tint i, full_tree = 0;\n \tread_tree_fn_t fn = NULL;\n \tenum ls_tree_cmdmode cmdmode = MODE_DEFAULT;\n-\tstruct ls_tree_options options = {\n-\t\t.line_termination = '\\n',\n-\t};\n+\tint null_termination = 0;\n+\tstruct ls_tree_options options = { 0 };\n \tconst struct option ls_tree_options[] = {\n \t\tOPT_BIT('d', NULL, &options.ls_options, N_(\"only show trees\"),\n \t\t\tLS_TREE_ONLY),\n@@ -352,8 +380,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\tLS_RECURSIVE),\n \t\tOPT_BIT('t', NULL, &options.ls_options, N_(\"show trees when recursing\"),\n \t\t\tLS_SHOW_TREES),\n-\t\tOPT_SET_INT('z', NULL, &options.line_termination,\n-\t\t\t    N_(\"terminate entries with NUL byte\"), 0),\n+\t\tOPT_BOOL('z', NULL, &null_termination,\n+\t\t\t N_(\"terminate entries with NUL byte\")),\n \t\tOPT_CMDMODE('l', \"long\", &cmdmode, N_(\"include object size\"),\n \t\t\t    MODE_LONG),\n \t\tOPT_CMDMODE(0, \"name-only\", &cmdmode, N_(\"list only filenames\"),\n@@ -383,6 +411,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, ls_tree_options,\n \t\t\t     ls_tree_usage, 0);\n+\toptions.null_termination = null_termination;\n+\n \tif (full_tree) {\n \t\toptions.ls_tree_prefix = prefix = NULL;\n \t\toptions.chomp_prefix = 0;\n-- \n2.39.0.98.g174eaa53973.dirty\n\n"},{"id":"470149","messageId":"20230112091135.20050-6-tenglong.tl@alibaba-inc.com","threadId":"59074","inReplyTo":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","subject":"[PATCH 5/6] ls-tree: cleanup the redundant SPACE","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-01-12T09:11:34Z","receivedAt":"2023-01-12T09:20:58Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long <dyroneteng@gmail.com>\n\nAn redundant space was found in ls-tree.c, which is no doubt\na small change, but it might be OK to make a commit on its own.\n\nSigned-off-by: Teng Long <dyroneteng@gmail.com>\n---\n builtin/ls-tree.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex a743959f2b3..72eb70823d5 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -14,7 +14,7 @@\n #include \"parse-options.h\"\n #include \"pathspec.h\"\n \n-static const  char * const ls_tree_usage[] = {\n+static const char * const ls_tree_usage[] = {\n \tN_(\"git ls-tree [<options>] <tree-ish> [<path>...]\"),\n \tNULL\n };\n-- \n2.39.0.98.g174eaa53973.dirty\n\n"},{"id":"470150","messageId":"20230112091135.20050-7-tenglong.tl@alibaba-inc.com","threadId":"59074","inReplyTo":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","subject":"[PATCH 6/6] t3104: remove shift code in 'test_ls_tree_format'","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2023-01-12T09:11:35Z","receivedAt":"2023-01-12T09:21:11Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long <dyroneteng@gmail.com>\n\nIn t3104-ls-tree-format.sh, There is a legacy 'shift 2' code\nand the relevant code block no longer depends on it anymore,\nso let's remove it for a small cleanup.\n\nSigned-off-by: Teng Long <dyroneteng@gmail.com>\n---\n t/t3104-ls-tree-format.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/t/t3104-ls-tree-format.sh b/t/t3104-ls-tree-format.sh\nindex 383896667b6..74053978f49 100755\n--- a/t/t3104-ls-tree-format.sh\n+++ b/t/t3104-ls-tree-format.sh\n@@ -20,7 +20,6 @@ test_ls_tree_format () {\n \tformat=$1 &&\n \topts=$2 &&\n \tfmtopts=$3 &&\n-\tshift 2 &&\n \n \ttest_expect_success \"ls-tree '--format=<$format>' is like options '$opts $fmtopts'\" '\n \t\tgit ls-tree $opts -r HEAD >expect &&\n-- \n2.39.0.98.g174eaa53973.dirty\n\n"},{"id":"470190","messageId":"230112.86h6wv248r.gmgdl@evledraar.gmail.com","threadId":"59074","inReplyTo":"20230112091135.20050-6-tenglong.tl@alibaba-inc.com","subject":"Re: [PATCH 5/6] ls-tree: cleanup the redundant SPACE","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T14:53:07Z","receivedAt":"2023-01-12T15:05:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jan 12 2023, Teng Long wrote:\n\n> From: Teng Long <dyroneteng@gmail.com>\n>\n> An redundant space was found in ls-tree.c, which is no doubt\n> a small change, but it might be OK to make a commit on its own.\n\nNo need to update this series for just this, but if you're doing an\nupdate it would be nice to mention that this minor typo/unwanted syntax\nhas been there since 61fdbcf98b4 (ls-tree: migrate to parse-options,\n2009-11-13).\n\nI.e. to clarify that this is rather ancient, and not some fix-up for a\nrecent topic (or fixing things earlier in this topic itself).\n\n> Signed-off-by: Teng Long <dyroneteng@gmail.com>\n> ---\n>  builtin/ls-tree.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index a743959f2b3..72eb70823d5 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -14,7 +14,7 @@\n>  #include \"parse-options.h\"\n>  #include \"pathspec.h\"\n>  \n> -static const  char * const ls_tree_usage[] = {\n> +static const char * const ls_tree_usage[] = {\n>  \tN_(\"git ls-tree [<options>] <tree-ish> [<path>...]\"),\n>  \tNULL\n>  };\n\n"},{"id":"470191","messageId":"230112.86cz7j23x0.gmgdl@evledraar.gmail.com","threadId":"59074","inReplyTo":"20230112091135.20050-7-tenglong.tl@alibaba-inc.com","subject":"Re: [PATCH 6/6] t3104: remove shift code in 'test_ls_tree_format'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T14:56:22Z","receivedAt":"2023-01-12T15:12:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jan 12 2023, Teng Long wrote:\n\n> From: Teng Long <dyroneteng@gmail.com>\n>\n> In t3104-ls-tree-format.sh, There is a legacy 'shift 2' code\n> and the relevant code block no longer depends on it anymore,\n> so let's remove it for a small cleanup.\n\nLike with 6/6, we can do without an update for this, but if you're\nupdating I'd say something like this instead:\n\n\tThe \"shift 2\" added here when these tests were added in\n\t455923e0a15 (ls-tree: introduce \"--format\" option, 2022-03-23)\n\thas never been need. Let's remove it, as we're not in the habit\n\tof clearing arguments from \"$@\" unless we need to use \"$@\"\n\titself (or \"$*\") later.\n\nFWIW I think this dates back to my\nhttps://lore.kernel.org/git/RFC-patch-6.7-eac299f06ff-20211217T131635Z-avarab@gmail.com/;\nwhere there was no reason to do it either. I wouldn't normally \"shift\"\nfor no reason, so it probably dates back to some earlier version where I\nused \"$@\", which didn't make it to the list.\n\n> Signed-off-by: Teng Long <dyroneteng@gmail.com>\n> ---\n>  t/t3104-ls-tree-format.sh | 1 -\n>  1 file changed, 1 deletion(-)\n>\n> diff --git a/t/t3104-ls-tree-format.sh b/t/t3104-ls-tree-format.sh\n> index 383896667b6..74053978f49 100755\n> --- a/t/t3104-ls-tree-format.sh\n> +++ b/t/t3104-ls-tree-format.sh\n> @@ -20,7 +20,6 @@ test_ls_tree_format () {\n>  \tformat=$1 &&\n>  \topts=$2 &&\n>  \tfmtopts=$3 &&\n> -\tshift 2 &&\n>  \n>  \ttest_expect_success \"ls-tree '--format=<$format>' is like options '$opts $fmtopts'\" '\n>  \t\tgit ls-tree $opts -r HEAD >expect &&\n\n"},{"id":"470192","messageId":"230112.868ri723p0.gmgdl@evledraar.gmail.com","threadId":"59074","inReplyTo":"20230112091135.20050-1-tenglong.tl@alibaba-inc.com","subject":"Re: [PATCH 0/6] ls-tree.c: clean-up works","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-01-12T15:02:13Z","receivedAt":"2023-01-12T15:15:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jan 12 2023, Teng Long wrote:\n\n> From: Teng Long <dyroneteng@gmail.com>\n>\n> This patchset aims to do works on refactoring and cleanup\n> of ls-tree.c.\n>\n> The patches can be viewed as two parts:\n>\n>       * Part 1: from Ævar Arnfjörð Bjarmason which mentioned in\n>       his RFC patchset[1] and Ævar suggests to submit a separate clean-up\n>       series. After I read them, I accept them all.\n>\n>       * Part 2: fÆvarrom me which introduced in my RFC patchset[2] and after\n>       I accepted Ævar's patches, I keep two patches from [2], the left\n>       patches in [2] may no longer be needed.\n>\n> [1] https://public-inbox.org/git/RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com/\n> [2] https://public-inbox.org/git/20221117113023.65865-1-tenglong.tl@alibaba-inc.com/\n>\n> Thanks.\n\nThanks for picking this up. It would be great to have this queued up.\n\nIt looks good to me, although that's mostly self-approving, as the\nrange-diff between this and my [1] (and your description, but I wanted\nto sanity check it) shows it's mostly the same changes:\n\t\np\t1:  a7f86973d57 ! 1:  5004f03c4d2 ls-tree: don't use \"show_tree_data\" for \"fast\" callbacks\n\t    @@ Commit message\n\t     \n\t         1. https://lore.kernel.org/git/cover-0.7-00000000000-20220310T134811Z-avarab@gmail.com/\n\t     \n\t    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\t    +    Signed-off-by: Teng Long <dyronteng@gmail.com>\n\t    +\n\t      ## builtin/ls-tree.c ##\n\t     @@ builtin/ls-tree.c: static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n\t      \treturn recurse;\n\t2:  b046e6f5692 ! 2:  de5b4d0767a ls-tree: use a \"struct options\"\n\t    @@ Commit message\n\t         before, now it's just being brought to the surface.\n\t     \n\t         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\t    +    Signed-off-by: Teng Long <dyroneteng@gmail.com>\n\t     \n\t      ## builtin/ls-tree.c ##\n\t     @@\n\t3:  329bc581b79 ! 3:  8f4b59183f6 ls-tree: fold \"show_tree_data\" into \"cb\" struct\n\t    @@ Commit message\n\t         we'll use only for that callback.\n\t     \n\t         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\t    +    Signed-off-by: Teng Long <dyroneteng@gmail.com>\n\t     \n\t      ## builtin/ls-tree.c ##\n\t     @@ builtin/ls-tree.c: struct ls_tree_options {\n\t4:  de4f89645fa ! 4:  d701aceb101 ls-tree: make \"line_termination\" less generic\n\t    @@ Commit message\n\t         API itself does much the same thing.\n\t     \n\t         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\t    +    Signed-off-by: Teng Long <dyroneteng@gmail.com>\n\t     \n\t      ## builtin/ls-tree.c ##\n\t     @@ builtin/ls-tree.c: static void expand_objectsize(struct strbuf *line, const struct object_id *oid,\n\t-:  ----------- > 5:  72bf99a1c74 ls-tree: cleanup the redundant SPACE\n\t-:  ----------- > 6:  978b89e5ea8 t3104: remove shift code in 'test_ls_tree_format'\n\nI approve of that addition of the missing Signed-off-by (which I just\nforgot about).\n\nI left comments on the 2x new patches, but they were both nit-y\nsuggestions for maybe rephrasing the commit messages a bit, but as noted\nI don't think it's worth another re-roll (just maybe if others comment\non the other patches and/or find other issues worth re-rolling).\n"}]}