{"thread":{"id":"58810","subject":"[RFC PATCH 0/6] ls-tree: introduce '--pattern' option","startedAt":"2022-11-17T11:30:41Z","lastAt":"2023-03-28T21:05:08Z","messageCount":34,"participants":["Teng Long","Ævar Arnfjörð Bjarmason","Taylor Blau","Johannes Schindelin","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"467430","messageId":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":null,"subject":"[RFC PATCH 0/6] ls-tree: introduce '--pattern' option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-17T11:30:17Z","receivedAt":"2022-11-17T11:30:41Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \n\nThis RFC patch introduce a new \"ls-tree\" option \"--pattern\", aim to match the\nentries by regex then filter the output which we may want to achieve. It also\ncontains some commit for preparation or cleanup.\n\nThe idea may be not comprehensive and the tests for it might be insufficient\ntoo, but I'd like to listen the suggestion from the community to decide if it's\nworth going forward with.\n\nThanks.\n\nTeng Long (6):\n ls-tree: cleanup the redundant SPACE\n t3104: remove shift code in 'test_ls_tree_format'\n ls-tree: optimize params of 'show_tree_common_default_long()'\n ls-tree: improving cohension in the print code\n ls-tree: introduce 'match_pattern()' function\n ls-tree: introduce '--pattern' option\n\n Documentation/git-ls-tree.txt | 7 ++-\n builtin/ls-tree.c | 109 ++++++++++++++++++++++++++--------\n t/t3104-ls-tree-format.sh | 1 -\n t/t3106-ls-tree-pattern.sh | 70 ++++++++++++++++++++++\n 4 files changed, 161 insertions(+), 26 deletions(-)\n create mode 100755 t/t3106-ls-tree-pattern.sh\n\n-- \n2.38.1.426.g770fc8806cb.dirty\n\n"},{"id":"467431","messageId":"20221117113023.65865-2-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"[RFC PATCH 1/6] ls-tree: cleanup the redundant SPACE","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-17T11:30:18Z","receivedAt":"2022-11-17T11:30:44Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \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 \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 c3ea09281af..8256fc0bc97 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -32,7 +32,7 @@ struct show_tree_data {\n \tstruct strbuf *base;\n };\n \n-static const char * const ls_tree_usage[] = {\n+static const char * const ls_tree_usage[] = {\n \tN_(\"git ls-tree [] [...]\"),\n \tNULL\n };\n-- \n2.38.1.426.g770fc8806cb.dirty\n\n"},{"id":"467432","messageId":"20221117113023.65865-3-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"[RFC PATCH 2/6] t3104: remove shift code in 'test_ls_tree_format'","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-17T11:30:19Z","receivedAt":"2022-11-17T11:30:46Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \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 \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.38.1.426.g770fc8806cb.dirty\n\n"},{"id":"467433","messageId":"20221117113023.65865-4-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"[RFC PATCH 3/6] ls-tree: optimize params of 'show_tree_common_default_long()'","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-17T11:30:20Z","receivedAt":"2022-11-17T11:30:48Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \n\nFunction 'show_tree_common_default_long' has 3 parameters:\n\n * struct strbuf *base\n * const char *pathname\n * const size_t baselen\n\nAnd the struct 'show_tree_data' which was introduced in\n'e81517155e0370ae5433d256046ab287bb10d04d' is:\n\n struct show_tree_data {\n\t unsigned mode;\n\t enum object_type type;\n const struct object_id *oid;\n const char *pathname;\n struct strbuf *base;\n };\n\nActually, the struct includes all the data that we need to pass to the\nfunction, so we could make a small refactoring by using 'show_tree_data'\nstruct directly.\n\nSigned-off-by: Teng Long \n---\n builtin/ls-tree.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 8256fc0bc97..afb65af4280 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -200,15 +200,15 @@ static int show_tree_common(struct show_tree_data *data, int *recurse,\n \treturn ret;\n }\n \n-static void show_tree_common_default_long(struct strbuf *base,\n-\t\t\t\t\t const char *pathname,\n-\t\t\t\t\t const size_t baselen)\n+static void show_tree_common_default_long(struct show_tree_data *data)\n {\n-\tstrbuf_addstr(base, pathname);\n-\twrite_name_quoted_relative(base->buf,\n+\tint base_len = data->base->len;\n+\n+\tstrbuf_addstr(data->base, data->pathname);\n+\twrite_name_quoted_relative(data->base->buf,\n \t\t\t\t chomp_prefix ? ls_tree_prefix : NULL, stdout,\n \t\t\t\t line_termination);\n-\tstrbuf_setlen(base, baselen);\n+\tstrbuf_setlen(data->base, base_len);\n }\n \n static int show_tree_default(const struct object_id *oid, struct strbuf *base,\n@@ -225,7 +225,7 @@ static int show_tree_default(const struct object_id *oid, struct strbuf *base,\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+\tshow_tree_common_default_long(&data);\n \treturn recurse;\n }\n \n@@ -255,7 +255,7 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,\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+\tshow_tree_common_default_long(&data);\n \treturn recurse;\n }\n \n-- \n2.38.1.426.g770fc8806cb.dirty\n\n"},{"id":"467434","messageId":"20221117113023.65865-5-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"[RFC PATCH 4/6] ls-tree: improving cohension in the print code","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-17T11:30:21Z","receivedAt":"2022-11-17T11:30:54Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \n\nWe use 'show_tree_long()' and 'show_tree_default()' to print entries\nwhen we want the output in 'default' or 'default long' formats.\n\nFunction 'show_tree_common_default_long()' prints the \"path\" field as\nthe last part of output, the previous part which separately implements\nin 'show_tree_long()' and 'show_tree_default()'.\n\nWe can package the separate implementation together by the extension of\n\"size_text\" in struct \"show_tree_data\". By improving the cohesion in\nthese two locations, some benefits such as uniform processing of the\noutput can be achieved in future.\n\nSigned-off-by: Teng Long \n---\n builtin/ls-tree.c | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex afb65af4280..7661170f7ca 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -30,6 +30,7 @@ struct show_tree_data {\n \tconst struct object_id *oid;\n \tconst char *pathname;\n \tstruct strbuf *base;\n+\tchar *size_text;\n };\n \n static const char * const ls_tree_usage[] = {\n@@ -186,6 +187,7 @@ static int show_tree_common(struct show_tree_data *data, int *recurse,\n \tdata->oid = oid;\n \tdata->pathname = pathname;\n \tdata->base = base;\n+\tdata->size_text = NULL;\n \n \tif (type == OBJ_BLOB) {\n \t\tif (ls_options & LS_TREE_ONLY)\n@@ -204,6 +206,13 @@ static void show_tree_common_default_long(struct show_tree_data *data)\n {\n \tint base_len = data->base->len;\n \n+\tif (data->size_text)\n+\t\tprintf(\"%06o %s %s %7s\\t\", data->mode, type_name(data->type),\n+\t\t find_unique_abbrev(data->oid, abbrev), data->size_text);\n+\telse\n+\t\tprintf(\"%06o %s %s\\t\", data->mode, type_name(data->type),\n+\t\t find_unique_abbrev(data->oid, abbrev));\n+\n \tstrbuf_addstr(data->base, data->pathname);\n \twrite_name_quoted_relative(data->base->buf,\n \t\t\t\t chomp_prefix ? ls_tree_prefix : NULL, stdout,\n@@ -223,8 +232,6 @@ static int show_tree_default(const struct object_id *oid, struct strbuf *base,\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(&data);\n \treturn recurse;\n }\n@@ -253,8 +260,7 @@ 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+\tdata.size_text = size_text;\n \tshow_tree_common_default_long(&data);\n \treturn recurse;\n }\n-- \n2.38.1.426.g770fc8806cb.dirty\n\n"},{"id":"467435","messageId":"20221117113023.65865-6-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"[RFC PATCH 5/6] ls-tree: introduce 'match_pattern()' function","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-17T11:30:22Z","receivedAt":"2022-11-17T11:31:10Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \n\nIn preparation for actually implementing the '--pattern' option, we\nadd a new method called 'match_pattern' that uses regular expressions\nto match 'ls-tree' entities.\n\nSigned-off-by: Teng Long \n---\n builtin/ls-tree.c | 27 +++++++++++++++++++++++++++\n 1 file changed, 27 insertions(+)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 7661170f7ca..03dd3fbcb26 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -24,6 +24,7 @@ static struct pathspec pathspec;\n static int chomp_prefix;\n static const char *ls_tree_prefix;\n static const char *format;\n+static const char *pattern;\n struct show_tree_data {\n \tunsigned mode;\n \tenum object_type type;\n@@ -46,6 +47,32 @@ static enum ls_tree_cmdmode {\n \tMODE_OBJECT_ONLY,\n } cmdmode;\n \n+__attribute__((unused))\n+static int match_pattern(const char *line)\n+{\n+\tint ret = 0;\n+\tregex_t r;\n+\tregmatch_t m[1];\n+\tchar errbuf[64];\n+\n+\tret = regcomp(&r, pattern, 0);\n+\tif (ret) {\n+\t\tregerror(ret, &r, errbuf, sizeof(errbuf));\n+\t\tdie(\"failed regcomp() for pattern '%s' (%s)\", pattern, errbuf);\n+\t}\n+\tret = regexec(&r, line, 1, m, 0);\n+\tif (ret) {\n+\t\tif (ret == REG_NOMATCH)\n+\t\t\tgoto cleanup;\n+\t\tregerror(ret, &r, errbuf, sizeof(errbuf));\n+\t\tdie(\"failed regexec() for subject '%s' (%s)\", line, errbuf);\n+\t}\n+\n+cleanup:\n+\tregfree(&r);\n+\treturn ret;\n+}\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-- \n2.38.1.426.g770fc8806cb.dirty\n\n"},{"id":"467436","messageId":"20221117113023.65865-7-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"[RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-17T11:30:23Z","receivedAt":"2022-11-17T11:31:13Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"From: Teng Long \n\nThe \"--pattern\" option uses regular expressions to match each\nentry, then filter the output of \"ls-tree\" .\n\nSigned-off-by: Teng Long \n---\n Documentation/git-ls-tree.txt | 7 ++-\n builtin/ls-tree.c | 82 +++++++++++++++++++++++------------\n t/t3106-ls-tree-pattern.sh | 70 ++++++++++++++++++++++++++++++\n 3 files changed, 131 insertions(+), 28 deletions(-)\n create mode 100755 t/t3106-ls-tree-pattern.sh\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex 0240adb8eec..39346409f2f 100644\n--- a/Documentation/git-ls-tree.txt\n+++ b/Documentation/git-ls-tree.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n-\t [--name-only] [--name-status] [--object-only] [--full-name] [--full-tree] [--abbrev[=]] [--format=]\n+\t [--name-only] [--name-status] [--object-only] [--full-name] [--full-tree] [--abbrev[=]] [--format=] [--pattern=]\n \t [...]\n \n DESCRIPTION\n@@ -93,6 +93,11 @@ OPTIONS\n \tformat-altering options, including `--long`, `--name-only`\n \tand `--object-only`.\n \n+--pattern=::\n+ The is a string of regular expression format used to\n+ match each entry. Unmatched entries will be filtered and not\n+ dump to the output.\n+\n [...]::\n \tWhen paths are given, show them (note that this isn't really raw\n \tpathnames, but rather a list of patterns to match). Otherwise\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 03dd3fbcb26..576fc9ad16f 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -13,6 +13,7 @@\n #include \"builtin.h\"\n #include \"parse-options.h\"\n #include \"pathspec.h\"\n+#include \n \n static int line_termination = '\\n';\n #define LS_RECURSIVE 1\n@@ -25,6 +26,7 @@ static int chomp_prefix;\n static const char *ls_tree_prefix;\n static const char *format;\n static const char *pattern;\n+static regex_t *regex;\n struct show_tree_data {\n \tunsigned mode;\n \tenum object_type type;\n@@ -47,29 +49,29 @@ static enum ls_tree_cmdmode {\n \tMODE_OBJECT_ONLY,\n } cmdmode;\n \n-__attribute__((unused))\n static int match_pattern(const char *line)\n {\n \tint ret = 0;\n-\tregex_t r;\n \tregmatch_t m[1];\n \tchar errbuf[64];\n \n-\tret = regcomp(&r, pattern, 0);\n-\tif (ret) {\n-\t\tregerror(ret, &r, errbuf, sizeof(errbuf));\n-\t\tdie(\"failed regcomp() for pattern '%s' (%s)\", pattern, errbuf);\n+\tif (!regex) {\n+\t\tregex = xmalloc(sizeof(*regex));\n+\t\tret = regcomp(regex, pattern, 0);\n+\t\tif (ret) {\n+\t\t\tregerror(ret, regex, errbuf, sizeof(errbuf));\n+\t\t\tdie(\"failed regcomp() for pattern '%s' (%s)\", pattern, errbuf);\n+\t\t}\n \t}\n-\tret = regexec(&r, line, 1, m, 0);\n+\n+\tret = regexec(regex, line, 1, m, 0);\n \tif (ret) {\n \t\tif (ret == REG_NOMATCH)\n-\t\t\tgoto cleanup;\n-\t\tregerror(ret, &r, errbuf, sizeof(errbuf));\n+\t\t\treturn ret;\n+\t\tregerror(ret, regex, errbuf, sizeof(errbuf));\n \t\tdie(\"failed regexec() for subject '%s' (%s)\", line, errbuf);\n \t}\n \n-cleanup:\n-\tregfree(&r);\n \treturn ret;\n }\n \n@@ -194,8 +196,12 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \n \tbaselen = base->len;\n \tstrbuf_expand(&sb, format, expand_show_tree, &data);\n-\tstrbuf_addch(&sb, line_termination);\n-\tfwrite(sb.buf, sb.len, 1, stdout);\n+\n+\tif (!pattern || !match_pattern(sb.buf)) {\n+\t\tstrbuf_addch(&sb, line_termination);\n+\t\tfwrite(sb.buf, sb.len, 1, stdout);\n+\t}\n+\n \tstrbuf_release(&sb);\n \tstrbuf_setlen(base, baselen);\n \treturn recurse;\n@@ -232,19 +238,33 @@ static int show_tree_common(struct show_tree_data *data, int *recurse,\n static void show_tree_common_default_long(struct show_tree_data *data)\n {\n \tint base_len = data->base->len;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint sb_len = 0;\n \n \tif (data->size_text)\n-\t\tprintf(\"%06o %s %s %7s\\t\", data->mode, type_name(data->type),\n-\t\t find_unique_abbrev(data->oid, abbrev), data->size_text);\n+\t\tstrbuf_addf(&sb, \"%06o %s %s %7s\\t\", data->mode,\n+\t\t\t type_name(data->type),\n+\t\t\t find_unique_abbrev(data->oid, abbrev),\n+\t\t\t data->size_text);\n \telse\n-\t\tprintf(\"%06o %s %s\\t\", data->mode, type_name(data->type),\n-\t\t find_unique_abbrev(data->oid, abbrev));\n+\t\tstrbuf_addf(&sb, \"%06o %s %s\\t\", data->mode,\n+\t\t\t type_name(data->type),\n+\t\t\t find_unique_abbrev(data->oid, abbrev));\n \n \tstrbuf_addstr(data->base, data->pathname);\n-\twrite_name_quoted_relative(data->base->buf,\n-\t\t\t\t chomp_prefix ? ls_tree_prefix : NULL, stdout,\n-\t\t\t\t line_termination);\n+\tsb_len = sb.len;\n+\tstrbuf_addbuf(&sb, data->base);\n+\n+\tif (!pattern || !match_pattern(sb.buf)) {\n+\t\tstrbuf_setlen(&sb, sb_len);\n+\t\tprintf(\"%s\", sb.buf);\n+\t\twrite_name_quoted_relative(data->base->buf,\n+\t\t\t\t\t chomp_prefix ? ls_tree_prefix : NULL,\n+\t\t\t\t\t stdout, line_termination);\n+\t}\n \tstrbuf_setlen(data->base, base_len);\n+\n+\tstrbuf_release(&sb);\n }\n \n static int show_tree_default(const struct object_id *oid, struct strbuf *base,\n@@ -306,9 +326,11 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,\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+\tif (!pattern || !match_pattern(base->buf)) {\n+\t\twrite_name_quoted_relative(base->buf,\n+\t\t\t\t\t chomp_prefix ? ls_tree_prefix : NULL,\n+\t\t\t\t\t stdout, line_termination);\n+\t}\n \tstrbuf_setlen(base, baselen);\n \treturn recurse;\n }\n@@ -320,12 +342,14 @@ static int show_tree_object(const struct object_id *oid, struct strbuf *base,\n \tint early;\n \tint recurse;\n \tstruct show_tree_data data = { 0 };\n+\tconst char *oid_text = find_unique_abbrev(oid, abbrev);\n \n \tearly = show_tree_common(&data, &recurse, oid, base, pathname, mode);\n \tif (early >= 0)\n \t\treturn early;\n \n-\tprintf(\"%s%c\", find_unique_abbrev(oid, abbrev), line_termination);\n+\tif (!pattern || !match_pattern(oid_text))\n+\t\tprintf(\"%s%c\", oid_text, line_termination);\n \treturn recurse;\n }\n \n@@ -391,8 +415,10 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\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\t\t\t\t N_(\"format to use for the output\"),\n-\t\t\t\t\t PARSE_OPT_NONEG),\n+\t\t\t N_(\"format to use for the output\"),\n+\t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"pattern\", &pattern, \"pattern\",\n+\t\t\t \"pattern to use to match the output\"),\n \t\tOPT__ABBREV(&abbrev),\n \t\tOPT_END()\n \t};\n@@ -430,10 +456,12 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(ls_tree_usage, ls_tree_options);\n \tif (get_oid(argv[0], &oid))\n \t\tdie(\"Not a valid object name %s\", argv[0]);\n+\tif (pattern && !strlen(pattern))\n+\t\tdie(\"Not a valid pattern, the value is empty\");\n \n \t/*\n \t * show_recursive() rolls its own matching code and is\n-\t * generally ignorant of 'struct pathspec'. The magic mask\n+\t * generally ignorant f 'struct pathspec'. The magic mask\n \t * cannot be lifted until it is converted to use\n \t * match_pathspec() or tree_entry_interesting()\n \t */\ndiff --git a/t/t3106-ls-tree-pattern.sh b/t/t3106-ls-tree-pattern.sh\nnew file mode 100755\nindex 00000000000..e4a81c8c47e\n--- /dev/null\n+++ b/t/t3106-ls-tree-pattern.sh\n@@ -0,0 +1,70 @@\n+#!/bin/sh\n+\n+test_description='ls-tree pattern'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-t3100.sh\n+\n+test_expect_success 'setup' '\n+\tsetup_basic_ls_tree_data\n+'\n+\n+test_expect_success 'ls-tree pattern usage' '\n+\ttest_expect_code 129 git ls-tree --pattern HEAD &&\n+\ttest_expect_code 128 git ls-tree --pattern \"\" HEAD >err 2>&1 &&\n+\tgrep \"Not a valid pattern, the value is empty\" err\n+'\n+\n+test_expect_success 'combine with \"--object-only\"' '\n+\tcat > expect <<-EOF &&\n+\t\t6da7993\n+\tEOF\n+\n+\tgit ls-tree --object-only --abbrev=7 --pattern \"6da7993\" HEAD > actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'combine with \"--name-only\"' '\n+\tcat > expect <<-EOF &&\n+\t\t.gitmodules\n+\t\ttop-file.t\n+\tEOF\n+\n+\tgit ls-tree --name-only --pattern \"\\.\" HEAD > actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'combine with \"--long\"' '\n+\tcat > expect <<-EOF &&\n+\t\t100644 blob 6da7993 61\t.gitmodules\n+\t\t100644 blob 02dad95 9\ttop-file.t\n+\tEOF\n+\tgit ls-tree --long --abbrev=7 --pattern \"blob\" HEAD > actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'combine with \"--format\"' '\n+\t# Change the output format by replacing space separators with asterisks.\n+\tformat=\"%(objectmode)*%(objecttype)*%(objectname)%x09%(path)\" &&\n+\tpattern=\"100644\\*blob\" &&\n+\n+\tcat > expect <<-EOF &&\n+\t\t100644*blob*6da7993\t.gitmodules\n+\t\t100644*blob*02dad95\ttop-file.t\n+\tEOF\n+\n+\tgit ls-tree --abbrev=7 --format \"$format\" --pattern \"$pattern\" HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'default output format (only with \"--pattern\" option)' '\n+\tcat > expect <<-EOF &&\n+\t\t100644 blob 6da7993ca1a3435f63c598464f77bdc6dae15aa5\t.gitmodules\n+\t\t100644 blob 02dad956d9274a70f7fafe45a5ffa2e123acd9a8\ttop-file.t\n+\tEOF\n+\tgit ls-tree --pattern \"blob\" HEAD > actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.38.1.426.g770fc8806cb.dirty\n\n"},{"id":"467437","messageId":"221117.86k03tiudl.gmgdl@evledraar.gmail.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"Re: [RFC PATCH 0/6] ls-tree: introduce '--pattern' option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T13:22:20Z","receivedAt":"2022-11-17T13:27:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 17 2022, Teng Long wrote:\n\n> From: Teng Long \n>\n> This RFC patch introduce a new \"ls-tree\" option \"--pattern\", aim to match the\n> entries by regex then filter the output which we may want to achieve. It also\n> contains some commit for preparation or cleanup.\n>\n> The idea may be not comprehensive and the tests for it might be insufficient\n> too, but I'd like to listen the suggestion from the community to decide if it's\n> worth going forward with.\n\nI applied this series, compiled with CFLAGS=-O3, and:\n\t\n\t$ hyperfine './git ls-tree --pattern=[ab]c.*d -r HEAD' './git ls-tree -r HEAD | grep [ab]c.*d' -w 10 -r 20\n\tBenchmark 1: ./git ls-tree --pattern=[ab]c.*d -r HEAD\n\t Time (mean ± σ): 14.8 ms ± 0.1 ms [User: 12.0 ms, System: 2.8 ms]\n\t Range (min … max): 14.6 ms … 15.0 ms 20 runs\n\t\n\tBenchmark 2: ./git ls-tree -r HEAD | grep [ab]c.*d\n\t Time (mean ± σ): 12.5 ms ± 0.1 ms [User: 10.0 ms, System: 4.0 ms]\n\t Range (min … max): 12.4 ms … 12.8 ms 20 runs\n\t\n\tSummary\n\t './git ls-tree -r HEAD | grep [ab]c.*d' ran\n\t 1.18 ± 0.01 times faster than './git ls-tree --pattern=[ab]c.*d -r HEAD'\n\nSo the value-proposition isn't really clear to me, and the included\ndocs, commit messages & this CL don't answer the \"why not just 'grep'\"\nquestion?\n\nThat's faster even with another process for me, but likely that's\nbecause you're doing the regex matching really inefficiently\n(e.g. malloc-ing again for each line), which could be \"fixed\".\n\nBut in any setup which cares about the performance you're likely piping\nto another process anyway (the thing using the data), which could do\nthat filtering without thep \"grep\" process.\n\nSo I don't see the value in doing this, but maybe I'm just missing\nsomething.\n\nAnd, in terms of the complexity for git's implementation it would be\nreally good to avoid the complexity of a \"--pattern\", \"--sort-lines\"\netc., if those use-cases can be satisfied by piping into \"grep\" or\n\"sort\" instead.\n\nSome of the pre-cleanup here looks good, but it's unrelated to the rest\nof the series. I think in any case that it would be nice to see that as\nanother topic.\n"},{"id":"467438","messageId":"RFC-patch-1.4-2d8bcfe2cab-20221117T134528Z-avarab@gmail.com","threadId":"58810","inReplyTo":"RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com","subject":"[RFC PATCH 1/4] ls-tree: don't use \"show_tree_data\" for \"fast\" callbacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T13:48:14Z","receivedAt":"2022-11-17T13:48:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As 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---\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.38.0.1473.g172bcc0511c\n\n"},{"id":"467439","messageId":"RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com","threadId":"58810","inReplyTo":"20221117113023.65865-1-tenglong.tl@alibaba-inc.com","subject":"[RFC PATCH 0/4] ls-tree: pass state in struct, not globals","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T13:48:13Z","receivedAt":"2022-11-17T13:48:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"These are patches I've been carrying locally since April-ish, as a\nfollow-up to the \"ls-tree --format\" topic.\n\nI'm submitting them here in reply to Teng's parallel RFC[1].\n\nTeng: This conflicts with your topic, but re my suggestion of\nsubmitting a separate clean-up series in [2] maybe you could look this\nover, see how they differ from yours, and see what would make sense to\nkeep/incorporate for such a clean-up series?\n\nE.g. 1/4 here is the opposite approach of your 3/6[3], but as 3/4\neventually shows we don't need that struct for anything except that\ncallback case.\n\n1. https://lore.kernel.org/git/20221117113023.65865-1-tenglong.tl@alibaba-inc.com/\n2. https://lore.kernel.org/git/221117.86k03tiudl.gmgdl@evledraar.gmail.com/\n3. https://lore.kernel.org/git/20221117113023.65865-4-tenglong.tl@alibaba-inc.com/\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 | 255 +++++++++++++++++++++++++++-------------------\n 1 file changed, 149 insertions(+), 106 deletions(-)\n\n-- \n2.38.0.1473.g172bcc0511c\n\n"},{"id":"467440","messageId":"RFC-patch-3.4-28c07464b03-20221117T134528Z-avarab@gmail.com","threadId":"58810","inReplyTo":"RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com","subject":"[RFC PATCH 3/4] ls-tree: fold \"show_tree_data\" into \"cb\" struct","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T13:48:16Z","receivedAt":"2022-11-17T13:48:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"After 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 \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.38.0.1473.g172bcc0511c\n\n"},{"id":"467441","messageId":"RFC-patch-2.4-b8ec3172557-20221117T134528Z-avarab@gmail.com","threadId":"58810","inReplyTo":"RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com","subject":"[RFC PATCH 2/4] ls-tree: use a \"struct options\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T13:48:15Z","receivedAt":"2022-11-17T13:48:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As 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 \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 [] [...]\"),\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.38.0.1473.g172bcc0511c\n\n"},{"id":"467442","messageId":"RFC-patch-4.4-6efba6223a2-20221117T134528Z-avarab@gmail.com","threadId":"58810","inReplyTo":"RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com","subject":"[RFC PATCH 4/4] ls-tree: make \"line_termination\" less generic","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T13:48:17Z","receivedAt":"2022-11-17T13:48:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"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 \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.38.0.1473.g172bcc0511c\n\n"},{"id":"467443","messageId":"221117.86fsehisrw.gmgdl@evledraar.gmail.com","threadId":"58810","inReplyTo":"20221117113023.65865-5-tenglong.tl@alibaba-inc.com","subject":"Re: [RFC PATCH 4/6] ls-tree: improving cohension in the print code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T13:53:24Z","receivedAt":"2022-11-17T14:02:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 17 2022, Teng Long wrote:\n\n> From: Teng Long \n>\n> We use 'show_tree_long()' and 'show_tree_default()' to print entries\n> when we want the output in 'default' or 'default long' formats.\n>\n> Function 'show_tree_common_default_long()' prints the \"path\" field as\n> the last part of output, the previous part which separately implements\n> in 'show_tree_long()' and 'show_tree_default()'.\n>\n> We can package the separate implementation together by the extension of\n> \"size_text\" in struct \"show_tree_data\". By improving the cohesion in\n> these two locations, some benefits such as uniform processing of the\n> output can be achieved in future.\n>\n> Signed-off-by: Teng Long \n> ---\n> builtin/ls-tree.c | 14 ++++++++++----\n> 1 file changed, 10 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index afb65af4280..7661170f7ca 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -30,6 +30,7 @@ struct show_tree_data {\n> \tconst struct object_id *oid;\n> \tconst char *pathname;\n> \tstruct strbuf *base;\n> +\tchar *size_text;\n> };\n> \n> static const char * const ls_tree_usage[] = {\n> @@ -186,6 +187,7 @@ static int show_tree_common(struct show_tree_data *data, int *recurse,\n> \tdata->oid = oid;\n> \tdata->pathname = pathname;\n> \tdata->base = base;\n> +\tdata->size_text = NULL;\n> \n> \tif (type == OBJ_BLOB) {\n> \t\tif (ls_options & LS_TREE_ONLY)\n> @@ -204,6 +206,13 @@ static void show_tree_common_default_long(struct show_tree_data *data)\n> {\n> \tint base_len = data->base->len;\n> \n> +\tif (data->size_text)\n> +\t\tprintf(\"%06o %s %s %7s\\t\", data->mode, type_name(data->type),\n> +\t\t find_unique_abbrev(data->oid, abbrev), data->size_text);\n> +\telse\n> +\t\tprintf(\"%06o %s %s\\t\", data->mode, type_name(data->type),\n> +\t\t find_unique_abbrev(data->oid, abbrev));\n> +\n> \tstrbuf_addstr(data->base, data->pathname);\n> \twrite_name_quoted_relative(data->base->buf,\n> \t\t\t\t chomp_prefix ? ls_tree_prefix : NULL, stdout,\n> @@ -223,8 +232,6 @@ static int show_tree_default(const struct object_id *oid, struct strbuf *base,\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(&data);\n> \treturn recurse;\n> }\n> @@ -253,8 +260,7 @@ 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> +\tdata.size_text = size_text;\n> \tshow_tree_common_default_long(&data);\n> \treturn recurse;\n> }\n\nI think this is a good example of how not to split up commits.\n\nSo, in this case the reader is left wondering what the point is, how\ndoes having two callers, and introducing an exra parameter to flip\nbetween what one or the other wants make sense for a \"common\" function?\n\nIt doesn't, that code should just belong in those callers, so this is\njust making things more indirect.\n\nWe find out later in the series that the real reason is that this\neventually becomes an append to a \"struct strbuf\".\n\nAt that point we need to re-write these lines again, so in terms of\nreviewers having to look at it they'd wonder here, and then look a that\ncode again...\n\nThis is better just either squashed into a 6/6, or in an explicit\n\"here's some refactoring to make a subsequent change smaller\", which\ne.g. could move to using the strbuf already, which we'd later make \"real\" use of.\n\n\n"},{"id":"467444","messageId":"221117.86bkp5ispx.gmgdl@evledraar.gmail.com","threadId":"58810","inReplyTo":"20221117113023.65865-6-tenglong.tl@alibaba-inc.com","subject":"Re: [RFC PATCH 5/6] ls-tree: introduce 'match_pattern()' function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T14:02:34Z","receivedAt":"2022-11-17T14:03:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 17 2022, Teng Long wrote:\n\n> From: Teng Long \n>\n> In preparation for actually implementing the '--pattern' option, we\n> add a new method called 'match_pattern' that uses regular expressions\n> to match 'ls-tree' entities.\n>\n> Signed-off-by: Teng Long \n> ---\n> builtin/ls-tree.c | 27 +++++++++++++++++++++++++++\n> 1 file changed, 27 insertions(+)\n>\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 7661170f7ca..03dd3fbcb26 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -24,6 +24,7 @@ static struct pathspec pathspec;\n> static int chomp_prefix;\n> static const char *ls_tree_prefix;\n> static const char *format;\n> +static const char *pattern;\n> struct show_tree_data {\n> \tunsigned mode;\n> \tenum object_type type;\n> @@ -46,6 +47,32 @@ static enum ls_tree_cmdmode {\n> \tMODE_OBJECT_ONLY,\n> } cmdmode;\n> \n> +__attribute__((unused))\n\nThis isn't portable (we'd need a check in git-compat-util.h, and in any\ncase just squashing this whole commit into 6/6 where it actually gets\nused would be better.\n\n> +static int match_pattern(const char *line)\n> +{\n> +\tint ret = 0;\n> +\tregex_t r;\n> +\tregmatch_t m[1];\n> +\tchar errbuf[64];\n> +\n> +\tret = regcomp(&r, pattern, 0);\n> +\tif (ret) {\n> +\t\tregerror(ret, &r, errbuf, sizeof(errbuf));\n> +\t\tdie(\"failed regcomp() for pattern '%s' (%s)\", pattern, errbuf);\n> +\t}\n> +\tret = regexec(&r, line, 1, m, 0);\n> +\tif (ret) {\n> +\t\tif (ret == REG_NOMATCH)\n> +\t\t\tgoto cleanup;\n> +\t\tregerror(ret, &r, errbuf, sizeof(errbuf));\n> +\t\tdie(\"failed regexec() for subject '%s' (%s)\", line, errbuf);\n> +\t}\n> +\n> +cleanup:\n> +\tregfree(&r);\n> +\treturn ret;\n> +}\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\n"},{"id":"467445","messageId":"221117.867cztisk0.gmgdl@evledraar.gmail.com","threadId":"58810","inReplyTo":"20221117113023.65865-7-tenglong.tl@alibaba-inc.com","subject":"Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-17T14:03:11Z","receivedAt":"2022-11-17T14:06:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 17 2022, Teng Long wrote:\n\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 03dd3fbcb26..576fc9ad16f 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -13,6 +13,7 @@\n> #include \"builtin.h\"\n> #include \"parse-options.h\"\n> #include \"pathspec.h\"\n> +#include \n\nAside from anything else I've mentionded (e.g. overall goals), don't\ninclude \"<>\" headers in anything except git-compat-util.h and similar.\n\nIn this case we don't need this at all, but if we did it should be added\nthere...\n\n> [...]\n> \t}\n> -\tret = regexec(&r, line, 1, m, 0);\n> +\n> +\tret = regexec(regex, line, 1, m, 0);\n\nSome whitespace-churn after the last commit...\n\n> static void show_tree_common_default_long(struct show_tree_data *data)\n> {\n> \tint base_len = data->base->len;\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tint sb_len = 0;\n\nIt's size_t, not int, so if you need to keep track of a strbuf's length\nuse the type of its \"len\".\n\nThis would be better named \"oldlen\" or something...\n\n> +\t\tprintf(\"%s\", sb.buf);\n\nputs(sb.buf) instead;\n\n> +\tif (pattern && !strlen(pattern))\n\npattern && !*pattern is more idiomatic IMO.\n\n> +test_expect_success 'combine with \"--object-only\"' '\n> +\tcat > expect <<-EOF &&\n\nStyle: No \" \" after \">\"\n> +\t\t6da7993\n\nStyle: Let's avoid \\t\\t indenting for here-docs, just use the indenting\nof the \"cat\".\n"},{"id":"467469","messageId":"Y3ave2+kEwLTvtE6@nand.local","threadId":"58810","inReplyTo":"221117.86k03tiudl.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH 0/6] ls-tree: introduce '--pattern' option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-17T22:02:35Z","receivedAt":"2022-11-17T22:04:41Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Nov 17, 2022 at 02:22:20PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n> On Thu, Nov 17 2022, Teng Long wrote:\n>\n> > From: Teng Long \n> >\n> > This RFC patch introduce a new \"ls-tree\" option \"--pattern\", aim to match the\n> > entries by regex then filter the output which we may want to achieve. It also\n> > contains some commit for preparation or cleanup.\n> >\n> > The idea may be not comprehensive and the tests for it might be insufficient\n> > too, but I'd like to listen the suggestion from the community to decide if it's\n> > worth going forward with.\n>\n> I applied this series, compiled with CFLAGS=-O3, and:\n>\n> \t$ hyperfine './git ls-tree --pattern=[ab]c.*d -r HEAD' './git ls-tree -r HEAD | grep [ab]c.*d' -w 10 -r 20\n> \tBenchmark 1: ./git ls-tree --pattern=[ab]c.*d -r HEAD\n> \t Time (mean ± σ): 14.8 ms ± 0.1 ms [User: 12.0 ms, System: 2.8 ms]\n> \t Range (min … max): 14.6 ms … 15.0 ms 20 runs\n>\n> \tBenchmark 2: ./git ls-tree -r HEAD | grep [ab]c.*d\n> \t Time (mean ± σ): 12.5 ms ± 0.1 ms [User: 10.0 ms, System: 4.0 ms]\n> \t Range (min … max): 12.4 ms … 12.8 ms 20 runs\n>\n> \tSummary\n> \t './git ls-tree -r HEAD | grep [ab]c.*d' ran\n> \t 1.18 ± 0.01 times faster than './git ls-tree --pattern=[ab]c.*d -r HEAD'\n>\n> So the value-proposition isn't really clear to me, and the included\n> docs, commit messages & this CL don't answer the \"why not just 'grep'\"\n> question?\n>\n> That's faster even with another process for me, but likely that's\n> because you're doing the regex matching really inefficiently\n> (e.g. malloc-ing again for each line), which could be \"fixed\".\n>\n> But in any setup which cares about the performance you're likely piping\n> to another process anyway (the thing using the data), which could do\n> that filtering without thep \"grep\" process.\n>\n> So I don't see the value in doing this, but maybe I'm just missing\n> something.\n\nI think this falls into the same trap as the series on 'git show-ref\n--count' that I worked on earlier this year [1].\n\nAt the time, it seemed useful to me (since I was working in an\nenvironment where counting the number of lines from 'show-ref' was more\ncumbersome than teaching 'show-ref' how to perform the same task\nitself).\n\nAnd I stand by that value judgement, but sharing the patches with the\nGit mailing list under the intent to merge it in was wrong. Those\npatches were too niche to be more generally useful, and would only serve\nto clutter up the UI of show-ref for everybody else.\n\nSo I was glad to drop that topic. Now, I'd be curious to hear from Teng\nwhether or not there *is* something that we're missing, since if so, I\ndefinitely want to know what it is.\n\nBut absent of that, I tend to agree with Ævar that I'm not compelled by\nreplacing 'ls-tree | grep ' with 'ls-tree --pattern=',\nespecially if the latter is slower than the former.\n\nThanks,\nTaylor\n\n[1]: https://lore.kernel.org/git/cover.1654552560.git.me@ttaylorr.com/\n"},{"id":"467672","messageId":"20221121114150.3473-1-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"Y3ave2+kEwLTvtE6@nand.local","subject":"Re: [RFC PATCH 0/6] ls-tree: introduce '--pattern' option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-21T11:41:50Z","receivedAt":"2022-11-21T11:42:53Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Taylor Blau writes:\n\n> I think this falls into the same trap as the series on 'git show-ref\n> --count' that I worked on earlier this year [1].\n>\n> At the time, it seemed useful to me (since I was working in an\n> environment where counting the number of lines from 'show-ref' was more\n> cumbersome than teaching 'show-ref' how to perform the same task\n> itself).\n>\n> And I stand by that value judgement, but sharing the patches with the\n> Git mailing list under the intent to merge it in was wrong. Those\n> patches were too niche to be more generally useful, and would only serve\n> to clutter up the UI of show-ref for everybody else.\n>\n> So I was glad to drop that topic. Now, I'd be curious to hear from Teng\n> whether or not there *is* something that we're missing, since if so, I\n> definitely want to know what it is.\n>\n> But absent of that, I tend to agree with Ævar that I'm not compelled by\n> replacing 'ls-tree | grep ' with 'ls-tree --pattern=',\n> especially if the latter is slower than the former.\n>\n> Thanks,\n> Taylor\n>\n> [1]: https://lore.kernel.org/git/cover.1654552560.git.me@ttaylorr.com/\n\nhonestly, I just think it's useful to me, but omit the performance recession of\nthe option. I originally thought about it looks like the \"git tag -l \"\nor \"git branch -l \" usage, but it seems not as a regex matching on\nthem and it indeed executes faster than the pipe grep, because it seems like the\nformer has the more restrictive matching conditions (because if I move the\nlast aster， there is no output):\n\n\n✗ git branch -r --list \"avar*\" | wc -l\n 1498\n\n✗ hyperfine 'git branch -r --list \"avar*\"'\nBenchmark 1: git branch -r --list \"avar*\"\n Time (mean ± σ): 69.8 ms ± 3.1 ms [User: 25.8 ms, System: 42.7 ms]\n Range (min … max): 66.6 ms … 81.8 ms 35 runs\n\n✗ hyperfine 'git branch -r --list | grep \"avar\"'\nBenchmark 1: git branch -r --list | grep \"avar\"\n Time (mean ± σ): 76.4 ms ± 3.7 ms [User: 32.7 ms, System: 45.2 ms]\n Range (min … max): 72.9 ms … 85.5 ms 34 runs\n\n➜ Documentation git:(tl/extra_bitmap_relative_path) ✗ git branch -r --list \"avar\" | wc -l\n 0\n\nSo unless someone finds other benefits, such as the implementation of \"git tag\n-l \" to solve the performance problem, this method can bring benefits\nin the corresponding scenario. It's ok for me to continue or stop this patch.\n\nThanks.\n"},{"id":"467673","messageId":"20221121120001.5466-1-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com","subject":"[RFC PATCH 0/4] ls-tree: pass state in struct, not globals","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-11-21T12:00:01Z","receivedAt":"2022-11-21T12:00:18Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\"Ævar Arnfjörð Bjarmason\" write:\n\n\n> These are patches I've been carrying locally since April-ish, as a\n> follow-up to the \"ls-tree --format\" topic.\n\nCool.\n\n> Teng: This conflicts with your topic, but re my suggestion of\n> submitting a separate clean-up series in [2] maybe you could look this\n> over, see how they differ from yours, and see what would make sense to\n> keep/incorporate for such a clean-up series?\n\nYes, I'd like to.\n\n> E.g. 1/4 here is the opposite approach of your 3/6[3], but as 3/4\n> eventually shows we don't need that struct for anything except that\n> callback case.\n\nOk, I will check it out later.\n\n\nThanks.\n"},{"id":"467679","messageId":"221121.8635accxea.gmgdl@evledraar.gmail.com","threadId":"58810","inReplyTo":"20221121114150.3473-1-tenglong.tl@alibaba-inc.com","subject":"Re: [RFC PATCH 0/6] ls-tree: introduce '--pattern' option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-21T12:12:08Z","receivedAt":"2022-11-21T12:20:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 21 2022, Teng Long wrote:\n\n> Taylor Blau writes:\n>\n>> I think this falls into the same trap as the series on 'git show-ref\n>> --count' that I worked on earlier this year [1].\n>>\n>> At the time, it seemed useful to me (since I was working in an\n>> environment where counting the number of lines from 'show-ref' was more\n>> cumbersome than teaching 'show-ref' how to perform the same task\n>> itself).\n>>\n>> And I stand by that value judgement, but sharing the patches with the\n>> Git mailing list under the intent to merge it in was wrong. Those\n>> patches were too niche to be more generally useful, and would only serve\n>> to clutter up the UI of show-ref for everybody else.\n>>\n>> So I was glad to drop that topic. Now, I'd be curious to hear from Teng\n>> whether or not there *is* something that we're missing, since if so, I\n>> definitely want to know what it is.\n>>\n>> But absent of that, I tend to agree with Ævar that I'm not compelled by\n>> replacing 'ls-tree | grep ' with 'ls-tree --pattern=',\n>> especially if the latter is slower than the former.\n>>\n>> Thanks,\n>> Taylor\n>>\n>> [1]: https://lore.kernel.org/git/cover.1654552560.git.me@ttaylorr.com/\n>\n> honestly, I just think it's useful to me, but omit the performance recession of\n> the option. I originally thought about it looks like the \"git tag -l \"\n> or \"git branch -l \" usage, but it seems not as a regex matching on\n> them and it indeed executes faster than the pipe grep, because it seems like the\n> former has the more restrictive matching conditions (because if I move the\n> last aster， there is no output):\n>\n>\n> ✗ git branch -r --list \"avar*\" | wc -l\n> 1498\n>\n> ✗ hyperfine 'git branch -r --list \"avar*\"'\n> Benchmark 1: git branch -r --list \"avar*\"\n> Time (mean ± σ): 69.8 ms ± 3.1 ms [User: 25.8 ms, System: 42.7 ms]\n> Range (min … max): 66.6 ms … 81.8 ms 35 runs\n>\n> ✗ hyperfine 'git branch -r --list | grep \"avar\"'\n> Benchmark 1: git branch -r --list | grep \"avar\"\n> Time (mean ± σ): 76.4 ms ± 3.7 ms [User: 32.7 ms, System: 45.2 ms]\n> Range (min … max): 72.9 ms … 85.5 ms 34 runs\n>\n> ➜ Documentation git:(tl/extra_bitmap_relative_path) ✗ git branch -r --list \"avar\" | wc -l\n> 0\n\nYeah, that's the built-in wildmatch() (as in wildmatch.c\nin-tree)-powered matching we do, and support in pretty much anything\nthat takes a .\n\nI *thought* this feature was something like that that when I first\nglanced at it, i.e. to regex match ls-tree entries, but e.g. a\n--pattern=abc would still match that in the OID, as it's just grepping\nthe line.\n\nWhich I think is one way to draw the \"does this belong in git?\"\nline. I.e. a \"grep\" or your \"--pattern\" doesn't need to know anything\nabout sub-components of the line to be equivalent, whereas something\nthat just matches the path does.\n\nAlso, when you get into e.g. negative pathspecs and the like it becomes\neven more compelling.\n\nI do have some WIP patches somewhere to have our pathspecs support PCRE\nregexes.\n\nSo I think it would be neat, or at least worth exploring. But just\nhaving ls-tree or some random command support it isn't the righ way to\ngo about it IMO, it should be done via the pathspec API, if done at all.\n"},{"id":"468235","messageId":"221130.86pmd4ixqj.gmgdl@evledraar.gmail.com","threadId":"58810","inReplyTo":"20221117113023.65865-6-tenglong.tl@alibaba-inc.com","subject":"Re: [RFC PATCH 5/6] ls-tree: introduce 'match_pattern()' function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-30T09:39:07Z","receivedAt":"2022-11-30T09:43:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 17 2022, Teng Long wrote:\n\n(I saw this in the latest \"seen\" push-out)\n\n> From: Teng Long \n> [...]\n> +__attribute__((unused))\n> +static int match_pattern(const char *line)\n> +{\n> +\tint ret = 0;\n> +\tregex_t r;\n> +\tregmatch_t m[1];\n\nHere we hardcode the size of \"m\".\n\n(Re-arranged a bit)\n\n> +\n> +\tret = regcomp(&r, pattern, 0);\n> +\tif (ret) {\n> +\t\tregerror(ret, &r, errbuf, sizeof(errbuf));\n> +\t\tdie(\"failed regcomp() for pattern '%s' (%s)\", pattern, errbuf);\n\nNeeds _().\n\n> +\t}\n> +\tret = regexec(&r, line, 1, m, 0);\n\nHere we hardcode that \"1\" again, but we should use ARRAY_SIZE()\ninstead. See an existing example at:\n\n\tgit grep -W regexec.*ARRAY\n\n(Re-arranged from above)\n\n> +\tchar errbuf[64];\n\nThis is a super short errbuf, in other cases we hardcode this to 1024,\nwhich seems reasonable.\n"},{"id":"468878","messageId":"2q985o75-p6ro-3319-rqos-004621r0p7pq@tzk.qr","threadId":"58810","inReplyTo":"20221117113023.65865-7-tenglong.tl@alibaba-inc.com","subject":"Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-12-12T08:32:38Z","receivedAt":"2022-12-12T08:32:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 17 Nov 2022, Teng Long wrote:\n\n> diff --git a/t/t3106-ls-tree-pattern.sh b/t/t3106-ls-tree-pattern.sh\n> new file mode 100755\n> index 00000000000..e4a81c8c47e\n> --- /dev/null\n> +++ b/t/t3106-ls-tree-pattern.sh\n> @@ -0,0 +1,70 @@\n> +#!/bin/sh\n> +\n> +test_description='ls-tree pattern'\n> +\n> +TEST_PASSES_SANITIZE_LEAK=true\n> +. ./test-lib.sh\n> +. \"$TEST_DIRECTORY\"/lib-t3100.sh\n> +\n> +test_expect_success 'setup' '\n> +\tsetup_basic_ls_tree_data\n> +'\n> +\n> +test_expect_success 'ls-tree pattern usage' '\n> +\ttest_expect_code 129 git ls-tree --pattern HEAD &&\n> +\ttest_expect_code 128 git ls-tree --pattern \"\" HEAD >err 2>&1 &&\n> +\tgrep \"Not a valid pattern, the value is empty\" err\n> +'\n> +\n> +test_expect_success 'combine with \"--object-only\"' '\n> +\tcat > expect <<-EOF &&\n> +\t\t6da7993\n> +\tEOF\n> +\n> +\tgit ls-tree --object-only --abbrev=7 --pattern \"6da7993\" HEAD > actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'combine with \"--name-only\"' '\n> +\tcat > expect <<-EOF &&\n> +\t\t.gitmodules\n> +\t\ttop-file.t\n> +\tEOF\n> +\n> +\tgit ls-tree --name-only --pattern \"\\.\" HEAD > actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'combine with \"--long\"' '\n> +\tcat > expect <<-EOF &&\n> +\t\t100644 blob 6da7993 61\t.gitmodules\n> +\t\t100644 blob 02dad95 9\ttop-file.t\n> +\tEOF\n> +\tgit ls-tree --long --abbrev=7 --pattern \"blob\" HEAD > actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'combine with \"--format\"' '\n> +\t# Change the output format by replacing space separators with asterisks.\n> +\tformat=\"%(objectmode)*%(objecttype)*%(objectname)%x09%(path)\" &&\n> +\tpattern=\"100644\\*blob\" &&\n> +\n> +\tcat > expect <<-EOF &&\n> +\t\t100644*blob*6da7993\t.gitmodules\n> +\t\t100644*blob*02dad95\ttop-file.t\n> +\tEOF\n> +\n> +\tgit ls-tree --abbrev=7 --format \"$format\" --pattern \"$pattern\" HEAD >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'default output format (only with \"--pattern\" option)' '\n> +\tcat > expect <<-EOF &&\n> +\t\t100644 blob 6da7993ca1a3435f63c598464f77bdc6dae15aa5\t.gitmodules\n> +\t\t100644 blob 02dad956d9274a70f7fafe45a5ffa2e123acd9a8\ttop-file.t\n> +\tEOF\n> +\tgit ls-tree --pattern \"blob\" HEAD > actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n\nThe hard-coded object IDs break the `linux-sha256` job, as pointed out in\nhttps://github.com/git/git/blob/6ab7651d8669/whats-cooking.txt#L522-L537.\n\nPlease squash this in to address this (Junio, please feel free to\ncherry-pick this on top of `tl/ls-tree--pattern` to reduce the number of\nCI failures):\n\n-- snipsnap --\nSubject: [PATCH] fixup! ls-tree: introduce '--pattern' option\n\nWe need to avoid hard-coding OIDs because of the SHA-256 support.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3106-ls-tree-pattern.sh | 23 +++++++++++++++--------\n 1 file changed, 15 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t3106-ls-tree-pattern.sh b/t/t3106-ls-tree-pattern.sh\nindex e4a81c8c47e..8aaca307804 100755\n--- a/t/t3106-ls-tree-pattern.sh\n+++ b/t/t3106-ls-tree-pattern.sh\n@@ -17,11 +17,12 @@ test_expect_success 'ls-tree pattern usage' '\n '\n\n test_expect_success 'combine with \"--object-only\"' '\n+\toid=\"$(git rev-parse --short HEAD:.gitmodules)\" &&\n \tcat > expect <<-EOF &&\n-\t\t6da7993\n+\t\t$oid\n \tEOF\n\n-\tgit ls-tree --object-only --abbrev=7 --pattern \"6da7993\" HEAD > actual &&\n+\tgit ls-tree --object-only --abbrev=7 --pattern \"$oid\" HEAD > actual &&\n \ttest_cmp expect actual\n '\n\n@@ -36,22 +37,26 @@ test_expect_success 'combine with \"--name-only\"' '\n '\n\n test_expect_success 'combine with \"--long\"' '\n+\toid1=\"$(git rev-parse --short HEAD:.gitmodules)\" &&\n+\toid2=\"$(git rev-parse --short HEAD:top-file.t)\" &&\n \tcat > expect <<-EOF &&\n-\t\t100644 blob 6da7993 61\t.gitmodules\n-\t\t100644 blob 02dad95 9\ttop-file.t\n+\t\t100644 blob $oid1 61\t.gitmodules\n+\t\t100644 blob $oid2 9\ttop-file.t\n \tEOF\n \tgit ls-tree --long --abbrev=7 --pattern \"blob\" HEAD > actual &&\n \ttest_cmp expect actual\n '\n\n test_expect_success 'combine with \"--format\"' '\n+\toid1=\"$(git rev-parse --short HEAD:.gitmodules)\" &&\n+\toid2=\"$(git rev-parse --short HEAD:top-file.t)\" &&\n \t# Change the output format by replacing space separators with asterisks.\n \tformat=\"%(objectmode)*%(objecttype)*%(objectname)%x09%(path)\" &&\n \tpattern=\"100644\\*blob\" &&\n\n \tcat > expect <<-EOF &&\n-\t\t100644*blob*6da7993\t.gitmodules\n-\t\t100644*blob*02dad95\ttop-file.t\n+\t\t100644*blob*$oid1\t.gitmodules\n+\t\t100644*blob*$oid2\ttop-file.t\n \tEOF\n\n \tgit ls-tree --abbrev=7 --format \"$format\" --pattern \"$pattern\" HEAD >actual &&\n@@ -59,9 +64,11 @@ test_expect_success 'combine with \"--format\"' '\n '\n\n test_expect_success 'default output format (only with \"--pattern\" option)' '\n+\toid1=\"$(git rev-parse HEAD:.gitmodules)\" &&\n+\toid2=\"$(git rev-parse HEAD:top-file.t)\" &&\n \tcat > expect <<-EOF &&\n-\t\t100644 blob 6da7993ca1a3435f63c598464f77bdc6dae15aa5\t.gitmodules\n-\t\t100644 blob 02dad956d9274a70f7fafe45a5ffa2e123acd9a8\ttop-file.t\n+\t\t100644 blob $oid1\t.gitmodules\n+\t\t100644 blob $oid2\ttop-file.t\n \tEOF\n \tgit ls-tree --pattern \"blob\" HEAD > actual &&\n \ttest_cmp expect actual\n--\n2.38.1.windows.1.24.g5ff6506c583.dirty\n\n"},{"id":"468965","messageId":"xmqqtu20qinx.fsf@gitster.g","threadId":"58810","inReplyTo":"2q985o75-p6ro-3319-rqos-004621r0p7pq@tzk.qr","subject":"Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-12T23:57:54Z","receivedAt":"2022-12-12T23:58:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> The hard-coded object IDs break the `linux-sha256` job, as pointed out in\n> https://github.com/git/git/blob/6ab7651d8669/whats-cooking.txt#L522-L537.\n>\n> Please squash this in to address this (Junio, please feel free to\n> cherry-pick this on top of `tl/ls-tree--pattern` to reduce the number of\n> CI failures):\n\nThese days, I gather topics with known CI breakages near the tip of\n'seen', and push out only the good bottom half of 'seen' until such\na topic gets rerolled, at which time it gets added back to the set\nof topics pushed out on 'seen' again (and then ejected if it CI\nbreaks). I excluded the part with the topic from last night's\npushout.\n\nBy the way do you know anything about xterm-256color error in win\ntest(6)?\n\nhttps://github.com/git/git/actions/runs/3676139624/jobs/6216575838#step:5:196\n\nI do not think we hard-code any specific terminal name (other than\ndumb and possibly vt100) in our tests or binaries, so it may be\ncoming from the CI runner environment---some parts incorrectly think\nxterm-256color is available there while there is no support for the\nparticular terminal?\n\nThanks.\n"},{"id":"469001","messageId":"xmqqbko6y2q5.fsf@gitster.g","threadId":"58810","inReplyTo":"xmqqtu20qinx.fsf@gitster.g","subject":"Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-14T05:27:14Z","receivedAt":"2022-12-14T05:27:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>> The hard-coded object IDs break the `linux-sha256` job, as pointed out in\n>> https://github.com/git/git/blob/6ab7651d8669/whats-cooking.txt#L522-L537.\n>>\n>> Please squash this in to address this (Junio, please feel free to\n>> cherry-pick this on top of `tl/ls-tree--pattern` to reduce the number of\n>> CI failures):\n\nAfter re-reading the patches, I am very much inclined to drop this\ntopic, which does not add much value to the system and adds an odd\ncorner case in the UI.\n\nWho needs \"git ls-tree --pattern='blob 486' HEAD\" that is a synonym\nto \"git ls-tree HEAD | grep 'blob 486'\"? Should we end up adding\nthe same option to \"git ls-files\", \"git status\", \"git ls-remote\",\n\"git remote\", \"git branch --list\", etc. for consistency?\n\nThis is simply insane and goes directly against the \"one tool does\none job well, and can be combined with other such tools via pipe\",\nwhich is a key to scale the usability of a set of tools.\n"},{"id":"469008","messageId":"221214.86edt247xk.gmgdl@evledraar.gmail.com","threadId":"58810","inReplyTo":"xmqqbko6y2q5.fsf@gitster.g","subject":"Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-14T10:03:39Z","receivedAt":"2022-12-14T10:05:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 14 2022, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n>> Johannes Schindelin writes:\n>>\n>>> The hard-coded object IDs break the `linux-sha256` job, as pointed out in\n>>> https://github.com/git/git/blob/6ab7651d8669/whats-cooking.txt#L522-L537.\n>>>\n>>> Please squash this in to address this (Junio, please feel free to\n>>> cherry-pick this on top of `tl/ls-tree--pattern` to reduce the number of\n>>> CI failures):\n>\n> After re-reading the patches, I am very much inclined to drop this\n> topic, which does not add much value to the system and adds an odd\n> corner case in the UI.\n>\n> Who needs \"git ls-tree --pattern='blob 486' HEAD\" that is a synonym\n> to \"git ls-tree HEAD | grep 'blob 486'\"? Should we end up adding\n> the same option to \"git ls-files\", \"git status\", \"git ls-remote\",\n> \"git remote\", \"git branch --list\", etc. for consistency?\n>\n> This is simply insane and goes directly against the \"one tool does\n> one job well, and can be combined with other such tools via pipe\",\n> which is a key to scale the usability of a set of tools.\n\nI agree, and FWIW I read\nhttps://lore.kernel.org/git/20221121114150.3473-1-tenglong.tl@alibaba-inc.com/\nas the submitter of the topic agreeing to drop it ~3 weeks ago.\n"},{"id":"469009","messageId":"xmqqbko6w9qj.fsf@gitster.g","threadId":"58810","inReplyTo":"221214.86edt247xk.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-14T10:38:44Z","receivedAt":"2022-12-14T10:38:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> This is simply insane and goes directly against the \"one tool does\n>> one job well, and can be combined with other such tools via pipe\",\n>> which is a key to scale the usability of a set of tools.\n>\n> I agree, and FWIW I read\n> https://lore.kernel.org/git/20221121114150.3473-1-tenglong.tl@alibaba-inc.com/\n> as the submitter of the topic agreeing to drop it ~3 weeks ago.\n\nI actually read \"it's useful to me\" as resisting, but I already\ndiscarded the topic (not just \"I stopped merging it to 'seen'\", but\n\"I no longer have the topic branch with me\").\n\nThanks.\n"},{"id":"469431","messageId":"20221221114737.51375-1-tenglong.tl@alibaba-inc.com","threadId":"58810","inReplyTo":"RFC-patch-1.4-2d8bcfe2cab-20221117T134528Z-avarab@gmail.com","subject":"[RFC PATCH 1/4] ls-tree: don't use \"show_tree_data\" for \"fast\" callbacks","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-12-21T11:47:37Z","receivedAt":"2022-12-21T11:47:48Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\"Ævar Arnfjörð Bjarmason\" writes:\n\n> As noted in [1] the code that made it in as part of\n> 9c4d58ff2c3 (ls-tree: split up \"fast path\" callbacks, 2022-03-23) was\n> a \"maybe a good idea, maybe not\" RFC-quality patch. I hadn't looked\n> very carefully at the resulting patterns.\n>\n> The implementation shared the \"struct show_tree_data data\", which was\n> introduced in e81517155e0 (ls-tree: introduce struct \"show_tree_data\",\n> 2022-03-23) both for use in 455923e0a15 (ls-tree: introduce \"--format\"\n> option, 2022-03-23), and because the \"fat\" callback hadn't been split\n> up as 9c4d58ff2c3 did.\n>\n> Now that that's been done we can see that most of what\n> show_tree_common() was doing could be done lazily by the callbacks\n> themselves, who in the pre-image were often using an odd mis-match of\n> their own arguments and those same arguments stuck into the \"data\"\n> structure. Let's also have the callers initialize the \"type\", rather\n> than grabbing it from the \"data\" structure afterwards.\n>\n> 1. https://lore.kernel.org/git/cover-0.7-00000000000-20220310T134811Z-avarab@gmail.com/\n\nBecause in \"show_tree_common(&data, &recurse, oid, base, pathname, mode)\", the\n\"data\" and the other args exist redundant, we could just not to pass \"data\",\nbecause it's enough, do I understand right?\n\nThanks.\n"},{"id":"474230","messageId":"d566eb68-6476-a193-2acb-10106a154d3d@gmx.de","threadId":"58810","inReplyTo":"xmqqtu20qinx.fsf@gitster.g","subject":"win-test: unknown terminal \"xterm-256color\", was Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-03-27T10:37:33Z","receivedAt":"2023-03-27T10:38:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio\n\nOn Tue, 13 Dec 2022, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > The hard-coded object IDs break the `linux-sha256` job, as pointed out in\n> > https://github.com/git/git/blob/6ab7651d8669/whats-cooking.txt#L522-L537.\n> >\n> > Please squash this in to address this (Junio, please feel free to\n> > cherry-pick this on top of `tl/ls-tree--pattern` to reduce the number of\n> > CI failures):\n>\n> These days, I gather topics with known CI breakages near the tip of\n> 'seen', and push out only the good bottom half of 'seen' until such\n> a topic gets rerolled, at which time it gets added back to the set\n> of topics pushed out on 'seen' again (and then ejected if it CI\n> breaks). I excluded the part with the topic from last night's\n> pushout.\n>\n> By the way do you know anything about xterm-256color error in win\n> test(6)?\n>\n> https://github.com/git/git/actions/runs/3676139624/jobs/6216575838#step:5:196\n\nUnfortunately by the time I got back to this mail, the log had expired.\nHere is a link to the same symptom in a newer build:\n\nhttps://github.com/git/git/actions/runs/4523517641/jobs/7966768829#step:6:63\n\n> I do not think we hard-code any specific terminal name (other than\n> dumb and possibly vt100) in our tests or binaries, so it may be\n> coming from the CI runner environment---some parts incorrectly think\n> xterm-256color is available there while there is no support for the\n> particular terminal?\n\nThe TERM is hard-coded in the MSYS2 runtime:\nhttps://github.com/git-for-windows/msys2-runtime/commit/bd627864ab4189984cdb0892c00f91e39c4e8243\n\nNote: The MSYS2 runtime merely wants to ensure that `TERM` is set; If it\nalready has a value, that value remains unchanged.\n\nAnd to save on bandwidth/time (in a desperate attempt to counter the\never-growing runtimes of Git's CI builds), I liberally exclude files from\nthe \"minimal subset of Git for Windows' SDK\", e.g. `/usr/lib/terminfo/`\nand `/usr/share/terminfo/`. That's why `tput` cannot figure out what to do\nwith this `TERM` value.\n\nIf these `tput` errors become too much of a sore in your eye, I see two\nways forward:\n\n- Set `TERM=dumb` for the Windows jobs\n\n- Use a simple shim like this one in `ci/` (and maybe even in\n `t/test-lib.sh`):\n\n tput() {\n \tprintf '\\e[%sm%s'\"$(test sgr0 != $1 || echo '\\x0f')\" \"$( \\\n\t\tcase $1 in\n\t\tbold) echo 1;;\n\t\tdim) echo 2;;\n\t\trev) echo 7;;\n\t\tsetaf) echo 3$2;;\n\t\tsetab) echo 4$2;;\n\t\tesac \\\n\t)\"\n }\n\nPersonally, I do not really want to work on this, not before much bigger\nfish are fed first. For example, the friction regarding the CI build times\nis becoming quite crushing.\n\nCiao,\nJohannes\n"},{"id":"474258","messageId":"xmqqzg7ylye4.fsf@gitster.g","threadId":"58810","inReplyTo":"d566eb68-6476-a193-2acb-10106a154d3d@gmx.de","subject":"Re: win-test: unknown terminal \"xterm-256color\", was Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-27T20:42:11Z","receivedAt":"2023-03-27T20:42:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> The TERM is hard-coded in the MSYS2 runtime:\n> https://github.com/git-for-windows/msys2-runtime/commit/bd627864ab4189984cdb0892c00f91e39c4e8243\n>\n> Note: The MSYS2 runtime merely wants to ensure that `TERM` is set; If it\n> already has a value, that value remains unchanged.\n>\n> And to save on bandwidth/time (in a desperate attempt to counter the\n> ever-growing runtimes of Git's CI builds), I liberally exclude files from\n> the \"minimal subset of Git for Windows' SDK\", e.g. `/usr/lib/terminfo/`\n> and `/usr/share/terminfo/`. That's why `tput` cannot figure out what to do\n> with this `TERM` value.\n\nAh, so it is not like \"you ship vt100 but not xterm-color yet the\nruntime insists you are by default on xterm-color\", so \"set it to\nsomething your terminfo database understands\" would not work. I am\nnot sure what unintended fallouts there would be from setting it to\ndumb, though. Our tests are designed for unattended testing, and\nthey are even capable of running without control terminal, so it\nshould be OK to force TERM=dumb, I guess.\n\n> If these `tput` errors become too much of a sore in your eye, I see two\n> ways forward:\n\nNot at all. As long as it is clear that they are to be expected and\nto be ignored when fishing for real breakages, it is fine by me.\nOthers may care more than I do, but then they are welcome to send\nfixes your way ;-)\n\nThanks.\n"},{"id":"474349","messageId":"20230328180859.GB18558@coredump.intra.peff.net","threadId":"58810","inReplyTo":"xmqqzg7ylye4.fsf@gitster.g","subject":"Re: win-test: unknown terminal \"xterm-256color\", was Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-28T18:08:59Z","receivedAt":"2023-03-28T18:09:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 27, 2023 at 01:42:11PM -0700, Junio C Hamano wrote:\n\n> > And to save on bandwidth/time (in a desperate attempt to counter the\n> > ever-growing runtimes of Git's CI builds), I liberally exclude files from\n> > the \"minimal subset of Git for Windows' SDK\", e.g. `/usr/lib/terminfo/`\n> > and `/usr/share/terminfo/`. That's why `tput` cannot figure out what to do\n> > with this `TERM` value.\n> \n> Ah, so it is not like \"you ship vt100 but not xterm-color yet the\n> runtime insists you are by default on xterm-color\", so \"set it to\n> something your terminfo database understands\" would not work. I am\n> not sure what unintended fallouts there would be from setting it to\n> dumb, though. Our tests are designed for unattended testing, and\n> they are even capable of running without control terminal, so it\n> should be OK to force TERM=dumb, I guess.\n\nWe already force TERM=dumb in test-lib.sh, so the tests themselves\nshould behave the same. The original terminal is only used for colorized\noutput from the test harness itself. But I don't think we'd colorize\nanyway in CI, since stdout is not a terminal (and we tend to further go\nthrough \"prove\" anyway).\n\nSo it should be fine to just set TERM=dumb everywhere. What actually\nconfuses me is that we try to do so already:\n\n $ git grep -B1 dumb ci/lib.sh\n ci/lib.sh-# GitHub Action doesn't set TERM, which is required by tput\n ci/lib.sh:export TERM=${TERM:-dumb}\n\nPushing a stripped-down workflow file to just run \"echo $TERM\" shows\nthat it seems to already be set by Actions to \"dumb\" on ubuntu-latest,\nbut is xterm-256color on windows-latest.\n\nSo maybe we just want to make the line above unconditionally set $TERM?\n\n-Peff\n"},{"id":"474356","messageId":"xmqqbkkcfz9s.fsf@gitster.g","threadId":"58810","inReplyTo":"20230328180859.GB18558@coredump.intra.peff.net","subject":"Re: win-test: unknown terminal \"xterm-256color\", was Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-28T19:31:59Z","receivedAt":"2023-03-28T19:32:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So it should be fine to just set TERM=dumb everywhere. What actually\n> confuses me is that we try to do so already:\n>\n> $ git grep -B1 dumb ci/lib.sh\n> ci/lib.sh-# GitHub Action doesn't set TERM, which is required by tput\n> ci/lib.sh:export TERM=${TERM:-dumb}\n>\n> Pushing a stripped-down workflow file to just run \"echo $TERM\" shows\n> that it seems to already be set by Actions to \"dumb\" on ubuntu-latest,\n> but is xterm-256color on windows-latest.\n>\n> So maybe we just want to make the line above unconditionally set $TERM?\n\nI thought that Dscho earlier said xterm-256 is set by mingw when\nnothing is set to TERM, which explains why TERM=${TERM:-dumb} is\nnot good enough to \"fix\" this one for them.\n\nI am fine with an unconditional assignment for CI.\n\nThanks.\n"},{"id":"474359","messageId":"20230328195942.GA1173031@coredump.intra.peff.net","threadId":"58810","inReplyTo":"xmqqbkkcfz9s.fsf@gitster.g","subject":"Re: win-test: unknown terminal \"xterm-256color\", was Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-28T19:59:42Z","receivedAt":"2023-03-28T20:06:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 28, 2023 at 12:31:59PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > So it should be fine to just set TERM=dumb everywhere. What actually\n> > confuses me is that we try to do so already:\n> >\n> > $ git grep -B1 dumb ci/lib.sh\n> > ci/lib.sh-# GitHub Action doesn't set TERM, which is required by tput\n> > ci/lib.sh:export TERM=${TERM:-dumb}\n> >\n> > Pushing a stripped-down workflow file to just run \"echo $TERM\" shows\n> > that it seems to already be set by Actions to \"dumb\" on ubuntu-latest,\n> > but is xterm-256color on windows-latest.\n> >\n> > So maybe we just want to make the line above unconditionally set $TERM?\n> \n> I thought that Dscho earlier said xterm-256 is set by mingw when\n> nothing is set to TERM, which explains why TERM=${TERM:-dumb} is\n> not good enough to \"fix\" this one for them.\n\nAh, I took it to mean that in our code, we'd default to xterm256-color.\nBut perhaps it is the outer bash which is doing so, via that runtime. I\ntested with a dummy workflow like:\n\n name: fake ci\n on: [push, pull_request]\n jobs:\n ubuntu:\n runs-on: ubuntu-latest\n steps:\n - name: check TERM\n\t- run: |\n\t echo TERM=$TERM\n windows:\n runs-on: windows-latest\n steps:\n - name: check TERM\n shell: bash\n run: |\n echo TERM=$TERM\n\nand got \"dumb\" for ubuntu (which is not set by us; this is before\nci/lib.sh is run) and \"xterm256-color\" for windows (I guess because it's\nmingw bash).\n\nAt any rate, unconditionally setting TERM=dumb should cover all cases,\nI'd think. I'm happy to prepare a patch.\n\n-Peff\n"},{"id":"474370","messageId":"20230328204318.GA1274197@coredump.intra.peff.net","threadId":"58810","inReplyTo":"20230328195942.GA1173031@coredump.intra.peff.net","subject":"Re: win-test: unknown terminal \"xterm-256color\", was Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-03-28T20:43:18Z","receivedAt":"2023-03-28T20:43:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 28, 2023 at 03:59:42PM -0400, Jeff King wrote:\n\n> At any rate, unconditionally setting TERM=dumb should cover all cases,\n> I'd think. I'm happy to prepare a patch.\n\nHmph. So I started to do this, and notice there are a bunch of tput\ncalls in the ci/ scripts themselves.\n\nSo if I do a dummy workflow like this:\n\n-- >8 --\nname: fake ci\non: [push, pull_request]\njobs:\n ubuntu:\n runs-on: ubuntu-latest\n steps:\n - name: check TERM\n run: |\n echo TERM=$TERM\n echo $(tput setaf 1)this is setaf 1$(tput sgr0)\n windows:\n runs-on: windows-latest\n steps:\n - name: check TERM\n shell: bash\n run: |\n echo TERM=$TERM\n echo $(tput setaf 1)this is setaf 1$(tput sgr0)\n-- 8< --\n\nthen I note two interesting things:\n\n - the attempted colorization does nothing on ubuntu, because it's\n already TERM=dumb\n\n - the colorization on windows works! But I thought it was exactly\n these sorts of tputs that are causing the error messages.\n\nSo now I'm more confused than ever. I have a feeling we would be better\noff just using ansi codes instead of tput, as Johannes suggested\nearlier. But given the twists and turns here, and the fact that nobody\nreally seems that bothered by the status quo, I'm inclined to just leave\nit alone for now.\n\n-Peff\n"},{"id":"474379","messageId":"xmqqfs9oege8.fsf@gitster.g","threadId":"58810","inReplyTo":"20230328204318.GA1274197@coredump.intra.peff.net","subject":"Re: win-test: unknown terminal \"xterm-256color\", was Re: [RFC PATCH 6/6] ls-tree: introduce '--pattern' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-28T21:05:03Z","receivedAt":"2023-03-28T21:05:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> then I note two interesting things:\n>\n> - the attempted colorization does nothing on ubuntu, because it's\n> already TERM=dumb\n\nThis is expected, I think.\n\n> - the colorization on windows works! But I thought it was exactly\n> these sorts of tputs that are causing the error messages.\n\nYeah, tput complains about unknown terminal as terminfo database is\nnot shipped. But perhaps it falls back to bog-standard ANSI after\ndoing its complaining? I dunno.\n\n> ... I'm inclined to just leave it alone for now.\n\nThat's fine, too.\n\nThanks.\n\n"}]}