{"thread":{"id":"56905","subject":"[PATCH 0/3] support `--oid-only` in `ls-tree`","startedAt":"2021-11-15T11:52:34Z","lastAt":"2021-12-17T13:30:40Z","messageCount":55,"participants":["Teng Long","Ævar Arnfjörð Bjarmason","Đoàn Trần Công Danh","Jeff King","Junio C Hamano","Peter Baumann"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"441133","messageId":"20211115115153.48307-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":null,"subject":"[PATCH 0/3] support `--oid-only` in `ls-tree`","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-15T11:51:50Z","receivedAt":"2021-11-15T11:52:34Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Sometimes, we only want to get the objects from output of `ls-tree`\nand commands like `sed` or `cut` is usually used to intercept the\norigin output to achieve this purpose in practical.\n\nThe patch contains three commits\n\n 1. Implementation of the option.\n 2. Add new tests in \"t3104\".\n 3. Documentation modifications.\n\nI'm appreciate if someone help to review the patch.\n\nThanks.\n\nTeng Long (3):\n ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n t3104: add related tests for `--oid-only` option\n git-ls-tree.txt: description of the 'oid-only' option\n\n Documentation/git-ls-tree.txt | 8 +++--\n builtin/ls-tree.c | 11 +++++++\n t/t3104-ls-tree-oid.sh | 55 +++++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+), 2 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\n-- \n2.33.1.9.g5fbd2fc599.dirty\n\n"},{"id":"441134","messageId":"20211115115153.48307-2-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"20211115115153.48307-1-dyroneteng@gmail.com","subject":"[PATCH 1/3] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-15T11:51:51Z","receivedAt":"2021-11-15T11:52:35Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"This commit supply an option names `--oid-only` to let `git ls-tree`\nonly print out the OID of the object. `--oid-only` and `--name-only`\nare mutually exclusive in use.\n\nSigned-off-by: Teng Long \n---\n builtin/ls-tree.c | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c7..1f82229649 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -20,6 +20,7 @@ static int line_termination = '\\n';\n #define LS_SHOW_TREES 4\n #define LS_NAME_ONLY 8\n #define LS_SHOW_SIZE 16\n+#define LS_OID_ONLY 32\n static int abbrev;\n static int ls_options;\n static struct pathspec pathspec;\n@@ -90,6 +91,14 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n+\tif ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))\n+\t\tdie(_(\"cannot specify --oid-only and --name-only at the same time\"));\n+\n+\tif (ls_options & LS_OID_ONLY) {\n+\t\tprintf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n+\t\treturn 0;\n+\t}\n+\n \tif (!(ls_options & LS_NAME_ONLY)) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n \t\t\tchar size_text[24];\n@@ -139,6 +148,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\tLS_NAME_ONLY),\n \t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n \t\t\tLS_NAME_ONLY),\n+\t\tOPT_BIT(0, \"oid-only\", &ls_options, N_(\"list only oids\"),\n+\t\t\tLS_OID_ONLY),\n \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n-- \n2.33.1.9.g5fbd2fc599.dirty\n\n"},{"id":"441135","messageId":"20211115115153.48307-3-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"20211115115153.48307-1-dyroneteng@gmail.com","subject":"[PATCH 2/3] t3104: add related tests for `--oid-only` option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-15T11:51:52Z","receivedAt":"2021-11-15T11:52:43Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Signed-off-by: Teng Long \n---\n t/t3104-ls-tree-oid.sh | 55 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 55 insertions(+)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\ndiff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\nnew file mode 100755\nindex 0000000000..78ab9127c7\n--- /dev/null\n+++ b/t/t3104-ls-tree-oid.sh\n@@ -0,0 +1,55 @@\n+#!/bin/sh\n+\n+test_description='git ls-tree oids handling.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo 111 >1.txt &&\n+\techo 222 >2.txt &&\n+\tmkdir -p path0/a/b/c &&\n+\techo 333 >path0/a/b/c/3.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=$(git write-tree) &&\n+\techo $tree\n+'\n+\n+\n+test_expect_success 'specify with --oid-only' '\n+\tgit ls-tree --oid-only $tree >current &&\n+\tcat >expected <<\\EOF &&\n+58c9bdf9d017fcd178dc8c073cbfcbb7ff240d6c\n+c200906efd24ec5e783bee7f23b5d7c941b0c12c\n+4e3849a078083863912298a25db30997cb8ca6d6\n+EOF\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'specify with --oid-only and -r' '\n+\tgit ls-tree --oid-only -r $tree >current &&\n+\tcat >expected <<\\EOF &&\n+58c9bdf9d017fcd178dc8c073cbfcbb7ff240d6c\n+c200906efd24ec5e783bee7f23b5d7c941b0c12c\n+55bd0ac4c42e46cd751eb7405e12a35e61425550\n+EOF\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'specify with --oid-only and --abbrev' '\n+\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n+\tcat >expected <<\\EOF &&\n+58c9bd\n+c20090\n+4e3849\n+EOF\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'cannot specify --name-only and --oid-only as the same time' '\n+\ttest_must_fail git ls-tree --oid-only --name-only $tree >current 2>&1 >/dev/null &&\n+\techo \"fatal: cannot specify --oid-only and --name-only at the same time\" > expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_done\n-- \n2.33.1.9.g5fbd2fc599.dirty\n\n"},{"id":"441136","messageId":"20211115115153.48307-4-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"20211115115153.48307-1-dyroneteng@gmail.com","subject":"[PATCH 3/3] git-ls-tree.txt: description of the 'oid-only' option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-15T11:51:53Z","receivedAt":"2021-11-15T11:52:48Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Signed-off-by: Teng Long \n---\n Documentation/git-ls-tree.txt | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex db02d6d79a..bc711dc00a 100644\n--- a/Documentation/git-ls-tree.txt\n+++ b/Documentation/git-ls-tree.txt\n@@ -10,7 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n-\t [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n+\t [--name-only] [--name-status] [--oid-only]\n+\t [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n@@ -59,7 +60,10 @@ OPTIONS\n --name-only::\n --name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n-\n+\tCannot be used with `--oid-only` together.\n+--oid-only::\n+\tList only OIDs of the objects, one per line. Cannot be used with\n+\t`--name-only` or `--name-status` together.\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tlines, show the shortest prefix that is at least ''\n-- \n2.33.1.9.g5fbd2fc599.dirty\n\n"},{"id":"441139","messageId":"211115.86r1bhsb8x.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"20211115115153.48307-2-dyroneteng@gmail.com","subject":"Re: [PATCH 1/3] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-15T15:12:23Z","receivedAt":"2021-11-15T15:14:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nlorn Mon, Nov 15 2021, Teng Long wrote:\n\n> This commit supply an option names `--oid-only` to let `git ls-tree`\n> only print out the OID of the object. `--oid-only` and `--name-only`\n> are mutually exclusive in use.\n>\n> Signed-off-by: Teng Long \n> ---\n> builtin/ls-tree.c | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n>\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 3a442631c7..1f82229649 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -20,6 +20,7 @@ static int line_termination = '\\n';\n> #define LS_SHOW_TREES 4\n> #define LS_NAME_ONLY 8\n> #define LS_SHOW_SIZE 16\n> +#define LS_OID_ONLY 32\n> static int abbrev;\n> static int ls_options;\n> static struct pathspec pathspec;\n> @@ -90,6 +91,14 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> \telse if (ls_options & LS_TREE_ONLY)\n> \t\treturn 0;\n> \n> +\tif ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))\n> +\t\tdie(_(\"cannot specify --oid-only and --name-only at the same time\"));\n\nIf you make these an OPT_CMDMODE you get this behavior for free. See\ne.g. my\nhttps://lore.kernel.org/git/patch-v2-06.10-d945fc94774-20211112T221506Z-avarab@gmail.com/\n"},{"id":"441140","messageId":"211115.86mtm5saz7.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"20211115115153.48307-1-dyroneteng@gmail.com","subject":"Re: [PATCH 0/3] support `--oid-only` in `ls-tree`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-15T15:13:24Z","receivedAt":"2021-11-15T15:19:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 15 2021, Teng Long wrote:\n\n> Sometimes, we only want to get the objects from output of `ls-tree`\n> and commands like `sed` or `cut` is usually used to intercept the\n> origin output to achieve this purpose in practical.\n>\n> The patch contains three commits\n>\n> 1. Implementation of the option.\n> 2. Add new tests in \"t3104\".\n> 3. Documentation modifications.\n>\n> I'm appreciate if someone help to review the patch.\n\nI've looked it over, they look correct mostly, the test code in 2/3\nlooks a bit too complex (using find?).\n\nBut I'd much rather see this be done with adding strbuf_expand() to\nls-tree. I.e. its docs say that it can emit:\n\n SP SP TAB \n\nOr, with -l:\n\n SP SP SP TAB \n\nIf you use strbuf_expand() you can just define a default format of:\n\n %(objectmode) SP %(objecttype) SP %(objectname) TAB %(path)\n\nThen make the existing -l option a shorthand for tweaking that to:\n\n %(objectmode) SP %(objecttype) SP %(objectsize) SP %(objectname) TAB %(path)\n\nThen you can get what you want out of this with a simple:\n\n git ls-tree --format=\"%(objectname)\"\n\nSee e.g. git-cat-file for an existing use of strbuf_expand().\n"},{"id":"441143","messageId":"YZKCmPJzOwLeFhIJ@danh.dev","threadId":"56905","inReplyTo":"20211115115153.48307-3-dyroneteng@gmail.com","subject":"Re: [PATCH 2/3] t3104: add related tests for `--oid-only` option","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-11-15T15:54:00Z","receivedAt":"2021-11-15T15:54:21Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-11-15 19:51:52+0800, Teng Long wrote:\n> Signed-off-by: Teng Long \n> ---\n> t/t3104-ls-tree-oid.sh | 55 ++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 55 insertions(+)\n> create mode 100755 t/t3104-ls-tree-oid.sh\n> \n> diff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\n> new file mode 100755\n> index 0000000000..78ab9127c7\n> --- /dev/null\n> +++ b/t/t3104-ls-tree-oid.sh\n> @@ -0,0 +1,55 @@\n> +#!/bin/sh\n> +\n> +test_description='git ls-tree oids handling.'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +\techo 111 >1.txt &&\n> +\techo 222 >2.txt &&\n> +\tmkdir -p path0/a/b/c &&\n> +\techo 333 >path0/a/b/c/3.txt &&\n> +\tfind *.txt path* \\( -type f -o -type l \\) -print |\n> +\txargs git update-index --add &&\n> +\ttree=$(git write-tree) &&\n> +\techo $tree\n> +'\n> +\n> +\n> +test_expect_success 'specify with --oid-only' '\n> +\tgit ls-tree --oid-only $tree >current &&\n> +\tcat >expected <<\\EOF &&\n> +58c9bdf9d017fcd178dc8c073cbfcbb7ff240d6c\n> +c200906efd24ec5e783bee7f23b5d7c941b0c12c\n> +4e3849a078083863912298a25db30997cb8ca6d6\n> +EOF\n\nFailed with:\n\n\tGIT_TEST_DEFAULT_HASH=sha256 ./t3104-ls-tree-oid.sh\n\nI think we can use:\n\n\tgit ls-tree $tree | awk '{print $3}' >expected\n\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_success 'specify with --oid-only and -r' '\n> +\tgit ls-tree --oid-only -r $tree >current &&\n> +\tcat >expected <<\\EOF &&\n> +58c9bdf9d017fcd178dc8c073cbfcbb7ff240d6c\n> +c200906efd24ec5e783bee7f23b5d7c941b0c12c\n> +55bd0ac4c42e46cd751eb7405e12a35e61425550\n> +EOF\n> +\ttest_cmp current expected\n> +'\n\nDitto for this test and below tests.\n\n> +\n> +test_expect_success 'specify with --oid-only and --abbrev' '\n> +\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n> +\tcat >expected <<\\EOF &&\n> +58c9bd\n> +c20090\n> +4e3849\n> +EOF\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_success 'cannot specify --name-only and --oid-only as the same time' '\n> +\ttest_must_fail git ls-tree --oid-only --name-only $tree >current 2>&1 >/dev/null &&\n\nThe last redirection '>/dev/null' does nothing, me think.\n\n> +\techo \"fatal: cannot specify --oid-only and --name-only at the same time\" > expected &&\n\nStyle nit:\n\n\tuse '>expected' instead of '> expected'\n\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_done\n> -- \n> 2.33.1.9.g5fbd2fc599.dirty\n> \n\n-- \nDanh\n"},{"id":"441152","messageId":"YZKyC7c8b9PnyXG8@coredump.intra.peff.net","threadId":"56905","inReplyTo":"20211115115153.48307-2-dyroneteng@gmail.com","subject":"Re: [PATCH 1/3] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-15T19:16:27Z","receivedAt":"2021-11-15T20:31:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 15, 2021 at 07:51:51PM +0800, Teng Long wrote:\n\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 3a442631c7..1f82229649 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -20,6 +20,7 @@ static int line_termination = '\\n';\n> #define LS_SHOW_TREES 4\n> #define LS_NAME_ONLY 8\n> #define LS_SHOW_SIZE 16\n> +#define LS_OID_ONLY 32\n> static int abbrev;\n> static int ls_options;\n> static struct pathspec pathspec;\n> @@ -90,6 +91,14 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> \telse if (ls_options & LS_TREE_ONLY)\n> \t\treturn 0;\n> \n> +\tif ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))\n> +\t\tdie(_(\"cannot specify --oid-only and --name-only at the same time\"));\n\nThis seems reasonable to me. Letting them overwrite each other (i.e.,\n\"last one wins\") would also be fine, but we can always loosen to that\nbehavior later if we choose.\n\nThis is a somewhat funny place to put the check, though. It will be run\nfor every entry in the tree (so is a tiny bit less efficient, but also\nwould not trigger for an empty tree). It probably should go in\ncmd_ls_tree(), perhaps here:\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 1f82229649..3c9ea00489 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -91,9 +91,6 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))\n-\t\tdie(_(\"cannot specify --oid-only and --name-only at the same time\"));\n-\n \tif (ls_options & LS_OID_ONLY) {\n \t\tprintf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n \t\treturn 0;\n@@ -175,6 +172,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options))\n \t\tls_options |= LS_SHOW_TREES;\n \n+\tif ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))\n+\t\tdie(_(\"cannot specify --oid-only and --name-only at the same time\"));\n+\n \tif (argc < 1)\n \t\tusage_with_options(ls_tree_usage, ls_tree_options);\n \tif (get_oid(argv[0], &oid))\n\nÆvar also mentioned using OPT_CMDMODE(), which I think would naturally\nmove the logic in a similar way.\n\n-Peff\n"},{"id":"441154","messageId":"YZKzpv+n6bHJ2WOs@coredump.intra.peff.net","threadId":"56905","inReplyTo":"20211115115153.48307-1-dyroneteng@gmail.com","subject":"Re: [PATCH 0/3] support `--oid-only` in `ls-tree`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-15T19:23:18Z","receivedAt":"2021-11-15T20:32:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 15, 2021 at 07:51:50PM +0800, Teng Long wrote:\n\n> Sometimes, we only want to get the objects from output of `ls-tree`\n> and commands like `sed` or `cut` is usually used to intercept the\n> origin output to achieve this purpose in practical.\n> \n> The patch contains three commits\n> \n> 1. Implementation of the option.\n> 2. Add new tests in \"t3104\".\n> 3. Documentation modifications.\n> \n> I'm appreciate if someone help to review the patch.\n\nThis seems like a good feature to have. I think it would make sense to\nsquash the three patches into a single one. The documentation and test\npatches do not stand on their own, which is why there was nothing useful\nto say in their commit messages.\n\nThe implementation looks generally sensible (modulo the comments already\ngiven). I was surprised that there was not an existing ls-tree script\nthat these would fit into. But there really isn't; t3101 covers\n--name-only and other output, but is really focused on the pathnames\n(though I think it would be OK to refactor it to cover output more\ngenerally).\n\n-Peff\n"},{"id":"441167","messageId":"211115.86o86lqe3c.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"YZKwduZgH4BEpFzd@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] support `--oid-only` in `ls-tree`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-15T21:50:18Z","receivedAt":"2021-11-15T22:52:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 15 2021, Jeff King wrote:\n\n> On Mon, Nov 15, 2021 at 04:13:24PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> But I'd much rather see this be done with adding strbuf_expand() to\n>> ls-tree. I.e. its docs say that it can emit:\n>\n> I had a similar thought, but that's a much bigger task. I think it would\n> be reasonable to add --oid-only to match the existing --name-only, etc.\n> If we later add a custom --format option, then it can easily be folded\n> in and explained as \"this is an alias for --format=%(objectname)\", just\n> like --name-only would become \"this is an alias for --format=%(path)\".\n\nA quick patch to do it below, seems to work, passes all tests, but I\ndon't know how much I'd trust it. It's also quite an add use of\nstrbuf_expa(). We print to stdout directly since\nwrite_name_quoted_relative() really wants to write to stdout, and not\ngive you a buffer. But I guess it makes sense in a way.\n\nThe hardcoded %7s for %(objectsize) is a bit nasty, but I don't know if\nwe've got anything existing that handles format specifiers with\nstrbuf_expand() that we could steal.\n\nI really wouldn't trust this code much, I found it when writing it that\nour tests for ls-tree are really lacking, e.g. we may not have a single\ntest for \"-l\" anywhere (or maybe I didn't look enough, I was just\nrunning t/*ls*tree* while hacking it.\n\nI do thin that we should consider just going with --format in either\ncase if we agree that this is a good direction. I.e. could just support\n3-4 hardcoded formats now and die if anything else is specified.\n\nThen we'd be future-proof with the same interface expanding later, and\nwouldn't need to support options that we're only carrying because we\ndidn't implement the more generic format support.\n\n(Assume my Signed-off-by, if there's any interest...)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c71..e89daad4229 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -31,6 +31,20 @@ static const char * const ls_tree_usage[] = {\n \tNULL\n };\n \n+static const char *ls_tree_format_d = \"%(objectmode) %(objecttype) %(objectname)\t%(path)\";\n+static const char *ls_tree_format_l = \"%(objectmode) %(objecttype) %(objectname) %(objectsize)\t%(path)\";\n+static const char *ls_tree_format_n = \"%(path)\";\n+\n+struct expand_ls_tree_data {\n+\tconst char *format;\n+\tunsigned mode;\n+\tconst char *type;\n+\tconst struct object_id *oid;\n+\tint abbrev;\n+\tconst char *pathname;\n+\tconst char *basebuf;\n+};\n+\n static int show_recursive(const char *base, int baselen, const char *pathname)\n {\n \tint i;\n@@ -61,9 +75,69 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n \treturn 0;\n }\n \n+static size_t expand_show_tree(struct strbuf *sb,\n+\t\t\t const char *start,\n+\t\t\t void *context)\n+{\n+\tstruct expand_ls_tree_data *data = context;\n+\tconst char *end;\n+\tconst char *p;\n+\tsize_t len;\n+\tconst char *type = blob_type;\n+\n+\tif (sb->len) {\n+\t\tfputs(sb->buf, stdout);\n+\t\tstrbuf_reset(sb);\n+\t}\n+\n+\tif (*start != '(')\n+\t\tdie(_(\"bad format as of '%s'\"), start);\n+\tend = strchr(start + 1, ')');\n+\tif (!end)\n+\t\tdie(_(\"ls-tree format element '%s' does not end in ')'\"),\n+\t\t start);\n+\tlen = end - start + 1;\n+\n+\tif (skip_prefix(start, \"(objectmode)\", &p)) {\n+\t\tprintf(\"%06o\", data->mode);\n+\t} else if (skip_prefix(start, \"(objecttype)\", &p)) {\n+\t\tfputs(data->type, stdout);\n+\t} else if (skip_prefix(start, \"(objectsize)\", &p)) {\n+\t\tchar size_text[24];\n+\t\tconst struct object_id *oid = data->oid;\n+\n+\t\tif (!strcmp(type, blob_type)) {\n+\t\t\tunsigned long size;\n+\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n+\t\t\t\txsnprintf(size_text, sizeof(size_text),\n+\t\t\t\t\t \"BAD\");\n+\t\t\telse\n+\t\t\t\txsnprintf(size_text, sizeof(size_text),\n+\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n+\t\t} else {\n+\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n+\t\t}\n+\t\tprintf(\"%7s\", size_text);\n+\t} else if (skip_prefix(start, \"(objectname)\", &p)) {\n+\t\tfputs(find_unique_abbrev(data->oid, data->abbrev), stdout);\n+\t} else if (skip_prefix(start, \"(path)\", &p)) {\n+\t\twrite_name_quoted_relative(data->basebuf,\n+\t\t\t\t\t chomp_prefix ? ls_tree_prefix : NULL,\n+\t\t\t\t\t stdout, line_termination);\n+\n+\t} else {\n+\t\tunsigned int errlen = (unsigned long)len;\n+\t\tdie(_(\"bad ls-tree format specifiec %%%.*s\"), errlen, start);\t\n+\t}\n+\n+\treturn len;\n+}\n+\n static int show_tree(const struct object_id *oid, struct strbuf *base,\n \t\tconst char *pathname, unsigned mode, void *context)\n {\n+\tstruct expand_ls_tree_data *data = context;\n+\tstruct strbuf sb = STRBUF_INIT;\n \tint retval = 0;\n \tint baselen;\n \tconst char *type = blob_type;\n@@ -90,31 +164,18 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif (!(ls_options & LS_NAME_ONLY)) {\n-\t\tif (ls_options & LS_SHOW_SIZE) {\n-\t\t\tchar size_text[24];\n-\t\t\tif (!strcmp(type, blob_type)) {\n-\t\t\t\tunsigned long size;\n-\t\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n-\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n-\t\t\t\t\t\t \"BAD\");\n-\t\t\t\telse\n-\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n-\t\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n-\t\t\t} else\n-\t\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n-\t\t\tprintf(\"%06o %s %s %7s\\t\", mode, type,\n-\t\t\t find_unique_abbrev(oid, abbrev),\n-\t\t\t size_text);\n-\t\t} else\n-\t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n-\t\t\t find_unique_abbrev(oid, abbrev));\n-\t}\n \tbaselen = base->len;\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+\n+\tstrbuf_reset(&sb);\n+\tdata->mode = mode;\n+\tdata->type = type;\n+\tdata->oid = oid;\n+\tdata->abbrev = abbrev;\n+\tdata->pathname = pathname;\n+\tdata->basebuf = base->buf;\n+\tstrbuf_expand(&sb, data->format, expand_show_tree, data);\n+\n \tstrbuf_setlen(base, baselen);\n \treturn retval;\n }\n@@ -147,6 +208,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\tOPT__ABBREV(&abbrev),\n \t\tOPT_END()\n \t};\n+\tstruct expand_ls_tree_data ls_tree_cb_data = {\n+\t\t.format = ls_tree_format_d,\n+\t};\n \n \tgit_config(git_default_config, NULL);\n \tls_tree_prefix = prefix;\n@@ -161,8 +225,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t}\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 ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options)) {\n \t\tls_options |= LS_SHOW_TREES;\n+\t}\n+\tif (ls_options & LS_NAME_ONLY)\n+\t\tls_tree_cb_data.format = ls_tree_format_n;\n+\n+\tif (ls_options & LS_SHOW_SIZE)\n+\t\tls_tree_cb_data.format = ls_tree_format_l;\n \n \tif (argc < 1)\n \t\tusage_with_options(ls_tree_usage, ls_tree_options);\n@@ -185,6 +255,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \ttree = parse_tree_indirect(&oid);\n \tif (!tree)\n \t\tdie(\"not a tree object\");\n+\n \treturn !!read_tree(the_repository, tree,\n-\t\t\t &pathspec, show_tree, NULL);\n+\t\t\t &pathspec, show_tree, &ls_tree_cb_data);\n }\n"},{"id":"441214","messageId":"YZK0MKCYAJmG+pSU@coredump.intra.peff.net","threadId":"56905","inReplyTo":"YZKyC7c8b9PnyXG8@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-15T19:25:36Z","receivedAt":"2021-11-16T00:29:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 15, 2021 at 02:16:27PM -0500, Jeff King wrote:\n\n> On Mon, Nov 15, 2021 at 07:51:51PM +0800, Teng Long wrote:\n> \n> > diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> > index 3a442631c7..1f82229649 100644\n> > --- a/builtin/ls-tree.c\n> > +++ b/builtin/ls-tree.c\n> > @@ -20,6 +20,7 @@ static int line_termination = '\\n';\n> > #define LS_SHOW_TREES 4\n> > #define LS_NAME_ONLY 8\n> > #define LS_SHOW_SIZE 16\n> > +#define LS_OID_ONLY 32\n> > static int abbrev;\n> > static int ls_options;\n> > static struct pathspec pathspec;\n> > @@ -90,6 +91,14 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> > \telse if (ls_options & LS_TREE_ONLY)\n> > \t\treturn 0;\n> > \n> > +\tif ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))\n> > +\t\tdie(_(\"cannot specify --oid-only and --name-only at the same time\"));\n> \n> This seems reasonable to me. Letting them overwrite each other (i.e.,\n> \"last one wins\") would also be fine, but we can always loosen to that\n> behavior later if we choose.\n\nOh, and whichever direction we go, it would probably make sense for\n--long to be handled in the same way. I.e.:\n\n git ls-tree --long --oid-only\n\ndoes not really make sense. Though we currently just ignore --long for:\n\n git ls-tree --long --name-only\n\nwhich is arguably a bug.\n\n-Peff\n"},{"id":"441219","messageId":"YZKwduZgH4BEpFzd@coredump.intra.peff.net","threadId":"56905","inReplyTo":"211115.86mtm5saz7.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/3] support `--oid-only` in `ls-tree`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-15T19:09:42Z","receivedAt":"2021-11-16T01:13:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 15, 2021 at 04:13:24PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> But I'd much rather see this be done with adding strbuf_expand() to\n> ls-tree. I.e. its docs say that it can emit:\n\nI had a similar thought, but that's a much bigger task. I think it would\nbe reasonable to add --oid-only to match the existing --name-only, etc.\nIf we later add a custom --format option, then it can easily be folded\nin and explained as \"this is an alias for --format=%(objectname)\", just\nlike --name-only would become \"this is an alias for --format=%(path)\".\n\n-Peff\n"},{"id":"441583","messageId":"20211118084557.76846-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"YZKCmPJzOwLeFhIJ@danh.dev","subject":"Re: [PATCH 2/3] t3104: add related tests for `--oid-only` option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-18T08:45:57Z","receivedAt":"2021-11-18T08:46:24Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\non Mon, 15 Nov 2021 22:54:00 +0700, Đoàn Trần Công Danh wrote:\n\nThanks for helping to review this patch.\n\n> Failed with:\n>\n> GIT_TEST_DEFAULT_HASH=sha256 ./t3104-ls-tree-oid.sh\n\nYou totally right and the tests should pass both sha1 and sha256.\n\n> I think we can use:\n>\n> git ls-tree $tree | awk '{print $3}' >expected\n> ...\n> ...\n>\n> Ditto for this test and below tests.\n\nYes, correct and better.\n\nBut should also escape the dollar character to work.\n\n> The last redirection '>/dev/null' does nothing, me think.\n> Style nit:\n>\n>\tuse '>expected' instead of '> expected'\n\nYeah, that's my bad and will fix.\n"},{"id":"441585","messageId":"20211118092803.78395-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"211115.86r1bhsb8x.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/3] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-18T09:28:03Z","receivedAt":"2021-11-18T09:28:20Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\n> If you make these an OPT_CMDMODE you get this behavior for free. See\n> e.g. my\n> https://lore.kernel.org/git/patch-v2-06.10-d945fc94774-20211112T221506Z-avarab@gmail.com/\t \n\nThank you very much for providing this input.\n\nSo I try to read this patch your mentioned and try to repeat the idea in my understanding.\n\nFirst, OPT_CMDMODE() can be used for:\n\n 1. Easy for checking the combined command options, such as \"mutually exclusive\" conditions.\n\n 2. Die and output the error message consistently when the incompatible options are found.\n\n 3. Brings better extensibilites, no need to change a lot of if/elses.\n\nThen, you suggest to consider about to use OPT_CMDMODE instead of the current implementations.\n\nDid I understand your suggestion right and comprehensive?\n"},{"id":"441597","messageId":"211118.86czmx67yi.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"20211118092803.78395-1-dyroneteng@gmail.com","subject":"Re: [PATCH 1/3] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-18T11:00:23Z","receivedAt":"2021-11-18T11:04:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 18 2021, Teng Long wrote:\n\n>> If you make these an OPT_CMDMODE you get this behavior for free. See\n>> e.g. my\n>> https://lore.kernel.org/git/patch-v2-06.10-d945fc94774-20211112T221506Z-avarab@gmail.com/\t \n>\n> Thank you very much for providing this input.\n>\n> So I try to read this patch your mentioned and try to repeat the idea in my understanding.\n>\n> First, OPT_CMDMODE() can be used for:\n>\n> 1. Easy for checking the combined command options, such as \"mutually exclusive\" conditions.\n>\n> 2. Die and output the error message consistently when the incompatible options are found.\n>\n> 3. Brings better extensibilites, no need to change a lot of if/elses.\n>\n> Then, you suggest to consider about to use OPT_CMDMODE instead of the current implementations.\n>\n> Did I understand your suggestion right and comprehensive?\n\nYes, all of that is correct.\n\nIt's a way of defining N options, --foo, --bar, --baz, where combining\nany of them is an error.\n\nWe usually use it for a \"command mode\" (hence the name), but it can be\nused when the command has flags that are mutually exclusive.\n\nI think (but am not sure, and didn't check) that you can even use it for\n--foo AND --bar that are exclusive, and --other --flags that are also\nmutually exclusive (but could be combined with one of --foo or --bar),\nyou just need to provide another variable for it to set.\n\nBut I haven't tested that or used it like that, maybe it doesn't work\nfor some reason I'm forgetting...\n"},{"id":"441599","messageId":"20211118112348.85948-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"YZKyC7c8b9PnyXG8@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-18T11:23:48Z","receivedAt":"2021-11-18T11:25:52Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"On Mon, 15 Nov 2021 14:16:27 -0500, Jeff King wrote:\n\n> This is a somewhat funny place to put the check, though. It will be run\n> for every entry in the tree (so is a tiny bit less efficient, but also\n> would not trigger for an empty tree). It probably should go in\n> cmd_ls_tree(), perhaps here:\n\nYes, it's better here as a fail-fast case.\n\nAccording to the suggestion of the new location I think why not put the logic\nfurther head, after the parse_options() return, like:\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 1f82229649..003a9ade54 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -166,6 +166,10 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \n argc = parse_options(argc, argv, prefix, ls_tree_options,\n ls_tree_usage, 0);\n+\n+ if ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))\n+ die(_(\"cannot specify --oid-only and --name-only at the same time\"));\n+\n if (full_tree) {\n ls_tree_prefix = prefix = NULL;\n chomp_prefix = 0;\n\nWill it bring other new problems?\n\nThank you.\n"},{"id":"441685","messageId":"20211119025749.91014-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"211115.86o86lqe3c.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/3] support `--oid-only` in `ls-tree`","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-19T02:57:49Z","receivedAt":"2021-11-19T02:58:07Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"> A quick patch to do it below, seems to work, passes all tests, but I\n> don't know how much I'd trust it. It's also quite an add use of\n> strbuf_expa(). We print to stdout directly since\n> write_name_quoted_relative() really wants to write to stdout, and not\n> give you a buffer. But I guess it makes sense in a way.\n\nThanks for the patch and the inputs about \"strbuf_expa()\".\n\n> Then we'd be future-proof with the same interface expanding later, and\n> wouldn't need to support options that we're only carrying because we\n> didn't implement the more generic format support.\n\nI agree but like Peff said it maybe another bigger task. I think I will\nfirstly solve the existing problems in next patch.\n\nI will consider about the generic format support but not sure whether\nit will continue to iterate in this patchset.\n\n> (Assume my Signed-off-by, if there's any interest...)\n\nOf course I will.\n\nThank you very much for your advice and guidance again.\n"},{"id":"441716","messageId":"cover.1637321601.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"20211115115153.48307-1-dyroneteng@gmail.com","subject":"[PATCH v2 0/1] support `--oid-only` in `ls-tree`","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-19T12:09:26Z","receivedAt":"2021-11-19T12:10:06Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\nThis patch series supports for only outputing the \"objects\" (OID)\nwith a new option names `--oid-only`.\n\nChanges with the first patch are :\n\n 1. Three commits are squashed to 1 commit (Peff's advice)\n 2. The tests issues (Đoàn Trần Công Danh's advice)\n 3. Use `OPT_CMDMODE()` for mutually exclusive control\n (Ævar Arnfjörð Bjarmason's advice)\n\nSome discussions are not included in Patch 2 :\n\n 1. `git ls-tree --long --name-only` and\n `git ls-tree --long --oid-only` which is arguably a bug\n (Peff's advice)\n 2. Support `--format` for `git-ls-tree`\n (Ævar Arnfjörð Bjarmason's advice)\n\nThe reason why these 2 discussions not included is I'm not sure whether\nI should continue on the current patchset or start a new one. And for the\nsecond, I think current implementation is clear and simple to use, meeting\nthe needs of the moment. Maybe I will to support `--format` option, but\nbefore that, I'm appreciate if there are more suggestions appear.\n\nThanks.\n\nTeng Long (1):\n ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n\n Documentation/git-ls-tree.txt | 8 +++++--\n builtin/ls-tree.c | 27 ++++++++++++++++-------\n t/t3104-ls-tree-oid.sh | 40 +++++++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 10 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\nRange-diff against v1:\n1: c4479178d7 < -: ---------- ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n2: 853ebbcf88 < -: ---------- t3104: add related tests for `--oid-only` option\n3: 33c68c1f11 < -: ---------- git-ls-tree.txt: description of the 'oid-only' option\n-: ---------- > 1: 8b68568d6c ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n-- \n2.33.1.10.g1f74a882e4\n\n"},{"id":"441717","messageId":"8b68568d6cbe379d40c61c48bf446eaa88221df5.1637321601.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1637321601.git.dyroneteng@gmail.com","subject":"[PATCH v2 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-19T12:09:27Z","receivedAt":"2021-11-19T12:10:08Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Sometimes, we only want to get the objects from output of `ls-tree`\nand commands like `sed` or `cut` is usually used to intercept the\norigin output to achieve this purpose in practical.\n\nThis commit supply an option names `--oid-only` to let `git ls-tree`\nonly print out the OID of the object. `--oid-only` and `--name-only`\nare mutually exclusive in use.\n\nReviewed-by: Jeff King \nReviewed-by: Ævar Arnfjörð Bjarmason \nReviewed-by: Đoàn Trần Công Danh \nSigned-off-by: Teng Long \n---\n Documentation/git-ls-tree.txt | 8 +++++--\n builtin/ls-tree.c | 27 ++++++++++++++++-------\n t/t3104-ls-tree-oid.sh | 40 +++++++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 10 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex db02d6d79a..bc711dc00a 100644\n--- a/Documentation/git-ls-tree.txt\n+++ b/Documentation/git-ls-tree.txt\n@@ -10,7 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n-\t [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n+\t [--name-only] [--name-status] [--oid-only]\n+\t [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n@@ -59,7 +60,10 @@ OPTIONS\n --name-only::\n --name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n-\n+\tCannot be used with `--oid-only` together.\n+--oid-only::\n+\tList only OIDs of the objects, one per line. Cannot be used with\n+\t`--name-only` or `--name-status` together.\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tlines, show the shortest prefix that is at least ''\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c7..1e4a82e669 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -18,19 +18,26 @@ static int line_termination = '\\n';\n #define LS_RECURSIVE 1\n #define LS_TREE_ONLY 2\n #define LS_SHOW_TREES 4\n-#define LS_NAME_ONLY 8\n-#define LS_SHOW_SIZE 16\n+#define LS_SHOW_SIZE 8\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 \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 \n+enum {\n+\tMODE_UNSPECIFIED = 0,\n+\tMODE_NAME_ONLY,\n+\tMODE_OID_ONLY\n+};\n+\n+static int cmdmode = MODE_UNSPECIFIED;\n+\n static int show_recursive(const char *base, int baselen, const char *pathname)\n {\n \tint i;\n@@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif (!(ls_options & LS_NAME_ONLY)) {\n+\tif (cmdmode == 2) {\n+\t\tprintf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n+\t\treturn 0;\n+\t}\n+\n+\tif (cmdmode == 0) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n \t\t\tchar size_text[24];\n \t\t\tif (!strcmp(type, blob_type)) {\n@@ -135,10 +147,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n \t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n \t\t\tLS_SHOW_SIZE),\n-\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n-\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n+\t\tOPT_CMDMODE('n', \"name-only\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE('s', \"name-status\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode, N_(\"list only oids\"), MODE_OID_ONLY),\n \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\ndiff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\nnew file mode 100755\nindex 0000000000..4c02cdd3c3\n--- /dev/null\n+++ b/t/t3104-ls-tree-oid.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='git ls-tree oids handling.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo 111 >1.txt &&\n+\techo 222 >2.txt &&\n+\tmkdir -p path0/a/b/c &&\n+\techo 333 >path0/a/b/c/3.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=$(git write-tree) &&\n+\techo $tree\n+'\n+\n+test_expect_success 'usage: --oid-only' '\n+\tgit ls-tree --oid-only $tree >current &&\n+\tgit ls-tree $tree | awk \"{print \\$3}\" >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --oid-only with -r' '\n+\tgit ls-tree --oid-only -r $tree >current &&\n+\tgit ls-tree -r $tree | awk \"{print \\$3}\" >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --oid-only with --abbrev' '\n+\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n+\tgit ls-tree --abbrev=6 $tree | awk \"{print \\$3}\" > expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '\n+\ttest_incompatible_usage git ls-tree --oid-only --name-only\n+'\n+\n+test_done\n-- \n2.33.1.10.g1f74a882e4\n\n"},{"id":"441727","messageId":"211119.86wnl42ri2.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"8b68568d6cbe379d40c61c48bf446eaa88221df5.1637321601.git.dyroneteng@gmail.com","subject":"Re: [PATCH v2 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-19T13:30:52Z","receivedAt":"2021-11-19T13:40:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 19 2021, Teng Long wrote:\n\n> Reviewed-by: Jeff King \n> Reviewed-by: Ævar Arnfjörð Bjarmason \n> Reviewed-by: Đoàn Trần Công Danh \n\nPlease don't add the Reviewed-by headers yourself, either Junio\naccumulates them, or if someone explicitly mentions that you can add it\nwith their name it's OK.\n\nIt doesn't just mean this person reviewed this series in some ML thread,\nbut \"this person is 100% OK with this in its current form\".\n\n> \tList only filenames (instead of the \"long\" output), one per line.\n> -\n> +\tCannot be used with `--oid-only` together.\n\nBetter: \"Cannot be combined with OPT.\"\n\n> +--oid-only::\n> +\tList only OIDs of the objects, one per line. Cannot be used with\n> +\t`--name-only` or `--name-status` together.\n\nBetter: \"Cannot be combined with OPT or OPT2.\"\n\n> +enum {\n> +\tMODE_UNSPECIFIED = 0,\n> +\tMODE_NAME_ONLY,\n> +\tMODE_OID_ONLY\n> +};\n> +\n> +static int cmdmode = MODE_UNSPECIFIED;\n\nBetter:\n\nstatic enum {\n\tMODE_NAME_ONLY = 1,\n ...\n} cmdmode = MODE_NAME_ONLY;\n\nI.e. no need for the MODE_UNSPECIFIED just to skip past \"0\".\n\n> @@ -135,10 +147,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n> \t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n> \t\t\tLS_SHOW_SIZE),\n> -\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n> -\t\t\tLS_NAME_ONLY),\n> -\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n> -\t\t\tLS_NAME_ONLY),\n> +\t\tOPT_CMDMODE('n', \"name-only\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n> +\t\tOPT_CMDMODE('s', \"name-status\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n> +\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode, N_(\"list only oids\"), MODE_OID_ONLY),\n\nBetter to preserve the wrapping here, to stay within 79 columns.\n\n> +test_expect_success 'setup' '\n> +\techo 111 >1.txt &&\n> +\techo 222 >2.txt &&\n\nJust use:\n\n test_commit A &&\n test_commit B\n\netc?\n\n> +\tmkdir -p path0/a/b/c &&\n> +\techo 333 >path0/a/b/c/3.txt &&\n> +\tfind *.txt path* \\( -type f -o -type l \\) -print |\n> +\txargs git update-index --add &&\n> +\ttree=$(git write-tree) &&\n> +\techo $tree\n\nStray echo? Unclear why this test setup is so complex, shouldn't this just be (continued from above):\n\n mkdir -p C &&\n test_commit C/D.txt\n\nTo test nested dirs?\n\n> +'\n> +\n> +test_expect_success 'usage: --oid-only' '\n> +\tgit ls-tree --oid-only $tree >current &&\n> +\tgit ls-tree $tree | awk \"{print \\$3}\" >expected &&\n\n\njust cut -f1 instead of awk? Also don't put \"git\" on the LHS of a pipe,\nit might hide segfaults. Also applies to the below.\n\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_success 'usage: --oid-only with -r' '\n> +\tgit ls-tree --oid-only -r $tree >current &&\n> +\tgit ls-tree -r $tree | awk \"{print \\$3}\" >expected &&\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_success 'usage: --oid-only with --abbrev' '\n> +\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n> +\tgit ls-tree --abbrev=6 $tree | awk \"{print \\$3}\" > expected &&\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '\n> +\ttest_incompatible_usage git ls-tree --oid-only --name-only\n> +'\n\nHrm, did you copy this use of test_incompatible_usage from\nt1006-cat-file.sh without providing the function?\n\nMore data for:\nhttps://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/ :)\n\nBetter to use:\n\n test_expect_code 128 ... # (or was it 129?)\n"},{"id":"441769","messageId":"xmqqy25koxua.fsf@gitster.g","threadId":"56905","inReplyTo":"211119.86wnl42ri2.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-19T17:32:29Z","receivedAt":"2021-11-19T17:32:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\nI think that many points you raised in your message are valid, but\nthere is one thing that is not.\n\n>> +enum {\n>> +\tMODE_UNSPECIFIED = 0,\n>> +\tMODE_NAME_ONLY,\n>> +\tMODE_OID_ONLY\n>> +};\n>> +\n>> +static int cmdmode = MODE_UNSPECIFIED;\n>\n> Better:\n>\n> static enum {\n> \tMODE_NAME_ONLY = 1,\n> ...\n> } cmdmode = MODE_NAME_ONLY;\n>\n> I.e. no need for the MODE_UNSPECIFIED just to skip past \"0\".\n\nIf the original wanted to make the default to be \"unspecified\", your\nsuggestion changes the semantics.\n\n\"enum\" is not necessarily an \"int\", and because the pointer of\n\"cmdmode\" is given to OPT_CMDMODE(), which expects a pointer to\n\"int\", your suggestion breaks the code there, too.\n\nI wonder if cmdmode cannot be a on-stack variable in cmd_ls_tree()\nthat is passed as the context pointer to show_tree() via\nread_tree(), though. The enum definition still need to be visible\nthroughout the file, but such a structure would let us lose a\n\"global\" variable.\n\nThanks.\n\n"},{"id":"441917","messageId":"20211122074538.87255-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"211119.86wnl42ri2.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-22T07:45:38Z","receivedAt":"2021-11-22T07:47:11Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\nOn Fri, 19 Nov 2021 14:30:52 +0100, Ævar Arnfjörð Bjarmason wrote\n\n> Please don't add the Reviewed-by headers yourself, either Junio\n> accumulates them, or if someone explicitly mentions that you can add it\n> with their name it's OK.\n\nI think I misunderstood the meanings of the header before.\nThanks for the important tips.\n\n> Better: \"Cannot be combined with OPT.\"\n> Better: \"Cannot be combined with OPT or OPT2.\"\n> ...\n> Better to preserve the wrapping here, to stay within 79 columns.\n\nWill apply.\n\n> Just use:\n>\n> test_commit A &&\n> test_commit B\n>\n> etc?\n> ...\n> Stray echo? Unclear why this test setup is so complex, shouldn't this just be (continued from above):\n>\n> mkdir -p C &&\n> test_commit C/D.txt\n\n> To test nested dirs?\n\nWill apply.\n\n\n> just cut -f1 instead of awk? Also don't put \"git\" on the LHS of a pipe,\n> it might hide segfaults. Also applies to the below.\n>\n\nWill apply, and could you please describe the problem with more details?\n(appreciate if there is an executable example)\n\nThank you.\n"},{"id":"441921","messageId":"cover.1637567328.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1637321601.git.dyroneteng@gmail.com","subject":"[PATCH v3 0/1] ls-tree.c: support `--oid-only` option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-22T08:07:27Z","receivedAt":"2021-11-22T08:08:55Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Diffs from previous patch:\n\n 1. Remove \"Reviewed-by\" headers in commit message.\n 2. Optimize option descriptions in Doc.\n (Ævar Arnfjörð Bjarmason' advice)\n 3. Optimize and bugfix in \"t3104\".\n \t (Ævar Arnfjörð Bjarmason' advice)\n 4. The formatting problems of line wrappers (over 79 col)\n\nAll the advices are from Ævar Arnfjörð Bjarmason and Junio C Hamano,\nthank you very much.\n\nAlthought some advices are apply in this path, but some questions\nremains, they are in link [1].\n\n[1] https://public-inbox.org/git/20211122074538.87255-1-dyroneteng@gmail.com/\n\nTeng Long (1):\n ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n\n Documentation/git-ls-tree.txt | 8 +++++--\n builtin/ls-tree.c | 27 ++++++++++++++++-------\n t/t3104-ls-tree-oid.sh | 40 +++++++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 10 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\nRange-diff against v2:\n1: 8b68568d6c ! 1: 6c15b4c176 ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n @@ Commit message\n only print out the OID of the object. `--oid-only` and `--name-only`\n are mutually exclusive in use.\n \n - Reviewed-by: Jeff King \n - Reviewed-by: Ævar Arnfjörð Bjarmason \n - Reviewed-by: Đoàn Trần Công Danh \n Signed-off-by: Teng Long \n \n ## Documentation/git-ls-tree.txt ##\n-- \n2.33.1.10.g438dd9044d.dirty\n\n"},{"id":"441922","messageId":"6c15b4c176b7c03072fa59a4efd9f6fea7d62eae.1637567328.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1637567328.git.dyroneteng@gmail.com","subject":"[PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-22T08:07:28Z","receivedAt":"2021-11-22T08:08:57Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Sometimes, we only want to get the objects from output of `ls-tree`\nand commands like `sed` or `cut` is usually used to intercept the\norigin output to achieve this purpose in practical.\n\nThis commit supply an option names `--oid-only` to let `git ls-tree`\nonly print out the OID of the object. `--oid-only` and `--name-only`\nare mutually exclusive in use.\n\nSigned-off-by: Teng Long \n---\n Documentation/git-ls-tree.txt | 8 +++++--\n builtin/ls-tree.c | 27 ++++++++++++++++-------\n t/t3104-ls-tree-oid.sh | 40 +++++++++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 10 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex db02d6d79a..bc711dc00a 100644\n--- a/Documentation/git-ls-tree.txt\n+++ b/Documentation/git-ls-tree.txt\n@@ -10,7 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n-\t [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n+\t [--name-only] [--name-status] [--oid-only]\n+\t [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n@@ -59,7 +60,10 @@ OPTIONS\n --name-only::\n --name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n-\n+\tCannot be used with `--oid-only` together.\n+--oid-only::\n+\tList only OIDs of the objects, one per line. Cannot be used with\n+\t`--name-only` or `--name-status` together.\n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n \tlines, show the shortest prefix that is at least ''\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c7..1e4a82e669 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -18,19 +18,26 @@ static int line_termination = '\\n';\n #define LS_RECURSIVE 1\n #define LS_TREE_ONLY 2\n #define LS_SHOW_TREES 4\n-#define LS_NAME_ONLY 8\n-#define LS_SHOW_SIZE 16\n+#define LS_SHOW_SIZE 8\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 \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 \n+enum {\n+\tMODE_UNSPECIFIED = 0,\n+\tMODE_NAME_ONLY,\n+\tMODE_OID_ONLY\n+};\n+\n+static int cmdmode = MODE_UNSPECIFIED;\n+\n static int show_recursive(const char *base, int baselen, const char *pathname)\n {\n \tint i;\n@@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif (!(ls_options & LS_NAME_ONLY)) {\n+\tif (cmdmode == 2) {\n+\t\tprintf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n+\t\treturn 0;\n+\t}\n+\n+\tif (cmdmode == 0) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n \t\t\tchar size_text[24];\n \t\t\tif (!strcmp(type, blob_type)) {\n@@ -135,10 +147,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n \t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n \t\t\tLS_SHOW_SIZE),\n-\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n-\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n+\t\tOPT_CMDMODE('n', \"name-only\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE('s', \"name-status\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode, N_(\"list only oids\"), MODE_OID_ONLY),\n \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\ndiff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\nnew file mode 100755\nindex 0000000000..4c02cdd3c3\n--- /dev/null\n+++ b/t/t3104-ls-tree-oid.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='git ls-tree oids handling.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo 111 >1.txt &&\n+\techo 222 >2.txt &&\n+\tmkdir -p path0/a/b/c &&\n+\techo 333 >path0/a/b/c/3.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=$(git write-tree) &&\n+\techo $tree\n+'\n+\n+test_expect_success 'usage: --oid-only' '\n+\tgit ls-tree --oid-only $tree >current &&\n+\tgit ls-tree $tree | awk \"{print \\$3}\" >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --oid-only with -r' '\n+\tgit ls-tree --oid-only -r $tree >current &&\n+\tgit ls-tree -r $tree | awk \"{print \\$3}\" >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --oid-only with --abbrev' '\n+\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n+\tgit ls-tree --abbrev=6 $tree | awk \"{print \\$3}\" > expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '\n+\ttest_incompatible_usage git ls-tree --oid-only --name-only\n+'\n+\n+test_done\n-- \n2.33.1.10.g438dd9044d.dirty\n\n"},{"id":"441926","messageId":"211122.8635no1lwp.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"20211122074538.87255-1-dyroneteng@gmail.com","subject":"Re: [PATCH v2 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-22T11:14:14Z","receivedAt":"2021-11-22T11:15:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 22 2021, Teng Long wrote:\n\n> On Fri, 19 Nov 2021 14:30:52 +0100, Ævar Arnfjörð Bjarmason wrote\n\n>> just cut -f1 instead of awk? Also don't put \"git\" on the LHS of a pipe,\n>> it might hide segfaults. Also applies to the below.\n>>\n>\n> Will apply, and could you please describe the problem with more details?\n> (appreciate if there is an executable example)\n\nRun this in a terminal:\n\n git stawtus | cat; echo $?;\n\nThe LHS of the pipe fails, but the exit code of that command is\nhidden. So we prefer:\n\n git stawtus >out && # fails\n [...]\n\n\n\n"},{"id":"441994","messageId":"CAJm9OHd_MDjFqXok663O8bQmvdPCjrQeEHumPLDpxqVC4i4=1Q@mail.gmail.com","threadId":"56905","inReplyTo":"6c15b4c176b7c03072fa59a4efd9f6fea7d62eae.1637567328.git.dyroneteng@gmail.com","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Peter Baumann","fromEmail":"peter.baumann@gmail.com","sentAt":"2021-11-22T18:11:01Z","receivedAt":"2021-11-22T18:11:20Z","isPatch":true,"sender":{"key":"peter.baumann@gmail.com","avatar":null},"body":"[ Sorry if you receive this mail twice, it seems like it didn't get\nthrough the first time. ]\n\nOn Mon, Nov 22, 2021 at 9:50 AM Teng Long wrote:\n>\n> Sometimes, we only want to get the objects from output of `ls-tree`\n> and commands like `sed` or `cut` is usually used to intercept the\n> origin output to achieve this purpose in practical.\n>\n> This commit supply an option names `--oid-only` to let `git ls-tree`\n> only print out the OID of the object. `--oid-only` and `--name-only`\n> are mutually exclusive in use.\n>\n> Signed-off-by: Teng Long \n> ---\n> Documentation/git-ls-tree.txt | 8 +++++--\n> builtin/ls-tree.c | 27 ++++++++++++++++-------\n> t/t3104-ls-tree-oid.sh | 40 +++++++++++++++++++++++++++++++++++\n> 3 files changed, 65 insertions(+), 10 deletions(-)\n> create mode 100755 t/t3104-ls-tree-oid.sh\n>\n> diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\n> index db02d6d79a..bc711dc00a 100644\n> --- a/Documentation/git-ls-tree.txt\n> +++ b/Documentation/git-ls-tree.txt\n> @@ -10,7 +10,8 @@ SYNOPSIS\n> --------\n> [verse]\n> 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n> - [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n> + [--name-only] [--name-status] [--oid-only]\n> + [--full-name] [--full-tree] [--abbrev[=]]\n> [...]\n>\n\nShouldn't the synopsis also indicate that the options are exclusive, e.g.\n [--name-only | --oid-only] ?\n\nBesides adding the new --oid-only mode, you also add one letter acronyms for\n [-n | --name-only]\n [-s | --name-status ]\nand one letter abbreviation\n [-o | --oid-only ]\nwhich are all undocumented in the help page. If we want the short one\nletter version,\nthey should be documented. For me, it is at least questionable why we\nintroduce them\nand more so in a commit adding --oid-only.\n\n\n> DESCRIPTION\n> @@ -59,7 +60,10 @@ OPTIONS\n> --name-only::\n> --name-status::\n> List only filenames (instead of the \"long\" output), one per line.\n> -\n> + Cannot be used with `--oid-only` together.\n> +--oid-only::\n> + List only OIDs of the objects, one per line. Cannot be used with\n> + `--name-only` or `--name-status` together.\n> --abbrev[=]::\n> Instead of showing the full 40-byte hexadecimal object\n> lines, show the shortest prefix that is at least ''\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 3a442631c7..1e4a82e669 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -18,19 +18,26 @@ static int line_termination = '\\n';\n> #define LS_RECURSIVE 1\n> #define LS_TREE_ONLY 2\n> #define LS_SHOW_TREES 4\n> -#define LS_NAME_ONLY 8\n> -#define LS_SHOW_SIZE 16\n> +#define LS_SHOW_SIZE 8\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>\n> -static const char * const ls_tree_usage[] = {\n> +static const char * const ls_tree_usage[] = {\n> N_(\"git ls-tree [] [...]\"),\n> NULL\n> };\n>\n> +enum {\n> + MODE_UNSPECIFIED = 0,\n> + MODE_NAME_ONLY,\n> + MODE_OID_ONLY\n> +};\n> +\n> +static int cmdmode = MODE_UNSPECIFIED;\n> +\n> static int show_recursive(const char *base, int baselen, const char *pathname)\n> {\n> int i;\n> @@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> else if (ls_options & LS_TREE_ONLY)\n> return 0;\n>\n> - if (!(ls_options & LS_NAME_ONLY)) {\n> + if (cmdmode == 2) {\n> + printf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n> + return 0;\n> + }\n> +\n> + if (cmdmode == 0) {\n> if (ls_options & LS_SHOW_SIZE) {\n> char size_text[24];\n> if (!strcmp(type, blob_type)) {\n> @@ -135,10 +147,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> N_(\"terminate entries with NUL byte\"), 0),\n> OPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n> LS_SHOW_SIZE),\n> - OPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n> - LS_NAME_ONLY),\n> - OPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n> - LS_NAME_ONLY),\n> + OPT_CMDMODE('n', \"name-only\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n> + OPT_CMDMODE('s', \"name-status\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n> + OPT_CMDMODE('o', \"oid-only\", &cmdmode, N_(\"list only oids\"), MODE_OID_ONLY),\n> OPT_SET_INT(0, \"full-name\", &chomp_prefix,\n> N_(\"use full path names\"), 0),\n> OPT_BOOL(0, \"full-tree\", &full_tree,\n> diff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\n> new file mode 100755\n> index 0000000000..4c02cdd3c3\n> --- /dev/null\n> +++ b/t/t3104-ls-tree-oid.sh\n> @@ -0,0 +1,40 @@\n> +#!/bin/sh\n> +\n> +test_description='git ls-tree oids handling.'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> + echo 111 >1.txt &&\n> + echo 222 >2.txt &&\n> + mkdir -p path0/a/b/c &&\n> + echo 333 >path0/a/b/c/3.txt &&\n> + find *.txt path* \\( -type f -o -type l \\) -print |\n\nI don't see the test using any symbolic links. Why are we searching for\nsymbolic links with \"-type l\" here?\n\n-Peter\n\n> + xargs git update-index --add &&\n> + tree=$(git write-tree) &&\n> + echo $tree\n> +'\n\n> +\n> +test_expect_success 'usage: --oid-only' '\n> + git ls-tree --oid-only $tree >current &&\n> + git ls-tree $tree | awk \"{print \\$3}\" >expected &&\n> + test_cmp current expected\n> +'\n> +\n> +test_expect_success 'usage: --oid-only with -r' '\n> + git ls-tree --oid-only -r $tree >current &&\n> + git ls-tree -r $tree | awk \"{print \\$3}\" >expected &&\n> + test_cmp current expected\n> +'\n> +\n> +test_expect_success 'usage: --oid-only with --abbrev' '\n> + git ls-tree --oid-only --abbrev=6 $tree >current &&\n> + git ls-tree --abbrev=6 $tree | awk \"{print \\$3}\" > expected &&\n> + test_cmp current expected\n> +'\n> +\n> +test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '\n> + test_incompatible_usage git ls-tree --oid-only --name-only\n> +'\n> +\n> +test_done\n> --\n> 2.33.1.10.g438dd9044d.dirty\n>\n"},{"id":"442008","messageId":"xmqqczmsf2d2.fsf@gitster.g","threadId":"56905","inReplyTo":"6c15b4c176b7c03072fa59a4efd9f6fea7d62eae.1637567328.git.dyroneteng@gmail.com","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-22T18:54:01Z","receivedAt":"2021-11-22T18:54:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> Sometimes, we only want to get the objects from output of `ls-tree`\n> and commands like `sed` or `cut` is usually used to intercept the\n> origin output to achieve this purpose in practical.\n\n\"in practical\" -> \"in practice\".\n\nThat's true and that is exactly this plumbing command was designed\nto be used.\n\n> This commit supply an option names `--oid-only` to let `git ls-tree`\n> only print out the OID of the object. `--oid-only` and `--name-only`\n> are mutually exclusive in use.\n\n Teach the \"--oid-only\" option to tell the command to only show\n the object name, just like \"--name-only\" option tells the\n command to only show the path component, for each entry. These\n two options are mutually exclusive.\n\nperhaps?\n\nThe above leaves \"mode-only\" and \"type-only\". I wonder if it is a\nbetter design to add just one new option, --hide-fields, and make\nthe existing --name-only into a synonym to\n\n git ls-tree --hide-fields=mode,type,object $T\n\nwhich would mean we do not need to end up with four mutually\nexclusive commands, and anybody who wants to only see object names\ncan do\n\n git ls-tree --hide-fields=mode,type,file $T\n\nNote: the above uses the terminology in the OUTPUT FORMAT section;\nif we want to use \"name\" instead of \"file\", I am perfectly OK with\nit, but then we should update the documentation to match.\n\nCome to think of it, I think \"--show-fields\" may work even better\nthan \"--hide-fields\". We can use it to get rid of the \"--long\"\noption:\n\n git ls-tree --show-fields=mode,type,object,size,file $T\n\nwould be equivelent to\n\n git ls-tree --long $T\n\nThe field order may need to be thought through, especially when \"-z\"\noutput is not being used. We may need a rule to require \"file\" to\nbe at the end, if exists, or even simpler rule \"you can choose which\nfields are shown but the order they come out is not affected\" (i.e.\n\"--show-fields=mode,type\" and \"--show-fields=type,mode\" give the\nsame output).\n\nI am OK if we started with \"only a single field allowed\" and extend\nit to support multiple fields later (until that happens, we cannot\nemulate the \"--long\" output, though). Then we do not have to answer\ntwo tricky questions, what to do with the output order, and what\nfield separators are used in the output.\n"},{"id":"442068","messageId":"YZwyXAn+gxUZ+aD9@danh.dev","threadId":"56905","inReplyTo":"6c15b4c176b7c03072fa59a4efd9f6fea7d62eae.1637567328.git.dyroneteng@gmail.com","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-11-23T00:14:20Z","receivedAt":"2021-11-23T00:14:26Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-11-22 16:07:28+0800, Teng Long wrote:\n> Sometimes, we only want to get the objects from output of `ls-tree`\n> and commands like `sed` or `cut` is usually used to intercept the\n> origin output to achieve this purpose in practical.\n> \n> This commit supply an option names `--oid-only` to let `git ls-tree`\n> only print out the OID of the object. `--oid-only` and `--name-only`\n> are mutually exclusive in use.\n> \n> Signed-off-by: Teng Long \n> ---\n> Documentation/git-ls-tree.txt | 8 +++++--\n> builtin/ls-tree.c | 27 ++++++++++++++++-------\n> t/t3104-ls-tree-oid.sh | 40 +++++++++++++++++++++++++++++++++++\n> 3 files changed, 65 insertions(+), 10 deletions(-)\n> create mode 100755 t/t3104-ls-tree-oid.sh\n> \n> diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\n> index db02d6d79a..bc711dc00a 100644\n> --- a/Documentation/git-ls-tree.txt\n> +++ b/Documentation/git-ls-tree.txt\n> @@ -10,7 +10,8 @@ SYNOPSIS\n> --------\n> [verse]\n> 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n> -\t [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n> +\t [--name-only] [--name-status] [--oid-only]\n\nPlease indicate those options are incompatible (as someone else said):\n\n\t[--name-only | --name-status | --oid-only]\n\n> +\t [--full-name] [--full-tree] [--abbrev[=]]\n> \t [...]\n> \n> DESCRIPTION\n> @@ -59,7 +60,10 @@ OPTIONS\n> --name-only::\n> --name-status::\n> \tList only filenames (instead of the \"long\" output), one per line.\n> -\n> +\tCannot be used with `--oid-only` together.\n> +--oid-only::\n> +\tList only OIDs of the objects, one per line. Cannot be used with\n> +\t`--name-only` or `--name-status` together.\n> --abbrev[=]::\n> \tInstead of showing the full 40-byte hexadecimal object\n> \tlines, show the shortest prefix that is at least ''\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 3a442631c7..1e4a82e669 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -18,19 +18,26 @@ static int line_termination = '\\n';\n> #define LS_RECURSIVE 1\n> #define LS_TREE_ONLY 2\n> #define LS_SHOW_TREES 4\n> -#define LS_NAME_ONLY 8\n> -#define LS_SHOW_SIZE 16\n> +#define LS_SHOW_SIZE 8\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> \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> \n> +enum {\n> +\tMODE_UNSPECIFIED = 0,\n> +\tMODE_NAME_ONLY,\n> +\tMODE_OID_ONLY\n> +};\n> +\n> +static int cmdmode = MODE_UNSPECIFIED;\n> +\n> static int show_recursive(const char *base, int baselen, const char *pathname)\n> {\n> \tint i;\n> @@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> \telse if (ls_options & LS_TREE_ONLY)\n> \t\treturn 0;\n> \n> -\tif (!(ls_options & LS_NAME_ONLY)) {\n> +\tif (cmdmode == 2) {\n\nI think it's better to use the enum name:\n\n\tif (cmdmode == MODE_OID_ONLY) {\n\n> +\t\tprintf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tif (cmdmode == 0) {\n\nDitto:\n\n\tif (cmdmode == MODE_UNSPECIFIED) {\n\nSpeaking about this, where will MODE_NAME_ONLY be used?\n\n> \t\tif (ls_options & LS_SHOW_SIZE) {\n> \t\t\tchar size_text[24];\n> \t\t\tif (!strcmp(type, blob_type)) {\n> @@ -135,10 +147,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n> \t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n> \t\t\tLS_SHOW_SIZE),\n> -\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n> -\t\t\tLS_NAME_ONLY),\n> -\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n> -\t\t\tLS_NAME_ONLY),\n> +\t\tOPT_CMDMODE('n', \"name-only\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n> +\t\tOPT_CMDMODE('s', \"name-status\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n> +\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode, N_(\"list only oids\"), MODE_OID_ONLY),\n> \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n> \t\t\t N_(\"use full path names\"), 0),\n> \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n> diff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\n> new file mode 100755\n> index 0000000000..4c02cdd3c3\n> --- /dev/null\n> +++ b/t/t3104-ls-tree-oid.sh\n> @@ -0,0 +1,40 @@\n> +#!/bin/sh\n> +\n> +test_description='git ls-tree oids handling.'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +\techo 111 >1.txt &&\n> +\techo 222 >2.txt &&\n> +\tmkdir -p path0/a/b/c &&\n> +\techo 333 >path0/a/b/c/3.txt &&\n> +\tfind *.txt path* \\( -type f -o -type l \\) -print |\n> +\txargs git update-index --add &&\n> +\ttree=$(git write-tree) &&\n> +\techo $tree\n> +'\n> +\n> +test_expect_success 'usage: --oid-only' '\n> +\tgit ls-tree --oid-only $tree >current &&\n> +\tgit ls-tree $tree | awk \"{print \\$3}\" >expected &&\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_success 'usage: --oid-only with -r' '\n> +\tgit ls-tree --oid-only -r $tree >current &&\n> +\tgit ls-tree -r $tree | awk \"{print \\$3}\" >expected &&\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_success 'usage: --oid-only with --abbrev' '\n> +\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n> +\tgit ls-tree --abbrev=6 $tree | awk \"{print \\$3}\" > expected &&\n> +\ttest_cmp current expected\n> +'\n> +\n> +test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '\n> +\ttest_incompatible_usage git ls-tree --oid-only --name-only\n> +'\n> +\n> +test_done\n\nIt seems like you haven't updated the test code from v2\n\n-- \nDanh\n"},{"id":"442076","messageId":"211123.86tug3wu8v.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"xmqqczmsf2d2.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-23T01:09:05Z","receivedAt":"2021-11-23T01:12:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 22 2021, Junio C Hamano wrote:\n\n> Teng Long writes:\n>\n>> Sometimes, we only want to get the objects from output of `ls-tree`\n>> and commands like `sed` or `cut` is usually used to intercept the\n>> origin output to achieve this purpose in practical.\n>\n> \"in practical\" -> \"in practice\".\n>\n> That's true and that is exactly this plumbing command was designed\n> to be used.\n>\n>> This commit supply an option names `--oid-only` to let `git ls-tree`\n>> only print out the OID of the object. `--oid-only` and `--name-only`\n>> are mutually exclusive in use.\n>\n> Teach the \"--oid-only\" option to tell the command to only show\n> the object name, just like \"--name-only\" option tells the\n> command to only show the path component, for each entry. These\n> two options are mutually exclusive.\n>\n> perhaps?\n>\n> The above leaves \"mode-only\" and \"type-only\". I wonder if it is a\n> better design to add just one new option, --hide-fields, and make\n> the existing --name-only into a synonym to\n>\n> git ls-tree --hide-fields=mode,type,object $T\n>\n> which would mean we do not need to end up with four mutually\n> exclusive commands, and anybody who wants to only see object names\n> can do\n>\n> git ls-tree --hide-fields=mode,type,file $T\n>\n> Note: the above uses the terminology in the OUTPUT FORMAT section;\n> if we want to use \"name\" instead of \"file\", I am perfectly OK with\n> it, but then we should update the documentation to match.\n>\n> Come to think of it, I think \"--show-fields\" may work even better\n> than \"--hide-fields\". We can use it to get rid of the \"--long\"\n> option:\n>\n> git ls-tree --show-fields=mode,type,object,size,file $T\n>\n> would be equivelent to\n>\n> git ls-tree --long $T\n>\n> The field order may need to be thought through, especially when \"-z\"\n> output is not being used. We may need a rule to require \"file\" to\n> be at the end, if exists, or even simpler rule \"you can choose which\n> fields are shown but the order they come out is not affected\" (i.e.\n> \"--show-fields=mode,type\" and \"--show-fields=type,mode\" give the\n> same output).\n>\n> I am OK if we started with \"only a single field allowed\" and extend\n> it to support multiple fields later (until that happens, we cannot\n> emulate the \"--long\" output, though). Then we do not have to answer\n> two tricky questions, what to do with the output order, and what\n> field separators are used in the output.\n\nAll of which (and more) would also be addressed in an obvious way by\njust supporting --format as I suggested in\nhttps://lore.kernel.org/git/211115.86o86lqe3c.gmgdl@evledraar.gmail.com/;\ndon't you think that's a better approach?\n\nAs noted in\nhttps://lore.kernel.org/git/211115.86mtm5saz7.gmgdl@evledraar.gmail.com/\nwe could start by simply dying if the format is not on a small list of\nformats we handle, i.e. not implement the strbuf_expand() change to\nstart with.\n\nA --show-fields and --hide-fields seems like a rather elaborate\ninterface in lieu of just having a --format.\n\nYou'd presumably then want a --field-seperator and\n--name-field-separator (we use SP and TAB for the two, currently), so\nwe've got N option now just to emulate what a --format would do for us.\n"},{"id":"442080","messageId":"xmqqpmqrachl.fsf@gitster.g","threadId":"56905","inReplyTo":"211123.86tug3wu8v.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-23T01:26:30Z","receivedAt":"2021-11-23T01:26:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> All of which (and more) would also be addressed in an obvious way by\n> just supporting --format as I suggested in\n> https://lore.kernel.org/git/211115.86o86lqe3c.gmgdl@evledraar.gmail.com/;\n> don't you think that's a better approach?\n\nThat is what I would call over-engineering that I would rather not\nto have in low level plumbing.\n\nI am all for making _parsing_ the output from the tool easier by\nscripts; I am not interested in eliminating the _output_ by scripts.\nThey should capture and format the pieces we output in any way they\nwant.\n\nSo, no, I do not think it is a better approach at all.\n"},{"id":"442088","messageId":"211123.868rxfwqdw.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"xmqqpmqrachl.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-23T02:28:15Z","receivedAt":"2021-11-23T02:35:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 22 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> All of which (and more) would also be addressed in an obvious way by\n>> just supporting --format as I suggested in\n>> https://lore.kernel.org/git/211115.86o86lqe3c.gmgdl@evledraar.gmail.com/;\n>> don't you think that's a better approach?\n>\n> That is what I would call over-engineering that I would rather not\n> to have in low level plumbing.\n>\n> I am all for making _parsing_ the output from the tool easier by\n> scripts; I am not interested in eliminating the _output_ by scripts.\n> They should capture and format the pieces we output in any way they\n> want.\n>\n> So, no, I do not think it is a better approach at all.\n\nWe've got --format for for-each-ref and family (also branch etc.), and\nfor the \"log\" family.\n\nI'm not sure I understand what you're saying, do you think if we could\ngo back and change it that the \"FIELD NAMES\" in git-for-each-ref (which\nis plumbing) would have been better done as\n--field-name=refname,objecttype,... etc?\n\nHaving used it extensively it's been very hard to have the flexibility\nof formatting, e.g. to specify arbitrary delimiters.\n\nIt also leaves the door open to teaching ls-tree etc. the %(if) syntax\nin the ref-filter, e.g. if you'd like to only print out certain data for\ncertain object types or whatever.\n"},{"id":"442089","messageId":"xmqqlf1fa8cj.fsf@gitster.g","threadId":"56905","inReplyTo":"211123.868rxfwqdw.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-23T02:55:56Z","receivedAt":"2021-11-23T02:56:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> That is what I would call over-engineering that I would rather not\n>> to have in low level plumbing.\n>> ...\n> We've got --format for for-each-ref and family (also branch etc.), and\n> for the \"log\" family.\n\nBut I didn't comment on them. ls-tree is a lot lower-level plumbing\nwhere --format does not belong in my mind.\n\n\n\n"},{"id":"442091","messageId":"xmqqh7c3a6hs.fsf@gitster.g","threadId":"56905","inReplyTo":"xmqqlf1fa8cj.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-23T03:35:59Z","receivedAt":"2021-11-23T03:36:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> That is what I would call over-engineering that I would rather not\n>>> to have in low level plumbing.\n>>> ...\n>> We've got --format for for-each-ref and family (also branch etc.), and\n>> for the \"log\" family.\n>\n> But I didn't comment on them. ls-tree is a lot lower-level plumbing\n> where --format does not belong in my mind.\n\nThere is a lot more practical reason why I'd prefer a less flexible\nand good enough interface.\n\nI can see, without coding it myself but from mere memory of how the\ncode looked like, how such a \"we allow you to choose which field to\ninclude, but you do not get to choose the order of fields or any\nother string in the output\" can be done with minimum disruption to\nthe existing code and without introducing a bug. On the other hand,\nI am fairly certain that anything more flexible than that will risk\nnew bugs involved in any large shuffling of the code, which I am\ngetting tired of.\n\nSo there.\n"},{"id":"442092","messageId":"cover.1637642029.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1637567328.git.dyroneteng@gmail.com","subject":"[PATCH v4 0/1] ls-tree.c: support `--oid-only` option","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-23T04:58:05Z","receivedAt":"2021-11-23T04:58:22Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Thanks for the discussions on v3 (even I send a patch with\nwrong contents and the right cover). So I looked at them, and\nI think I have to send a new patch first, so this includes:\n\n 1. Commit message modifications (Junio C Hamano's advice)\n 2. Documentation modifications (Peter Baumann's advice)\n 3. To use the MODE enum name instead (Đoàn Trần Công Danh's advice)\n\nThe other discussions I will reply today.\n\nTeng Long (1):\n ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n\n Documentation/git-ls-tree.txt | 18 ++++++++++++---\n builtin/ls-tree.c | 30 +++++++++++++++++-------\n t/t3104-ls-tree-oid.sh | 43 +++++++++++++++++++++++++++++++++++\n 3 files changed, 80 insertions(+), 11 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\nRange-diff against v3:\n1: 8b68568d6c ! 1: 63876dbeb7 ls-tree.c: support `--oid-only` option for \"git-ls-tree\"\n @@ Commit message\n \n Sometimes, we only want to get the objects from output of `ls-tree`\n and commands like `sed` or `cut` is usually used to intercept the\n - origin output to achieve this purpose in practical.\n + origin output to achieve this purpose in practice.\n \n - This commit supply an option names `--oid-only` to let `git ls-tree`\n - only print out the OID of the object. `--oid-only` and `--name-only`\n - are mutually exclusive in use.\n + This commit teach the \"--oid-only\" option to tell the command to\n + only show the object name, just like \"--name-only\" option tells the\n + command to only show the path component, for each entry. These two\n + options are mutually exclusive.\n \n - Reviewed-by: Jeff King \n - Reviewed-by: Ævar Arnfjörð Bjarmason \n - Reviewed-by: Đoàn Trần Công Danh \n Signed-off-by: Teng Long \n \n ## Documentation/git-ls-tree.txt ##\n -@@ Documentation/git-ls-tree.txt: SYNOPSIS\n +@@ Documentation/git-ls-tree.txt: git-ls-tree - List the contents of a tree object\n + SYNOPSIS\n --------\n [verse]\n - 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n +-'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n -\t [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n -+\t [--name-only] [--name-status] [--oid-only]\n ++'git ls-tree' [-d] [-r] [-t] [-l] [-z] [-n] [-s] [-o]\n ++\t [--name-only | --oid-only]\n ++\t [--name-status | --oid-only]\n +\t [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n @@ Documentation/git-ls-tree.txt: OPTIONS\n + \t\\0 line termination on output and do not quote filenames.\n + \tSee OUTPUT FORMAT below for more information.\n + \n ++-n::\n --name-only::\n - --name-status::\n +---name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n --\n -+\tCannot be used with `--oid-only` together.\n ++\tCannot be combined with `--oid-only`.\n ++\n ++-s::\n ++--name-status::\n ++\tConsistent behavior with `--name-only`.\n ++\n ++-o::\n +--oid-only::\n -+\tList only OIDs of the objects, one per line. Cannot be used with\n -+\t`--name-only` or `--name-status` together.\n ++\tList only names of the objects, one per line. Cannot be combined\n ++\twith `--name-only` or `--name-status`.\n + \n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\n - \tlines, show the shortest prefix that is at least ''\n \n ## builtin/ls-tree.c ##\n @@ builtin/ls-tree.c: static int line_termination = '\\n';\n @@ builtin/ls-tree.c: static int show_tree(const struct object_id *oid, struct strb\n \t\treturn 0;\n \n -\tif (!(ls_options & LS_NAME_ONLY)) {\n -+\tif (cmdmode == 2) {\n ++\tif (cmdmode == MODE_OID_ONLY) {\n +\t\tprintf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n +\t\treturn 0;\n +\t}\n +\n -+\tif (cmdmode == 0) {\n ++\tif (cmdmode == MODE_UNSPECIFIED) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n \t\t\tchar size_text[24];\n \t\t\tif (!strcmp(type, blob_type)) {\n @@ builtin/ls-tree.c: int cmd_ls_tree(int argc, const char **argv, const char *pref\n -\t\t\tLS_NAME_ONLY),\n -\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n -\t\t\tLS_NAME_ONLY),\n -+\t\tOPT_CMDMODE('n', \"name-only\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n -+\t\tOPT_CMDMODE('s', \"name-status\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n -+\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode, N_(\"list only oids\"), MODE_OID_ONLY),\n ++\t\tOPT_CMDMODE('n', \"name-only\", &cmdmode,\n ++\t\t\t N_(\"list only filenames\"), MODE_NAME_ONLY),\n ++\t\tOPT_CMDMODE('s', \"name-status\", &cmdmode,\n ++\t\t\t N_(\"list only filenames\"), MODE_NAME_ONLY),\n ++\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode,\n ++\t\t\t N_(\"list only oids\"), MODE_OID_ONLY),\n \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n @@ t/t3104-ls-tree-oid.sh (new)\n +. ./test-lib.sh\n +\n +test_expect_success 'setup' '\n -+\techo 111 >1.txt &&\n -+\techo 222 >2.txt &&\n -+\tmkdir -p path0/a/b/c &&\n -+\techo 333 >path0/a/b/c/3.txt &&\n ++\ttest_commit A &&\n ++\ttest_commit B &&\n ++\tmkdir -p C &&\n ++\ttest_commit C/D.txt &&\n +\tfind *.txt path* \\( -type f -o -type l \\) -print |\n +\txargs git update-index --add &&\n +\ttree=$(git write-tree) &&\n @@ t/t3104-ls-tree-oid.sh (new)\n +\n +test_expect_success 'usage: --oid-only' '\n +\tgit ls-tree --oid-only $tree >current &&\n -+\tgit ls-tree $tree | awk \"{print \\$3}\" >expected &&\n ++\tgit ls-tree $tree >result &&\n ++\tcut -f1 result | cut -d \" \" -f3 >expected &&\n +\ttest_cmp current expected\n +'\n +\n +test_expect_success 'usage: --oid-only with -r' '\n +\tgit ls-tree --oid-only -r $tree >current &&\n -+\tgit ls-tree -r $tree | awk \"{print \\$3}\" >expected &&\n ++\tgit ls-tree -r $tree >result &&\n ++\tcut -f1 result | cut -d \" \" -f3 >expected &&\n +\ttest_cmp current expected\n +'\n +\n +test_expect_success 'usage: --oid-only with --abbrev' '\n +\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n -+\tgit ls-tree --abbrev=6 $tree | awk \"{print \\$3}\" > expected &&\n ++\tgit ls-tree --abbrev=6 $tree >result &&\n ++\tcut -f1 result | cut -d \" \" -f3 >expected &&\n +\ttest_cmp current expected\n +'\n +\n -+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '\n -+\ttest_incompatible_usage git ls-tree --oid-only --name-only\n ++test_expect_success 'usage: incompatible options: --name-only with --oid-only' '\n ++\ttest_expect_code 129 git ls-tree --oid-only --name-only\n +'\n +\n +test_done\n-- \n2.33.1.10.g75523f744f.dirty\n\n"},{"id":"442093","messageId":"63876dbeb7f880c0001649b49d5265b744c42765.1637642029.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1637642029.git.dyroneteng@gmail.com","subject":"[PATCH v4 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-11-23T04:58:06Z","receivedAt":"2021-11-23T04:58:27Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Sometimes, we only want to get the objects from output of `ls-tree`\nand commands like `sed` or `cut` is usually used to intercept the\norigin output to achieve this purpose in practice.\n\nThis commit teach the \"--oid-only\" option to tell the command to\nonly show the object name, just like \"--name-only\" option tells the\ncommand to only show the path component, for each entry. These two\noptions are mutually exclusive.\n\nSigned-off-by: Teng Long \n---\n Documentation/git-ls-tree.txt | 18 ++++++++++++---\n builtin/ls-tree.c | 30 +++++++++++++++++-------\n t/t3104-ls-tree-oid.sh | 43 +++++++++++++++++++++++++++++++++++\n 3 files changed, 80 insertions(+), 11 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex db02d6d79a..fd2a871ca5 100644\n--- a/Documentation/git-ls-tree.txt\n+++ b/Documentation/git-ls-tree.txt\n@@ -9,8 +9,10 @@ git-ls-tree - List the contents of a tree object\n SYNOPSIS\n --------\n [verse]\n-'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n-\t [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n+'git ls-tree' [-d] [-r] [-t] [-l] [-z] [-n] [-s] [-o]\n+\t [--name-only | --oid-only]\n+\t [--name-status | --oid-only]\n+\t [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n@@ -56,9 +58,19 @@ OPTIONS\n \t\\0 line termination on output and do not quote filenames.\n \tSee OUTPUT FORMAT below for more information.\n \n+-n::\n --name-only::\n---name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n+\tCannot be combined with `--oid-only`.\n+\n+-s::\n+--name-status::\n+\tConsistent behavior with `--name-only`.\n+\n+-o::\n+--oid-only::\n+\tList only names of the objects, one per line. Cannot be combined\n+\twith `--name-only` or `--name-status`.\n \n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c7..0c2153a5ad 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -18,19 +18,26 @@ static int line_termination = '\\n';\n #define LS_RECURSIVE 1\n #define LS_TREE_ONLY 2\n #define LS_SHOW_TREES 4\n-#define LS_NAME_ONLY 8\n-#define LS_SHOW_SIZE 16\n+#define LS_SHOW_SIZE 8\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 \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 \n+enum {\n+\tMODE_UNSPECIFIED = 0,\n+\tMODE_NAME_ONLY,\n+\tMODE_OID_ONLY\n+};\n+\n+static int cmdmode = MODE_UNSPECIFIED;\n+\n static int show_recursive(const char *base, int baselen, const char *pathname)\n {\n \tint i;\n@@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif (!(ls_options & LS_NAME_ONLY)) {\n+\tif (cmdmode == MODE_OID_ONLY) {\n+\t\tprintf(\"%s\\n\", find_unique_abbrev(oid, abbrev));\n+\t\treturn 0;\n+\t}\n+\n+\tif (cmdmode == MODE_UNSPECIFIED) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n \t\t\tchar size_text[24];\n \t\t\tif (!strcmp(type, blob_type)) {\n@@ -135,10 +147,12 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n \t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n \t\t\tLS_SHOW_SIZE),\n-\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n-\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n+\t\tOPT_CMDMODE('n', \"name-only\", &cmdmode,\n+\t\t\t N_(\"list only filenames\"), MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE('s', \"name-status\", &cmdmode,\n+\t\t\t N_(\"list only filenames\"), MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode,\n+\t\t\t N_(\"list only oids\"), MODE_OID_ONLY),\n \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\ndiff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\nnew file mode 100755\nindex 0000000000..2d349f6e46\n--- /dev/null\n+++ b/t/t3104-ls-tree-oid.sh\n@@ -0,0 +1,43 @@\n+#!/bin/sh\n+\n+test_description='git ls-tree oids handling.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit A &&\n+\ttest_commit B &&\n+\tmkdir -p C &&\n+\ttest_commit C/D.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=$(git write-tree) &&\n+\techo $tree\n+'\n+\n+test_expect_success 'usage: --oid-only' '\n+\tgit ls-tree --oid-only $tree >current &&\n+\tgit ls-tree $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --oid-only with -r' '\n+\tgit ls-tree --oid-only -r $tree >current &&\n+\tgit ls-tree -r $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --oid-only with --abbrev' '\n+\tgit ls-tree --oid-only --abbrev=6 $tree >current &&\n+\tgit ls-tree --abbrev=6 $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-only with --oid-only' '\n+\ttest_expect_code 129 git ls-tree --oid-only --name-only\n+'\n+\n+test_done\n-- \n2.33.1.10.g75523f744f.dirty\n\n"},{"id":"442109","messageId":"211123.86mtlvumps.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"xmqqh7c3a6hs.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-23T11:04:06Z","receivedAt":"2021-11-23T11:37:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 22 2021, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>>> That is what I would call over-engineering that I would rather not\n>>>> to have in low level plumbing.\n>>>> ...\n>>> We've got --format for for-each-ref and family (also branch etc.), and\n>>> for the \"log\" family.\n>>\n>> But I didn't comment on them. ls-tree is a lot lower-level plumbing\n>> where --format does not belong in my mind.\n\nYes, you're just talking about ls-tree here. I'm just trying to\nunderstand what you meant with:\n\n I am not interested in eliminating the _output_ by scripts.\n They should capture and format the pieces we output in any way they\n want.\n\nWhich reads to me like \"we provide the data, you pretty-fy it\". In this\ncase the proposed feature doesn't need a patch to git, but can also be\ndone as:\n\n git ls-tree HEAD | cut -d$'\\t' -f1 | cut -d ' ' -f3\n\nI think it's useful. I'm just trying to understand what you think about\nsuch plumbing output.\n\n> There is a lot more practical reason why I'd prefer a less flexible\n> and good enough interface.\n>\n> I can see, without coding it myself but from mere memory of how the\n> code looked like, how such a \"we allow you to choose which field to\n> include, but you do not get to choose the order of fields or any\n> other string in the output\" can be done with minimum disruption to\n> the existing code and without introducing a bug. On the other hand,\n> I am fairly certain that anything more flexible than that will risk\n> new bugs involved in any large shuffling of the code, which I am\n> getting tired of.\n\nTo be clear, I wasn't talking about running with the WIP patch I had in\n<211115.86o86lqe3c.gmgdl@evledraar.gmail.com> here, but that the\ninterface wolud leave the door open to it. So something like the below.\n\nThis works to do what --oid-only does without adding that switch,\ninstead we add it tou our list of 4 supported hardcoded formats, all of\nwhich map to one of the MODE_* flags.\n\nWe could just document that we support that limited list for now, and\nthat we might add more in the future.\n\nSo it's just a way of adding a new MODE_* without supporting an ever\ngrowing list of flags, --oid-only, --objectmode-only, --objectsize-only\netc.\n\nThen if we'd ever want to generalize this in the future we can pick up\nsometing like my WIP patch and we'd have support for any arbitrary\nformat.\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 1e4a82e669a..e1e746ae02a 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -30,10 +30,11 @@ static const char * const ls_tree_usage[] = {\n \tNULL\n };\n \n-enum {\n+enum ls_tree_cmdmode {\n \tMODE_UNSPECIFIED = 0,\n \tMODE_NAME_ONLY,\n-\tMODE_OID_ONLY\n+\tMODE_OID_ONLY,\n+\tMODE_LONG,\n };\n \n static int cmdmode = MODE_UNSPECIFIED;\n@@ -131,11 +132,22 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \treturn retval;\n }\n \n+static struct {\n+\tenum ls_tree_cmdmode cmdmode;\n+\tconst char *fmt;\n+} allowed_formats[] = {\n+\t{ MODE_UNSPECIFIED,\t\"%(objectmode) %(objecttype) %(objectname)%09%(path)\" },\n+\t{ MODE_NAME_ONLY,\t\"%(path)\" },\n+\t{ MODE_OID_ONLY,\t\"%(objectname)\" },\n+\t{ MODE_LONG,\t\t\"%(objectmode) %(objecttype) %(objectsize) %(objectname)%09%(path)\" },\n+};\n+\n int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_id oid;\n \tstruct tree *tree;\n \tint i, full_tree = 0;\n+\tconst char *format = NULL;\n \tconst struct option ls_tree_options[] = {\n \t\tOPT_BIT('d', NULL, &ls_options, N_(\"only show trees\"),\n \t\t\tLS_TREE_ONLY),\n@@ -149,7 +161,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\tLS_SHOW_SIZE),\n \t\tOPT_CMDMODE('n', \"name-only\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n \t\tOPT_CMDMODE('s', \"name-status\", &cmdmode, N_(\"list only filenames\"), MODE_NAME_ONLY),\n-\t\tOPT_CMDMODE('o', \"oid-only\", &cmdmode, N_(\"list only oids\"), MODE_OID_ONLY),\n+\t\tOPT_STRING(0 , \"format\", &format, N_(\"format\"),\n+\t\t\t N_(\"(limited) format to use for the output\")),\n \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n@@ -170,6 +183,22 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\tls_tree_prefix = prefix = NULL;\n \t\tchomp_prefix = 0;\n \t}\n+\n+\tif (format && cmdmode)\n+\t\tdie(_(\"--format and --name-only, --long etc. are incompatible\"));\n+\tif (format) {\n+\t\tsize_t i;\n+\n+\t\tfor (i = 0; i <= ARRAY_SIZE(allowed_formats); i++) {\n+\t\t\tif (i == ARRAY_SIZE(allowed_formats))\n+\t\t\t\tdie(_(\"your --format=%s is not on the whitelist of supported formats\"), format);\n+\t\t\tif (!strcmp(format, allowed_formats[i].fmt)) {\n+\t\t\t\tcmdmode = allowed_formats[i].cmdmode;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\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"},{"id":"442204","messageId":"xmqqmtlu7bb0.fsf@gitster.g","threadId":"56905","inReplyTo":"63876dbeb7f880c0001649b49d5265b744c42765.1637642029.git.dyroneteng@gmail.com","subject":"Re: [PATCH v4 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-23T22:32:35Z","receivedAt":"2021-11-23T22:32:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> Sometimes, we only want to get the objects from output of `ls-tree`\n> and commands like `sed` or `cut` is usually used to intercept the\n> origin output to achieve this purpose in practice.\n\nI can guess what \"intercept the origin\" wants to say, but it does\nnot roll off the tongue very well.\n\n> This commit teach the \"--oid-only\" option to tell the command to\n> only show the object name, just like \"--name-only\" option tells the\n> command to only show the path component, for each entry. These two\n> options are mutually exclusive.\n\ncf. Documentation/SubmittingPatches[[imperative-mood]]\n\nPerhaps like\n\n We usually pipe the output from `git ls-files` to tools like\n `sed` or `cut` when we only want to extract some fields.\n\n When we want only the pathname component, we can pass\n `--name-only` option to omit such a pipeline, but there are no\n options for extracting other fields.\n\n Teach the \"--oid-only\" option to the command to only show the\n object name. This option cannot be used together with\n \"--name-only\" or \"--long\".\n\nThis does not work well with \"--long\", right?\n\n> -'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n> -\t [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=]]\n> +'git ls-tree' [-d] [-r] [-t] [-l] [-z] [-n] [-s] [-o]\n\nWhere does the addition of -n/-s/-o fit in the theme of the patch?\nThis being a plumbing command, an existing option with long name\ndoes not deserve a shorthand (and --oid-only should start with long\nname only, too).\n\nIf we were to introduce -n as yet another synonym for --name-only\nand --name-status (which we would not), the right way to spell it\nin the synopsis section would be:\n\n\t[-n | --name-only | --name-status]\n \nI would think, but this advise is only for your next topic. We are\nnot going to add such a synonym.\n\n> +\t [--name-only | --oid-only]\n> +\t [--name-status | --oid-only]\n\nThis looks very iffy. Has this been proof-read before sending out?\n\n> +\t [--full-name] [--full-tree] [--abbrev[=]]\n> \t [...]\n> \n> DESCRIPTION\n> @@ -56,9 +58,19 @@ OPTIONS\n> \t\\0 line termination on output and do not quote filenames.\n> \tSee OUTPUT FORMAT below for more information.\n> \n> +-n::\n> --name-only::\n> ---name-status::\n> \tList only filenames (instead of the \"long\" output), one per line.\n> +\tCannot be combined with `--oid-only`.\n> +\n> +-s::\n> +--name-status::\n> +\tConsistent behavior with `--name-only`.\n\nUsually we say A is \"Consistent\" with B when A and B are different\nbut are moral equivalent in their respective contexts. These are\nidentical, there is no difference.\n\nLose all of the above change, except for \"Cannot be combined with\".\nThe original is just fine.\n\n> +-o::\n> +--oid-only::\n> +\tList only names of the objects, one per line. Cannot be combined\n> +\twith `--name-only` or `--name-status`.\n\nOr \"--long\"? Does this work with it?\n\nALso, lose \"-o\".\n\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 3a442631c7..0c2153a5ad 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -18,19 +18,26 @@ static int line_termination = '\\n';\n> #define LS_RECURSIVE 1\n> #define LS_TREE_ONLY 2\n> #define LS_SHOW_TREES 4\n> -#define LS_NAME_ONLY 8\n> -#define LS_SHOW_SIZE 16\n> +#define LS_SHOW_SIZE 8\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> \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> \n> +enum {\n> +\tMODE_UNSPECIFIED = 0,\n> +\tMODE_NAME_ONLY,\n> +\tMODE_OID_ONLY\n> +};\n\nI suspect that \"--long\" would be part of this, if we were to go this\nroute.\n\nOPT_CMDMODE() is a handy way to ensure \"--name-only\", \"--oid-only\",\nand \"--long\" are not given together, but it may be overkill to make\nonly two or three options mutually exclusive.\n\nIn any case, once we pass the parsing part, the code should\ntranslate the option into a bitmask that specifies which among\n, , , , and fields are\nshown. It will result in cleaner code in show_tree() if it uses\nthat set of fields to decide what is shown and how without looking\nat the cmdmode enum.\n\n"},{"id":"443140","messageId":"20211206075223.56031-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"xmqqmtlu7bb0.fsf@gitster.g","subject":"Re: [PATCH v4 1/1] ls-tree.c: support `--oid-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-12-06T07:52:23Z","receivedAt":"2021-12-06T07:53:44Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"On Tue, 23 Nov 2021 14:32:35 -0800, Junio C Hamano wrote:\n\n> I can guess what \"intercept the origin\" wants to say, but it does\n>not roll off the tongue very well.\n>\n> cf. Documentation/SubmittingPatches[[imperative-mood]]\n\nWill learn and borrow your sentences and be shown in next patch :)\n\n> This does not work well with \"--long\", right?\n\nI think so.\n\nPeff pointed out this is arguably a bug before because\n`git ls-tree --long --name-only` do not really make sense (column of\nthe object size is not shown in result).\n\n> Usually we say A is \"Consistent\" with B when A and B are different\n> but are moral equivalent in their respective contexts. These are\n> identical, there is no difference.\n\nClearly understood now.\n\n> Lose all of the above change, except for \"Cannot be combined with\".\n> The original is just fine.\n\nWill.\n\n> Or \"--long\"? Does this work with it?\n\nAs I understand the disscussed context so far, the \"--long\", \"--name-only\"\nand \"--oid-only\" they should be mutually exclusive with each other.\n\n> I suspect that \"--long\" would be part of this, if we were to go this\n> route.\n\nAgree.\n\n> OPT_CMDMODE() is a handy way to ensure \"--name-only\", \"--oid-only\",\n> and \"--long\" are not given together, but it may be overkill to make\n> only two or three options mutually exclusive.\n>\n> In any case, once we pass the parsing part, the code should\n> translate the option into a bitmask that specifies which among\n> , , , , and fields are\n> shown. It will result in cleaner code in show_tree() if it uses\n> that set of fields to decide what is shown and how without looking\n> at the cmdmode enum.\n\nMake sense.\n\nThanks.\n"},{"id":"443395","messageId":"cover.1638891420.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1637642029.git.dyroneteng@gmail.com","subject":"[PATCH v5 0/1] support `--object-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-12-08T02:08:30Z","receivedAt":"2021-12-08T02:08:42Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\nDiffs from patch v4:\n\n* Change `--oid-only` to `--object-only`.\n Word \"oid\" may not be easily understood for users.\n\n* The commit message was modified in terms of Junio's advice.\n\n* Use \"OPT_CMDMODE()\" to make `--name-only`, `--object-only` and\n `--long` mutually exclusive with each other.\n\n* After options been parsed, translate options to bitmask, then use\n cleaner bitwise to determine which fields will be shown.\n\n* Add tests for mutually exclusive options.\n\n* Documentation modifications about the change of option name.\n\nThanks.\n\nTeng Long (1):\n ls-tree.c: support `--object-only` option for \"git-ls-tree\"\n\n Documentation/git-ls-tree.txt | 7 +-\n builtin/ls-tree.c | 125 ++++++++++++++++++++++++----------\n t/t3103-ls-tree-misc.sh | 8 +++\n t/t3104-ls-tree-oid.sh | 51 ++++++++++++++\n 4 files changed, 154 insertions(+), 37 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\nRange-diff against v4:\n-: ---------- > 1: 38d55a878c ls-tree.c: support `--object-only` option for \"git-ls-tree\"\n-- \n2.33.1.10.gd2a07a0ec5.dirty\n\n"},{"id":"443396","messageId":"38d55a878c51104f0a5337481fdcd5c8818660ac.1638891420.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1638891420.git.dyroneteng@gmail.com","subject":"[PATCH v5 1/1] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-12-08T02:08:31Z","receivedAt":"2021-12-08T02:08:44Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"We usually pipe the output from `git ls-trees` to tools like\n`sed` or `cut` when we only want to extract some fields.\n\nWhen we want only the pathname component, we can pass\n`--name-only` option to omit such a pipeline, but there are no\noptions for extracting other fields.\n\nTeach the \"--object-only\" option to the command to only show the\nobject name. This option cannot be used together with\n\"--name-only\" or \"--long\" (mutually exclusive).\n\nSigned-off-by: Teng Long \n---\n Documentation/git-ls-tree.txt | 7 +-\n builtin/ls-tree.c | 125 ++++++++++++++++++++++++----------\n t/t3103-ls-tree-misc.sh | 8 +++\n t/t3104-ls-tree-oid.sh | 51 ++++++++++++++\n 4 files changed, 154 insertions(+), 37 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex db02d6d79a..729370f235 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] [--full-name] [--full-tree] [--abbrev[=]]\n+\t [--name-only] [--name-status] [--object-only] [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n@@ -59,6 +59,11 @@ OPTIONS\n --name-only::\n --name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n+\tCannot be combined with `--object-only`.\n+\n+--object-only::\n+\tList only names of the objects, one per line. Cannot be combined\n+\twith `--name-only` or `--name-status`.\n \n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c7..beaa8bf13b 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -16,21 +16,38 @@\n \n static int line_termination = '\\n';\n #define LS_RECURSIVE 1\n-#define LS_TREE_ONLY 2\n-#define LS_SHOW_TREES 4\n-#define LS_NAME_ONLY 8\n-#define LS_SHOW_SIZE 16\n+#define LS_TREE_ONLY 1 << 1\n+#define LS_SHOW_TREES 1 << 2\n+#define LS_NAME_ONLY 1 << 3\n+#define LS_SHOW_SIZE 1 << 4\n+#define LS_OBJECT_ONLY 1 << 5\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 unsigned int shown_bits = 0;\n+#define SHOW_DEFAULT 29 /* 11101 size is not shown to output by default */\n+#define SHOW_MODE 1 << 4\n+#define SHOW_TYPE 1 << 3\n+#define SHOW_OBJECT_NAME 1 << 2\n+#define SHOW_SIZE 1 << 1\n+#define SHOW_FILE_NAME 1\n \n static const char * const ls_tree_usage[] = {\n \tN_(\"git ls-tree [] [...]\"),\n \tNULL\n };\n \n+enum {\n+\tMODE_UNSPECIFIED = 0,\n+\tMODE_NAME_ONLY,\n+\tMODE_OBJECT_ONLY,\n+\tMODE_LONG\n+};\n+\n+static int cmdmode = MODE_UNSPECIFIED;\n+\n static int show_recursive(const char *base, int baselen, const char *pathname)\n {\n \tint i;\n@@ -66,6 +83,7 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n {\n \tint retval = 0;\n \tint baselen;\n+\tint follow = 0;\n \tconst char *type = blob_type;\n \n \tif (S_ISGITLINK(mode)) {\n@@ -74,8 +92,8 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \t\t *\n \t\t * Something similar to this incomplete example:\n \t\t *\n-\t\tif (show_subprojects(base, baselen, pathname))\n-\t\t\tretval = READ_TREE_RECURSIVE;\n+\t\t * if (show_subprojects(base, baselen, pathname))\n+\t\t *\tretval = READ_TREE_RECURSIVE;\n \t\t *\n \t\t */\n \t\ttype = commit_type;\n@@ -90,35 +108,67 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif (!(ls_options & LS_NAME_ONLY)) {\n-\t\tif (ls_options & LS_SHOW_SIZE) {\n-\t\t\tchar size_text[24];\n-\t\t\tif (!strcmp(type, blob_type)) {\n-\t\t\t\tunsigned long size;\n-\t\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n-\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n-\t\t\t\t\t\t \"BAD\");\n-\t\t\t\telse\n-\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n-\t\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n-\t\t\t} else\n-\t\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n-\t\t\tprintf(\"%06o %s %s %7s\\t\", mode, type,\n-\t\t\t find_unique_abbrev(oid, abbrev),\n-\t\t\t size_text);\n+\tif (shown_bits & SHOW_MODE) {\n+\t\tprintf(\"%06o\",mode);\n+\t\tfollow = 1;\n+\t}\n+\tif (shown_bits & SHOW_TYPE) {\n+\t\tprintf(\"%s%s\", follow == 1 ? \" \" : \"\", type);\n+\t\tfollow = 1;\n+\t}\n+\tif (shown_bits & SHOW_OBJECT_NAME) {\n+\t\tprintf(\"%s%s\", follow == 1 ? \" \" : \"\",\n+\t\t find_unique_abbrev(oid, abbrev));\n+\t\tif (!(shown_bits ^ SHOW_OBJECT_NAME))\n+\t\t\tprintf(\"%c\", line_termination);\n+\t\tfollow = 1;\n+\t}\n+\tif (shown_bits & SHOW_SIZE) {\n+\t\tchar size_text[24];\n+\t\tif (!strcmp(type, blob_type)) {\n+\t\t\tunsigned long size;\n+\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n+\t\t\t\txsnprintf(size_text, sizeof(size_text), \"BAD\");\n+\t\t\telse\n+\t\t\t\txsnprintf(size_text, sizeof(size_text),\n+\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n \t\t} else\n-\t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n-\t\t\t find_unique_abbrev(oid, abbrev));\n+\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n+\t\tprintf(\"%s%7s\", follow == 1 ? \" \" : \"\", size_text);\n+\t\tfollow = 1;\n+\t}\n+\tif (shown_bits & SHOW_FILE_NAME) {\n+\t\tif (follow)\n+\t\t\tprintf(\"\\t\");\n+\t\tbaselen = base->len;\n+\t\tstrbuf_addstr(base, pathname);\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\tstrbuf_setlen(base, baselen);\n \t}\n-\tbaselen = base->len;\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-\tstrbuf_setlen(base, baselen);\n \treturn retval;\n }\n \n+static int parse_shown_fields(void)\n+{\n+\tif (cmdmode == MODE_NAME_ONLY) {\n+\t\tshown_bits = SHOW_FILE_NAME;\n+\t\treturn 0;\n+\t}\n+\tif (cmdmode == MODE_OBJECT_ONLY) {\n+\t\tshown_bits = SHOW_OBJECT_NAME;\n+\t\treturn 0;\n+\t}\n+\tif (!ls_options || (ls_options & LS_RECURSIVE)\n+\t || (ls_options & LS_SHOW_TREES)\n+\t || (ls_options & LS_TREE_ONLY))\n+\t\tshown_bits = SHOW_DEFAULT;\n+\tif (cmdmode == MODE_LONG)\n+\t\tshown_bits = SHOW_DEFAULT | SHOW_SIZE;\n+\treturn 1;\n+}\n+\n int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_id oid;\n@@ -133,12 +183,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\tLS_SHOW_TREES),\n \t\tOPT_SET_INT('z', NULL, &line_termination,\n \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n-\t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n-\t\t\tLS_SHOW_SIZE),\n-\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n-\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\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+\t\t\t MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE(0, \"name-status\", &cmdmode, N_(\"list only filenames\"),\n+\t\t\t MODE_NAME_ONLY),\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\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n@@ -169,6 +221,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tif (get_oid(argv[0], &oid))\n \t\tdie(\"Not a valid object name %s\", argv[0]);\n \n+\tparse_shown_fields();\n \t/*\n \t * show_recursive() rolls its own matching code and is\n \t * generally ignorant of 'struct pathspec'. The magic mask\ndiff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh\nindex 14520913af..75e38b0a51 100755\n--- a/t/t3103-ls-tree-misc.sh\n+++ b/t/t3103-ls-tree-misc.sh\n@@ -22,4 +22,12 @@ test_expect_success 'ls-tree fails with non-zero exit code on broken tree' '\n \ttest_must_fail git ls-tree -r HEAD\n '\n \n+test_expect_success 'usage: incompatible options: --name-status with --long' '\n+\ttest_expect_code 129 git ls-tree --long --name-status\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-only with --long' '\n+\ttest_expect_code 129 git ls-tree --long --name-only\n+'\n+\n test_done\ndiff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\nnew file mode 100755\nindex 0000000000..81304e7b13\n--- /dev/null\n+++ b/t/t3104-ls-tree-oid.sh\n@@ -0,0 +1,51 @@\n+#!/bin/sh\n+\n+test_description='git ls-tree objects handling.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit A &&\n+\ttest_commit B &&\n+\tmkdir -p C &&\n+\ttest_commit C/D.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=$(git write-tree) &&\n+\techo $tree\n+'\n+\n+test_expect_success 'usage: --object-only' '\n+\tgit ls-tree --object-only $tree >current &&\n+\tgit ls-tree $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --object-only with -r' '\n+\tgit ls-tree --object-only -r $tree >current &&\n+\tgit ls-tree -r $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --object-only with --abbrev' '\n+\tgit ls-tree --object-only --abbrev=6 $tree >current &&\n+\tgit ls-tree --abbrev=6 $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-only with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --name-only\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-status with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --name-status\n+'\n+\n+test_expect_success 'usage: incompatible options: --long with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --long\n+'\n+\n+test_done\n-- \n2.33.1.10.gd2a07a0ec5.dirty\n\n"},{"id":"444162","messageId":"xmqqk0g5656r.fsf@gitster.g","threadId":"56905","inReplyTo":"38d55a878c51104f0a5337481fdcd5c8818660ac.1638891420.git.dyroneteng@gmail.com","subject":"Re: [PATCH v5 1/1] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-15T19:25:48Z","receivedAt":"2021-12-15T19:25:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> We usually pipe the output from `git ls-trees` to tools like\n> `sed` or `cut` when we only want to extract some fields.\n>\n> When we want only the pathname component, we can pass\n> `--name-only` option to omit such a pipeline, but there are no\n> options for extracting other fields.\n>\n> Teach the \"--object-only\" option to the command to only show the\n> object name. This option cannot be used together with\n> \"--name-only\" or \"--long\" (mutually exclusive).\n\nI notice that this changed from --oid to --object and I agree that\nit would probably be more friendly to end users. In fact, this\n\n $ sed -ne '/^SYNOPSIS/,/^DESCRIPTION/p' Documentation/git-*.txt |\n grep -e -oid\n\ndid not find any hits.\n\n> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\n> index 3a442631c7..beaa8bf13b 100644\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -16,21 +16,38 @@\n> \n> static int line_termination = '\\n';\n> #define LS_RECURSIVE 1\n> -#define LS_TREE_ONLY 2\n> -#define LS_SHOW_TREES 4\n> -#define LS_NAME_ONLY 8\n> -#define LS_SHOW_SIZE 16\n> +#define LS_TREE_ONLY 1 << 1\n> +#define LS_SHOW_TREES 1 << 2\n> +#define LS_NAME_ONLY 1 << 3\n> +#define LS_SHOW_SIZE 1 << 4\n> +#define LS_OBJECT_ONLY 1 << 5\n\nIt usually is a good idea to enclose these in () so that they are\nsafe to use in any context in a statement.\n\nLuckily, bitwise-or and bitwise-and, which are the most likely\ncandidates for these symbols to be used with, bind looser than\nleft-shift, so something like \n\n\tif ((LS_TREE_ONLY | LS_SHOW_TREES) & opt)\n\t\t... do this ...\n\nis safe either way, but (LS_TREE_ONLY + LS_SHOW_TREES) would have\ndifferent value with and without () around (1 << N).\n\n> +static unsigned int shown_bits = 0;\n\nStyle: we do not initialize statics explicitly to zero.\n\n> +#define SHOW_DEFAULT 29 /* 11101 size is not shown to output by default */\n> +#define SHOW_MODE 1 << 4\n> +#define SHOW_TYPE 1 << 3\n> +#define SHOW_OBJECT_NAME 1 << 2\n> +#define SHOW_SIZE 1 << 1\n> +#define SHOW_FILE_NAME 1\n\nLikewise. It is a bit curious to see these listed in decreasing\norder, though.\n\n> static const char * const ls_tree_usage[] = {\n> \tN_(\"git ls-tree [] [...]\"),\n> \tNULL\n> };\n> \n> +enum {\n> +\tMODE_UNSPECIFIED = 0,\n> +\tMODE_NAME_ONLY,\n> +\tMODE_OBJECT_ONLY,\n> +\tMODE_LONG\n\nIt is a good idea to leave a comma even after the last element,\n_unless_ there is a strong reason why the element that currently is\nat the last MUST stay to be last when new elements are added to the\nenum. That way, a future patch that adds a new element can add it\nto the list with a patch noise with fewer lines.\n\n> +};\n> +\n> +static int cmdmode = MODE_UNSPECIFIED;\n\nThis is also initializing a static variable to zero, and arguments\ncan be made either way: (A) unspecified is set to zero in enum\ndefinition exactly so that we can use zero to signal the variable is\nunspecified, so an explicit zero initialization here goes against\nthe spirit of choosing 0 as MODE_UNSPECIFIED; or (B) enum definition\ncan be scrambled in future changes to use something other than zero\nfor MODE_UNSPECIFIED, and explicitly writing it like this is more\nfuture-proof.\n\nI am OK with the way it is written (i.e. (B)).\n\n> @@ -66,6 +83,7 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> {\n> \tint retval = 0;\n> \tint baselen;\n> +\tint follow = 0;\n\nA better name, anybody?\n\nThis bit is to keep track of the fact that we made _some_ output\nalready so any further output needs an inter-field space before\nwriting what it wants to write out.\n\n> \tconst char *type = blob_type;\n> \n> \tif (S_ISGITLINK(mode)) {\n> @@ -74,8 +92,8 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> \t\t *\n> \t\t * Something similar to this incomplete example:\n> \t\t *\n> -\t\tif (show_subprojects(base, baselen, pathname))\n> -\t\t\tretval = READ_TREE_RECURSIVE;\n> +\t\t * if (show_subprojects(base, baselen, pathname))\n> +\t\t *\tretval = READ_TREE_RECURSIVE;\n> \t\t *\n> \t\t */\n\nNice ;-)\n\n> \t\ttype = commit_type;\n> @@ -90,35 +108,67 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n> \telse if (ls_options & LS_TREE_ONLY)\n> \t\treturn 0;\n> \n> -\tif (!(ls_options & LS_NAME_ONLY)) {\n> -\t\tif (ls_options & LS_SHOW_SIZE) {\n> -\t\t\tchar size_text[24];\n> -\t\t\tif (!strcmp(type, blob_type)) {\n> -\t\t\t\tunsigned long size;\n> -\t\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n> -\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n> -\t\t\t\t\t\t \"BAD\");\n> -\t\t\t\telse\n> -\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n> -\t\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n> -\t\t\t} else\n> -\t\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n> -\t\t\tprintf(\"%06o %s %s %7s\\t\", mode, type,\n> -\t\t\t find_unique_abbrev(oid, abbrev),\n> -\t\t\t size_text);\n> +\tif (shown_bits & SHOW_MODE) {\n> +\t\tprintf(\"%06o\",mode);\n\nSP before ','.\n\n> +\t\tfollow = 1;\n> +\t}\n> +\tif (shown_bits & SHOW_TYPE) {\n> +\t\tprintf(\"%s%s\", follow == 1 ? \" \" : \"\", type);\n> +\t\tfollow = 1;\n> +\t}\n> +\tif (shown_bits & SHOW_OBJECT_NAME) {\n> +\t\tprintf(\"%s%s\", follow == 1 ? \" \" : \"\",\n> +\t\t find_unique_abbrev(oid, abbrev));\n> +\t\tif (!(shown_bits ^ SHOW_OBJECT_NAME))\n> +\t\t\tprintf(\"%c\", line_termination);\n\nCurious. I wonder if we can get rid of these two lines (and the\nline_termination bit in the SHOW_FILE_NAME part), and have an\nunconditional \n\n\tputchar(line_termination);\n\nat the end of the function.\n\nThat way, we could in the future choose to introduce a feature to\nshow only and nothing else, which may be useful\nfor taking per-type stats.\n\nWe need to stop using write_name_quoted_relative() in SHOW_FILE_NAME\npart, because the helper insists that the name written by it must be\nat the end of the entry, if we go that route, but it may be a good\nchange in the longer term.\n\n> +\t\tfollow = 1;\n> +\t}\n> +\tif (shown_bits & SHOW_SIZE) {\n> +\t\tchar size_text[24];\n> +\t\tif (!strcmp(type, blob_type)) {\n> +\t\t\tunsigned long size;\n> +\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n> +\t\t\t\txsnprintf(size_text, sizeof(size_text), \"BAD\");\n> +\t\t\telse\n> +\t\t\t\txsnprintf(size_text, sizeof(size_text),\n> +\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n> \t\t} else\n> -\t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n> -\t\t\t find_unique_abbrev(oid, abbrev));\n> +\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n> +\t\tprintf(\"%s%7s\", follow == 1 ? \" \" : \"\", size_text);\n> +\t\tfollow = 1;\n> +\t}\n> +\tif (shown_bits & SHOW_FILE_NAME) {\n> +\t\tif (follow)\n> +\t\t\tprintf(\"\\t\");\n> +\t\tbaselen = base->len;\n> +\t\tstrbuf_addstr(base, pathname);\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\tstrbuf_setlen(base, baselen);\n> \t}\n\nBut the above nits aside, the updated organization of this function\nlooks much cleaner than the original. Nicely reorganized.\n\n> -\tbaselen = base->len;\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> -\tstrbuf_setlen(base, baselen);\n> \treturn retval;\n> }\n\nThanks.\n"},{"id":"444203","messageId":"20211216121630.14553-1-dyroneteng@gmail.com","threadId":"56905","inReplyTo":"xmqqk0g5656r.fsf@gitster.g","subject":"Re: [PATCH v5 1/1] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-12-16T12:16:30Z","receivedAt":"2021-12-16T12:16:38Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"\n> I notice that this changed from --oid to --object and I agree that\n> it would probably be more friendly to end users. In fact, this\n>\n> $ sed -ne '/^SYNOPSIS/,/^DESCRIPTION/p' Documentation/git-*.txt |\n> grep -e -oid\n>\n> did not find any hits.\n\nThank you for comfirming that.\n\n> It usually is a good idea to enclose these in () so that they are\n> safe to use in any context in a statement.\n> \n> Luckily, bitwise-or and bitwise-and, which are the most likely\n> candidates for these symbols to be used with, bind looser than\n> left-shift, so something like \n> \n> \tif ((LS_TREE_ONLY | LS_SHOW_TREES) & opt)\n> \t\t... do this ...\n> \n> is safe either way, but (LS_TREE_ONLY + LS_SHOW_TREES) would have\n> different value with and without () around (1 << N).\n\nMake sense.\n\nWill fix in patch v6.\n\n\n> Style: we do not initialize statics explicitly to zero.\n> ...\n> Likewise. It is a bit curious to see these listed in decreasing\n> order, though.\n\nWill fix.\n\n> It is a good idea to leave a comma even after the last element,\n> _unless_ there is a strong reason why the element that currently is\n> at the last MUST stay to be last when new elements are added to the\n> enum. That way, a future patch that adds a new element can add it\n> to the list with a patch noise with fewer lines.\n\nVery clear explanation.\n\nWill fix.\n\n> A better name, anybody?\n> \n> This bit is to keep track of the fact that we made _some_ output\n> already so any further output needs an inter-field space before\n> writing what it wants to write out.\n\nI found a word \"interspace\", it looks like a little better than the old\none. I will rename to it in next patch, and If there's a better idea,\nwill apply further.\n\n> SP before ','.\n\nWill fix.\n\n\n> Curious. I wonder if we can get rid of these two lines (and the\n> line_termination bit in the SHOW_FILE_NAME part), and have an\n> unconditional \n> \n> \tputchar(line_termination);\n> \n> at the end of the function.\n> \n> That way, we could in the future choose to introduce a feature to\n> show only and nothing else, which may be useful\n> for taking per-type stats.\n> \n> We need to stop using write_name_quoted_relative() in SHOW_FILE_NAME\n> part, because the helper insists that the name written by it must be\n> at the end of the entry, if we go that route, but it may be a good\n> change in the longer term.\n\nLet me try to represent to make sure I understand your suggestion\nsufficiently.\n\n\"write_name_quoted_relative\" is used to compute the relative file name\nby \"prefix\" and output the name and a line termination to the given FD.\n\nWe do not want use \"write_name_quoted_relative\" in here because the\nfunction alway output a line termination after \"name\", this may bring\nsome inconvenience because the \"name\" may not be the last field in the\nfuture.\n\nSo, instead:\n\nWe need to calculate the file name (relative path and quotes if need)\nwithout \"write_name_quoted_relative\" and then output the line\ntermination before return.\n\n\nThanks.\n"},{"id":"444258","messageId":"xmqq7dc49r7j.fsf@gitster.g","threadId":"56905","inReplyTo":"20211216121630.14553-1-dyroneteng@gmail.com","subject":"Re: [PATCH v5 1/1] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-16T21:26:24Z","receivedAt":"2021-12-16T21:26:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n>> A better name, anybody?\n>> \n>> This bit is to keep track of the fact that we made _some_ output\n>> already so any further output needs an inter-field space before\n>> writing what it wants to write out.\n>\n> I found a word \"interspace\", it looks like a little better than the old\n> one. I will rename to it in next patch, and If there's a better idea,\n> will apply further.\n\nYeah, it really is needs_inter_field_space but that is way too long.\n\n>> We need to stop using write_name_quoted_relative() in SHOW_FILE_NAME\n>> part, because the helper insists that the name written by it must be\n>> at the end of the entry, if we go that route, but it may be a good\n>> change in the longer term.\n>\n> Let me try to represent to make sure I understand your suggestion\n> sufficiently.\n>\n> \"write_name_quoted_relative\" is used to compute the relative file name\n> by \"prefix\" and output the name and a line termination to the given FD.\n>\n> We do not want use \"write_name_quoted_relative\" in here because the\n> function alway output a line termination after \"name\", this may bring\n> some inconvenience because the \"name\" may not be the last field in the\n> future.\n>\n> So, instead:\n>\n> We need to calculate the file name (relative path and quotes if need)\n> without \"write_name_quoted_relative\" and then output the line\n> termination before return.\n\nI think we are on the same page. We can work backwards, I think.\n\nWe have a repetitive\n\n if (mode should be shown) {\n show mode;\n record that we have already shown something;\n }\n if (type should be shown) {\n give inter-field-space if we have shown something;\n show type;\n record that we have already shown something;\n }\n ...\n\nthat ends with\n\n if (name should be shown) {\n give inter-field-space if we have shown something;\n show name PLUS line termination;\n\t}\n\nBut if we can make the last step to\n\n if (name should be shown) {\n give inter-field-space if we have shown something;\n show name;\n\t}\n\n\tgive line termination;\n\nit gets easier to support a combination that does not show name, and\nwe can have inter-record separator.\n\nBut write_name_quoted_relative() does not give the caller a choice\nto have no terminator, so we need to do something like this:\n\n\tif (shown_bits & SHOW_FILE_NAME) {\n\t\tconst char *name;\n struct strbuf name_buf = STRBUF_INIT;\n\n\t\tif (follow)\n\t\t\tprintf(\"\\t\");\n\t\tbaselen = base->len;\n\t\tstrbuf_addstr(base, pathname);\n \n\t\tname = relative_path(base->buf, \n\t\t\t\t chomp_prefix ? ls_tree_prefix : NULL,\n &name_buf);\n\t\tif (line_termination)\n\t\t\tquote_c_style(name, NULL, stdout, 0);\n\t\telse\n\t\t\tfputs(name, stdout);\n\t\tstrbuf_release(&name_buf);\n\t\tstrbuf_setlen(base, baselen);\n\t}\n\nI initially thought that extending write_name_quoted() and\nwrite_name_quoted_relative() to accept a special value or two for\nterminator to tell it not to add terminator would be sufficient (see\nbelow). I however think it is way too ugly to have the \"add no\nterminator and do not quote\" option at write_name_quoted() level,\nsimply because the caller that chooses as-is can simply do fputs()\nitself without bothering to use write_name_quoted(). So I am not\nconvinced that it will a good idea.\n\nIf we were to go that \"ugly helper\" route, the above can become even\nsimpler and closer to what you originally wrote, e.g.\n\n\tif (shown_bits & SHOW_FILE_NAME) {\n\t\tif (follow)\n\t\t\tprintf(\"\\t\");\n\t\tbaselen = base->len;\n\t\tstrbuf_addstr(base, pathname);\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,\n line_termination \n ? CQ_NO_TERMINATOR_C_QUOTED\n : CQ_NO_TERMINATOR_AS_IS);\n\t\tstrbuf_setlen(base, baselen);\n\t}\n\n\n quote.c | 5 +++--\n quote.h | 19 +++++++++++++++++++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git c/quote.c w/quote.c\nindex 26719d21d1..cbbcd8563f 100644\n--- c/quote.c\n+++ w/quote.c\n@@ -340,12 +340,13 @@ void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path,\n \n void write_name_quoted(const char *name, FILE *fp, int terminator)\n {\n-\tif (terminator) {\n+\tif (0 < terminator || terminator == CQ_NO_TERMINATOR_C_QUOTED) {\n \t\tquote_c_style(name, NULL, fp, 0);\n \t} else {\n \t\tfputs(name, fp);\n \t}\n-\tfputc(terminator, fp);\n+\tif (0 <= terminator)\n+\t\tfputc(terminator, fp);\n }\n \n void write_name_quoted_relative(const char *name, const char *prefix,\ndiff --git c/quote.h w/quote.h\nindex 87ff458b06..5c8c7cf952 100644\n--- c/quote.h\n+++ w/quote.h\n@@ -85,7 +85,26 @@ int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned);\n void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned);\n \n+/*\n+ * Write a name, typically a filename, followed by a terminator that\n+ * separates it from what comes next.\n+ * When terminator is NUL, the name is given as-is. Otherwise, the\n+ * name is c-quoted, suitable for text output. HT and LF are typical\n+ * values used for the terminator, but other positive values are possible.\n+ *\n+ * In addition to non-negative values two special values in terminator\n+ * are possible.\n+ * -1: show the name c-quoted, without adding any terminator.\n+ * -2: show the name as-is, without adding any terminator.\n+ */\n+#define CQ_NO_TERMINATOR_C_QUOTED\t(-1)\n+#define CQ_NO_TERMINATOR_AS_IS\t\t(-2)\n void write_name_quoted(const char *name, FILE *, int terminator);\n+\n+/*\n+ * Similar to the above, but the name is first made relative to the prefix\n+ * before being shown.\n+ */ \n void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator);\n \n\n"},{"id":"444261","messageId":"211216.86mtl09qwl.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"xmqq7dc49r7j.fsf@gitster.g","subject":"Re: [PATCH v5 1/1] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T21:29:45Z","receivedAt":"2021-12-16T21:33:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 16 2021, Junio C Hamano wrote:\n\n> Teng Long writes:\n>\n>>> A better name, anybody?\n>>> \n>>> This bit is to keep track of the fact that we made _some_ output\n>>> already so any further output needs an inter-field space before\n>>> writing what it wants to write out.\n>>\n>> I found a word \"interspace\", it looks like a little better than the old\n>> one. I will rename to it in next patch, and If there's a better idea,\n>> will apply further.\n>\n> Yeah, it really is needs_inter_field_space but that is way too long.\n>\n>>> We need to stop using write_name_quoted_relative() in SHOW_FILE_NAME\n>>> part, because the helper insists that the name written by it must be\n>>> at the end of the entry, if we go that route, but it may be a good\n>>> change in the longer term.\n>>\n>> Let me try to represent to make sure I understand your suggestion\n>> sufficiently.\n>>\n>> \"write_name_quoted_relative\" is used to compute the relative file name\n>> by \"prefix\" and output the name and a line termination to the given FD.\n>>\n>> We do not want use \"write_name_quoted_relative\" in here because the\n>> function alway output a line termination after \"name\", this may bring\n>> some inconvenience because the \"name\" may not be the last field in the\n>> future.\n>>\n>> So, instead:\n>>\n>> We need to calculate the file name (relative path and quotes if need)\n>> without \"write_name_quoted_relative\" and then output the line\n>> termination before return.\n>\n> I think we are on the same page. We can work backwards, I think.\n>\n> We have a repetitive\n>\n> if (mode should be shown) {\n> show mode;\n> record that we have already shown something;\n> }\n> if (type should be shown) {\n> give inter-field-space if we have shown something;\n> show type;\n> record that we have already shown something;\n> }\n> ...\n>\n> that ends with\n>\n> if (name should be shown) {\n> give inter-field-space if we have shown something;\n> show name PLUS line termination;\n> \t}\n>\n> But if we can make the last step to\n>\n> if (name should be shown) {\n> give inter-field-space if we have shown something;\n> show name;\n> \t}\n>\n> \tgive line termination;\n>\n> it gets easier to support a combination that does not show name, and\n> we can have inter-record separator.\n>\n> But write_name_quoted_relative() does not give the caller a choice\n> to have no terminator, so we need to do something like this:\n>\n> \tif (shown_bits & SHOW_FILE_NAME) {\n> \t\tconst char *name;\n> struct strbuf name_buf = STRBUF_INIT;\n>\n> \t\tif (follow)\n> \t\t\tprintf(\"\\t\");\n> \t\tbaselen = base->len;\n> \t\tstrbuf_addstr(base, pathname);\n> \n> \t\tname = relative_path(base->buf, \n> \t\t\t\t chomp_prefix ? ls_tree_prefix : NULL,\n> &name_buf);\n> \t\tif (line_termination)\n> \t\t\tquote_c_style(name, NULL, stdout, 0);\n> \t\telse\n> \t\t\tfputs(name, stdout);\n> \t\tstrbuf_release(&name_buf);\n> \t\tstrbuf_setlen(base, baselen);\n> \t}\n>\n> I initially thought that extending write_name_quoted() and\n> write_name_quoted_relative() to accept a special value or two for\n> terminator to tell it not to add terminator would be sufficient (see\n> below). I however think it is way too ugly to have the \"add no\n> terminator and do not quote\" option at write_name_quoted() level,\n> simply because the caller that chooses as-is can simply do fputs()\n> itself without bothering to use write_name_quoted(). So I am not\n> convinced that it will a good idea.\n>\n> If we were to go that \"ugly helper\" route, the above can become even\n> simpler and closer to what you originally wrote, e.g.\n>\n> \tif (shown_bits & SHOW_FILE_NAME) {\n> \t\tif (follow)\n> \t\t\tprintf(\"\\t\");\n> \t\tbaselen = base->len;\n> \t\tstrbuf_addstr(base, pathname);\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,\n> line_termination \n> ? CQ_NO_TERMINATOR_C_QUOTED\n> : CQ_NO_TERMINATOR_AS_IS);\n> \t\tstrbuf_setlen(base, baselen);\n> \t}\n>\n>\n> quote.c | 5 +++--\n> quote.h | 19 +++++++++++++++++++\n> 2 files changed, 22 insertions(+), 2 deletions(-)\n>\n> diff --git c/quote.c w/quote.c\n> index 26719d21d1..cbbcd8563f 100644\n> --- c/quote.c\n> +++ w/quote.c\n> @@ -340,12 +340,13 @@ void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path,\n> \n> void write_name_quoted(const char *name, FILE *fp, int terminator)\n> {\n> -\tif (terminator) {\n> +\tif (0 < terminator || terminator == CQ_NO_TERMINATOR_C_QUOTED) {\n> \t\tquote_c_style(name, NULL, fp, 0);\n> \t} else {\n> \t\tfputs(name, fp);\n> \t}\n> -\tfputc(terminator, fp);\n> +\tif (0 <= terminator)\n> +\t\tfputc(terminator, fp);\n> }\n> \n> void write_name_quoted_relative(const char *name, const char *prefix,\n> diff --git c/quote.h w/quote.h\n> index 87ff458b06..5c8c7cf952 100644\n> --- c/quote.h\n> +++ w/quote.h\n> @@ -85,7 +85,26 @@ int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n> size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned);\n> void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned);\n> \n> +/*\n> + * Write a name, typically a filename, followed by a terminator that\n> + * separates it from what comes next.\n> + * When terminator is NUL, the name is given as-is. Otherwise, the\n> + * name is c-quoted, suitable for text output. HT and LF are typical\n> + * values used for the terminator, but other positive values are possible.\n> + *\n> + * In addition to non-negative values two special values in terminator\n> + * are possible.\n> + * -1: show the name c-quoted, without adding any terminator.\n> + * -2: show the name as-is, without adding any terminator.\n> + */\n> +#define CQ_NO_TERMINATOR_C_QUOTED\t(-1)\n> +#define CQ_NO_TERMINATOR_AS_IS\t\t(-2)\n> void write_name_quoted(const char *name, FILE *, int terminator);\n> +\n> +/*\n> + * Similar to the above, but the name is first made relative to the prefix\n> + * before being shown.\n> + */ \n> void write_name_quoted_relative(const char *name, const char *prefix,\n> \t\t\t\tFILE *fp, int terminator);\n> \n\nIn my \"just make ls-tree support --format\" this whole thing is just:\n \n + if (prefix)\n + name = relative_path(name, prefix, &scratch);\n + quote_c_style(name, sb, NULL, 0);\n + strbuf_addch(sb, line_termination);\n\nBut of course that's an implementation that's moved away from the \"write\nstuff to a FH for me\" API in quote.c.\n\nSo I haven't looked carefully here, but you need this API just because\nof some constraint in the write_name_quoted()?\n\nPerhaps just running with that approach is better? Whether it's stealing\nthat approach, or doing away with --object-only for a --format...\n"},{"id":"444335","messageId":"cover.1639721750.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1638891420.git.dyroneteng@gmail.com","subject":"[PATCH v6 0/1] support `--object-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-12-17T06:57:08Z","receivedAt":"2021-12-17T06:57:25Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Diffs from patch v5:\n\n* Let \"write_name_quoted_relative\" support two terminations, they are\n \"CQ_NO_TERMINATOR_C_QUOTED\" and \"CQ_NO_TERMINATOR_AS_IS\" (They are\n used to let the function write \"name\" at a 'c_quoted' way or just\n 'as-is', but without print a terminator).\n \n* Rename \"follow\" to \"interspace\" (Representing whether any further\n output needs an inter-field space).\n\n* Nits fixed.\n\nMany thanks to Junio and Ævar for your help and patient explanation.\nI noticed Ævar suggest the solution with using `--format`, but in\nthis patch, the current approach continues. If this part of code needs\nto be improved or we want to support \"--format\" in \"ls-tree\" in the\nfuture, I'm more than glad to continue to contribute.\n\nThanks.\n\nTeng Long (1):\n ls-tree.c: support `--object-only` option for \"git-ls-tree\"\n\n Documentation/git-ls-tree.txt | 7 +-\n builtin/ls-tree.c | 131 ++++++++++++++++++++++++----------\n quote.c | 8 +--\n quote.h | 19 +++++\n t/t3103-ls-tree-misc.sh | 8 +++\n t/t3104-ls-tree-oid.sh | 51 +++++++++++++\n 6 files changed, 183 insertions(+), 41 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\nRange-diff against v5:\n1: 38d55a878c ! 1: 2e449d1c79 ls-tree.c: support `--object-only` option for \"git-ls-tree\"\n @@ builtin/ls-tree.c\n -#define LS_SHOW_TREES 4\n -#define LS_NAME_ONLY 8\n -#define LS_SHOW_SIZE 16\n -+#define LS_TREE_ONLY 1 << 1\n -+#define LS_SHOW_TREES 1 << 2\n -+#define LS_NAME_ONLY 1 << 3\n -+#define LS_SHOW_SIZE 1 << 4\n -+#define LS_OBJECT_ONLY 1 << 5\n ++#define LS_TREE_ONLY (1 << 1)\n ++#define LS_SHOW_TREES (1 << 2)\n ++#define LS_NAME_ONLY (1 << 3)\n ++#define LS_SHOW_SIZE (1 << 4)\n ++#define LS_OBJECT_ONLY (1 << 5)\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 unsigned int shown_bits = 0;\n -+#define SHOW_DEFAULT 29 /* 11101 size is not shown to output by default */\n -+#define SHOW_MODE 1 << 4\n -+#define SHOW_TYPE 1 << 3\n -+#define SHOW_OBJECT_NAME 1 << 2\n -+#define SHOW_SIZE 1 << 1\n ++static unsigned int shown_bits;\n +#define SHOW_FILE_NAME 1\n ++#define SHOW_SIZE (1 << 1)\n ++#define SHOW_OBJECT_NAME (1 << 2)\n ++#define SHOW_TYPE (1 << 3)\n ++#define SHOW_MODE (1 << 4)\n ++#define SHOW_DEFAULT 29 /* 11101 size is not shown to output by default */\n \n static const char * const ls_tree_usage[] = {\n \tN_(\"git ls-tree [] [...]\"),\n @@ builtin/ls-tree.c\n +\tMODE_UNSPECIFIED = 0,\n +\tMODE_NAME_ONLY,\n +\tMODE_OBJECT_ONLY,\n -+\tMODE_LONG\n ++\tMODE_LONG,\n +};\n +\n +static int cmdmode = MODE_UNSPECIFIED;\n @@ builtin/ls-tree.c: static int show_tree(const struct object_id *oid, struct strb\n {\n \tint retval = 0;\n \tint baselen;\n -+\tint follow = 0;\n ++\tint interspace = 0;\n \tconst char *type = blob_type;\n \n \tif (S_ISGITLINK(mode)) {\n @@ builtin/ls-tree.c: static int show_tree(const struct object_id *oid, struct strb\n -\t\t\t find_unique_abbrev(oid, abbrev),\n -\t\t\t size_text);\n +\tif (shown_bits & SHOW_MODE) {\n -+\t\tprintf(\"%06o\",mode);\n -+\t\tfollow = 1;\n ++\t\tprintf(\"%06o\", mode);\n ++\t\tinterspace = 1;\n +\t}\n +\tif (shown_bits & SHOW_TYPE) {\n -+\t\tprintf(\"%s%s\", follow == 1 ? \" \" : \"\", type);\n -+\t\tfollow = 1;\n ++\t\tprintf(\"%s%s\", interspace ? \" \" : \"\", type);\n ++\t\tinterspace = 1;\n +\t}\n +\tif (shown_bits & SHOW_OBJECT_NAME) {\n -+\t\tprintf(\"%s%s\", follow == 1 ? \" \" : \"\",\n ++\t\tprintf(\"%s%s\", interspace ? \" \" : \"\",\n +\t\t find_unique_abbrev(oid, abbrev));\n +\t\tif (!(shown_bits ^ SHOW_OBJECT_NAME))\n -+\t\t\tprintf(\"%c\", line_termination);\n -+\t\tfollow = 1;\n ++\t\t\tgoto LINE_FINISH;\n ++\t\tinterspace = 1;\n +\t}\n +\tif (shown_bits & SHOW_SIZE) {\n +\t\tchar size_text[24];\n @@ builtin/ls-tree.c: static int show_tree(const struct object_id *oid, struct strb\n -\t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n -\t\t\t find_unique_abbrev(oid, abbrev));\n +\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n -+\t\tprintf(\"%s%7s\", follow == 1 ? \" \" : \"\", size_text);\n -+\t\tfollow = 1;\n ++\t\tprintf(\"%s%7s\", interspace ? \" \" : \"\", size_text);\n ++\t\tinterspace = 1;\n +\t}\n +\tif (shown_bits & SHOW_FILE_NAME) {\n -+\t\tif (follow)\n ++\t\tif (interspace)\n +\t\t\tprintf(\"\\t\");\n +\t\tbaselen = base->len;\n +\t\tstrbuf_addstr(base, pathname);\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\t\t\t\t stdout,\n ++\t\t\t\t\t line_termination\n ++\t\t\t\t\t ? CQ_NO_TERMINATOR_C_QUOTED\n ++\t\t\t\t\t : CQ_NO_TERMINATOR_AS_IS);\n +\t\tstrbuf_setlen(base, baselen);\n \t}\n -\tbaselen = base->len;\n @@ builtin/ls-tree.c: static int show_tree(const struct object_id *oid, struct strb\n -\t\t\t\t chomp_prefix ? ls_tree_prefix : NULL,\n -\t\t\t\t stdout, line_termination);\n -\tstrbuf_setlen(base, baselen);\n ++\n ++LINE_FINISH:\n ++\tputchar(line_termination);\n \treturn retval;\n }\n \n @@ builtin/ls-tree.c: int cmd_ls_tree(int argc, const char **argv, const char *pref\n \t * show_recursive() rolls its own matching code and is\n \t * generally ignorant of 'struct pathspec'. The magic mask\n \n + ## quote.c ##\n +@@ quote.c: void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path,\n + \n + void write_name_quoted(const char *name, FILE *fp, int terminator)\n + {\n +-\tif (terminator) {\n ++\tif (0 < terminator || terminator == CQ_NO_TERMINATOR_C_QUOTED)\n + \t\tquote_c_style(name, NULL, fp, 0);\n +-\t} else {\n ++\telse\n + \t\tfputs(name, fp);\n +-\t}\n +-\tfputc(terminator, fp);\n ++\tif (0 <= terminator)\n ++\t\tfputc(terminator, fp);\n + }\n + \n + void write_name_quoted_relative(const char *name, const char *prefix,\n +\n + ## quote.h ##\n +@@ quote.h: int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n + #define CQUOTE_NODQ 01\n + size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned);\n + void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned);\n ++/*\n ++ * Write a name, typically a filename, followed by a terminator that\n ++ * separates it from what comes next.\n ++ * When terminator is NUL, the name is given as-is. Otherwise, the\n ++ * name is c-quoted, suitable for text output. HT and LF are typical\n ++ * values used for the terminator, but other positive values are possible.\n ++ *\n ++ * In addition to non-negative values two special values in terminator\n ++ * are possible.\n ++ *\n ++ * -1: show the name c-quoted, without adding any terminator.\n ++ * -2: show the name as-is, without adding any terminator.\n ++ */\n ++#define CQ_NO_TERMINATOR_C_QUOTED\t(-1)\n ++#define CQ_NO_TERMINATOR_AS_IS\t\t(-2)\n + \n + void write_name_quoted(const char *name, FILE *, int terminator);\n ++/*\n ++ * Similar to the above, but the name is first made relative to the prefix\n ++ * before being shown.\n ++ */\n + void write_name_quoted_relative(const char *name, const char *prefix,\n + \t\t\t\tFILE *fp, int terminator);\n + \n +\n ## t/t3103-ls-tree-misc.sh ##\n @@ t/t3103-ls-tree-misc.sh: test_expect_success 'ls-tree fails with non-zero exit code on broken tree' '\n \ttest_must_fail git ls-tree -r HEAD\n-- \n2.33.1.10.g5f17c1a2c1.dirty\n\n"},{"id":"444336","messageId":"2e449d1c792ff81da5f22c8bf65ed33c393d62f8.1639721750.git.dyroneteng@gmail.com","threadId":"56905","inReplyTo":"cover.1639721750.git.dyroneteng@gmail.com","subject":"[PATCH v6 1/1] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-12-17T06:57:09Z","receivedAt":"2021-12-17T06:57:28Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"We usually pipe the output from `git ls-trees` to tools like\n`sed` or `cut` when we only want to extract some fields.\n\nWhen we want only the pathname component, we can pass\n`--name-only` option to omit such a pipeline, but there are no\noptions for extracting other fields.\n\nTeach the \"--object-only\" option to the command to only show the\nobject name. This option cannot be used together with\n\"--name-only\" or \"--long\" (mutually exclusive).\n\nSigned-off-by: Teng Long \n---\n Documentation/git-ls-tree.txt | 7 +-\n builtin/ls-tree.c | 131 ++++++++++++++++++++++++----------\n quote.c | 8 +--\n quote.h | 19 +++++\n t/t3103-ls-tree-misc.sh | 8 +++\n t/t3104-ls-tree-oid.sh | 51 +++++++++++++\n 6 files changed, 183 insertions(+), 41 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex db02d6d79a..729370f235 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] [--full-name] [--full-tree] [--abbrev[=]]\n+\t [--name-only] [--name-status] [--object-only] [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n@@ -59,6 +59,11 @@ OPTIONS\n --name-only::\n --name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n+\tCannot be combined with `--object-only`.\n+\n+--object-only::\n+\tList only names of the objects, one per line. Cannot be combined\n+\twith `--name-only` or `--name-status`.\n \n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c7..e76c6e43e8 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -16,21 +16,38 @@\n \n static int line_termination = '\\n';\n #define LS_RECURSIVE 1\n-#define LS_TREE_ONLY 2\n-#define LS_SHOW_TREES 4\n-#define LS_NAME_ONLY 8\n-#define LS_SHOW_SIZE 16\n+#define LS_TREE_ONLY (1 << 1)\n+#define LS_SHOW_TREES (1 << 2)\n+#define LS_NAME_ONLY (1 << 3)\n+#define LS_SHOW_SIZE (1 << 4)\n+#define LS_OBJECT_ONLY (1 << 5)\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 unsigned int shown_bits;\n+#define SHOW_FILE_NAME 1\n+#define SHOW_SIZE (1 << 1)\n+#define SHOW_OBJECT_NAME (1 << 2)\n+#define SHOW_TYPE (1 << 3)\n+#define SHOW_MODE (1 << 4)\n+#define SHOW_DEFAULT 29 /* 11101 size is not shown to output by default */\n \n static const char * const ls_tree_usage[] = {\n \tN_(\"git ls-tree [] [...]\"),\n \tNULL\n };\n \n+enum {\n+\tMODE_UNSPECIFIED = 0,\n+\tMODE_NAME_ONLY,\n+\tMODE_OBJECT_ONLY,\n+\tMODE_LONG,\n+};\n+\n+static int cmdmode = MODE_UNSPECIFIED;\n+\n static int show_recursive(const char *base, int baselen, const char *pathname)\n {\n \tint i;\n@@ -66,6 +83,7 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n {\n \tint retval = 0;\n \tint baselen;\n+\tint interspace = 0;\n \tconst char *type = blob_type;\n \n \tif (S_ISGITLINK(mode)) {\n@@ -74,8 +92,8 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \t\t *\n \t\t * Something similar to this incomplete example:\n \t\t *\n-\t\tif (show_subprojects(base, baselen, pathname))\n-\t\t\tretval = READ_TREE_RECURSIVE;\n+\t\t * if (show_subprojects(base, baselen, pathname))\n+\t\t *\tretval = READ_TREE_RECURSIVE;\n \t\t *\n \t\t */\n \t\ttype = commit_type;\n@@ -90,35 +108,73 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif (!(ls_options & LS_NAME_ONLY)) {\n-\t\tif (ls_options & LS_SHOW_SIZE) {\n-\t\t\tchar size_text[24];\n-\t\t\tif (!strcmp(type, blob_type)) {\n-\t\t\t\tunsigned long size;\n-\t\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n-\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n-\t\t\t\t\t\t \"BAD\");\n-\t\t\t\telse\n-\t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n-\t\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n-\t\t\t} else\n-\t\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n-\t\t\tprintf(\"%06o %s %s %7s\\t\", mode, type,\n-\t\t\t find_unique_abbrev(oid, abbrev),\n-\t\t\t size_text);\n+\tif (shown_bits & SHOW_MODE) {\n+\t\tprintf(\"%06o\", mode);\n+\t\tinterspace = 1;\n+\t}\n+\tif (shown_bits & SHOW_TYPE) {\n+\t\tprintf(\"%s%s\", interspace ? \" \" : \"\", type);\n+\t\tinterspace = 1;\n+\t}\n+\tif (shown_bits & SHOW_OBJECT_NAME) {\n+\t\tprintf(\"%s%s\", interspace ? \" \" : \"\",\n+\t\t find_unique_abbrev(oid, abbrev));\n+\t\tif (!(shown_bits ^ SHOW_OBJECT_NAME))\n+\t\t\tgoto LINE_FINISH;\n+\t\tinterspace = 1;\n+\t}\n+\tif (shown_bits & SHOW_SIZE) {\n+\t\tchar size_text[24];\n+\t\tif (!strcmp(type, blob_type)) {\n+\t\t\tunsigned long size;\n+\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n+\t\t\t\txsnprintf(size_text, sizeof(size_text), \"BAD\");\n+\t\t\telse\n+\t\t\t\txsnprintf(size_text, sizeof(size_text),\n+\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n \t\t} else\n-\t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n-\t\t\t find_unique_abbrev(oid, abbrev));\n+\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n+\t\tprintf(\"%s%7s\", interspace ? \" \" : \"\", size_text);\n+\t\tinterspace = 1;\n+\t}\n+\tif (shown_bits & SHOW_FILE_NAME) {\n+\t\tif (interspace)\n+\t\t\tprintf(\"\\t\");\n+\t\tbaselen = base->len;\n+\t\tstrbuf_addstr(base, pathname);\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,\n+\t\t\t\t\t line_termination\n+\t\t\t\t\t ? CQ_NO_TERMINATOR_C_QUOTED\n+\t\t\t\t\t : CQ_NO_TERMINATOR_AS_IS);\n+\t\tstrbuf_setlen(base, baselen);\n \t}\n-\tbaselen = base->len;\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-\tstrbuf_setlen(base, baselen);\n+\n+LINE_FINISH:\n+\tputchar(line_termination);\n \treturn retval;\n }\n \n+static int parse_shown_fields(void)\n+{\n+\tif (cmdmode == MODE_NAME_ONLY) {\n+\t\tshown_bits = SHOW_FILE_NAME;\n+\t\treturn 0;\n+\t}\n+\tif (cmdmode == MODE_OBJECT_ONLY) {\n+\t\tshown_bits = SHOW_OBJECT_NAME;\n+\t\treturn 0;\n+\t}\n+\tif (!ls_options || (ls_options & LS_RECURSIVE)\n+\t || (ls_options & LS_SHOW_TREES)\n+\t || (ls_options & LS_TREE_ONLY))\n+\t\tshown_bits = SHOW_DEFAULT;\n+\tif (cmdmode == MODE_LONG)\n+\t\tshown_bits = SHOW_DEFAULT | SHOW_SIZE;\n+\treturn 1;\n+}\n+\n int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_id oid;\n@@ -133,12 +189,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\tLS_SHOW_TREES),\n \t\tOPT_SET_INT('z', NULL, &line_termination,\n \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n-\t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n-\t\t\tLS_SHOW_SIZE),\n-\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\n-\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n-\t\t\tLS_NAME_ONLY),\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+\t\t\t MODE_NAME_ONLY),\n+\t\tOPT_CMDMODE(0, \"name-status\", &cmdmode, N_(\"list only filenames\"),\n+\t\t\t MODE_NAME_ONLY),\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\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n@@ -169,6 +227,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tif (get_oid(argv[0], &oid))\n \t\tdie(\"Not a valid object name %s\", argv[0]);\n \n+\tparse_shown_fields();\n \t/*\n \t * show_recursive() rolls its own matching code and is\n \t * generally ignorant of 'struct pathspec'. The magic mask\ndiff --git a/quote.c b/quote.c\nindex 8a3a5e39eb..9f9da49fa2 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -340,12 +340,12 @@ void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path,\n \n void write_name_quoted(const char *name, FILE *fp, int terminator)\n {\n-\tif (terminator) {\n+\tif (0 < terminator || terminator == CQ_NO_TERMINATOR_C_QUOTED)\n \t\tquote_c_style(name, NULL, fp, 0);\n-\t} else {\n+\telse\n \t\tfputs(name, fp);\n-\t}\n-\tfputc(terminator, fp);\n+\tif (0 <= terminator)\n+\t\tfputc(terminator, fp);\n }\n \n void write_name_quoted_relative(const char *name, const char *prefix,\ndiff --git a/quote.h b/quote.h\nindex 049d8dd0b3..ec3f862545 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -84,8 +84,27 @@ int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n #define CQUOTE_NODQ 01\n size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned);\n void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned);\n+/*\n+ * Write a name, typically a filename, followed by a terminator that\n+ * separates it from what comes next.\n+ * When terminator is NUL, the name is given as-is. Otherwise, the\n+ * name is c-quoted, suitable for text output. HT and LF are typical\n+ * values used for the terminator, but other positive values are possible.\n+ *\n+ * In addition to non-negative values two special values in terminator\n+ * are possible.\n+ *\n+ * -1: show the name c-quoted, without adding any terminator.\n+ * -2: show the name as-is, without adding any terminator.\n+ */\n+#define CQ_NO_TERMINATOR_C_QUOTED\t(-1)\n+#define CQ_NO_TERMINATOR_AS_IS\t\t(-2)\n \n void write_name_quoted(const char *name, FILE *, int terminator);\n+/*\n+ * Similar to the above, but the name is first made relative to the prefix\n+ * before being shown.\n+ */\n void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator);\n \ndiff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh\nindex 14520913af..75e38b0a51 100755\n--- a/t/t3103-ls-tree-misc.sh\n+++ b/t/t3103-ls-tree-misc.sh\n@@ -22,4 +22,12 @@ test_expect_success 'ls-tree fails with non-zero exit code on broken tree' '\n \ttest_must_fail git ls-tree -r HEAD\n '\n \n+test_expect_success 'usage: incompatible options: --name-status with --long' '\n+\ttest_expect_code 129 git ls-tree --long --name-status\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-only with --long' '\n+\ttest_expect_code 129 git ls-tree --long --name-only\n+'\n+\n test_done\ndiff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\nnew file mode 100755\nindex 0000000000..81304e7b13\n--- /dev/null\n+++ b/t/t3104-ls-tree-oid.sh\n@@ -0,0 +1,51 @@\n+#!/bin/sh\n+\n+test_description='git ls-tree objects handling.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit A &&\n+\ttest_commit B &&\n+\tmkdir -p C &&\n+\ttest_commit C/D.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=$(git write-tree) &&\n+\techo $tree\n+'\n+\n+test_expect_success 'usage: --object-only' '\n+\tgit ls-tree --object-only $tree >current &&\n+\tgit ls-tree $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --object-only with -r' '\n+\tgit ls-tree --object-only -r $tree >current &&\n+\tgit ls-tree -r $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --object-only with --abbrev' '\n+\tgit ls-tree --object-only --abbrev=6 $tree >current &&\n+\tgit ls-tree --abbrev=6 $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-only with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --name-only\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-status with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --name-status\n+'\n+\n+test_expect_success 'usage: incompatible options: --long with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --long\n+'\n+\n+test_done\n-- \n2.33.1.10.g5f17c1a2c1.dirty\n\n"},{"id":"444349","messageId":"211217.86o85f8jey.gmgdl@evledraar.gmail.com","threadId":"56905","inReplyTo":"2e449d1c792ff81da5f22c8bf65ed33c393d62f8.1639721750.git.dyroneteng@gmail.com","subject":"Re: [PATCH v6 1/1] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:09:48Z","receivedAt":"2021-12-17T13:12:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 17 2021, Teng Long wrote:\n\n> [...]\n> int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct object_id oid;\n> @@ -133,12 +189,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n> \t\t\tLS_SHOW_TREES),\n> \t\tOPT_SET_INT('z', NULL, &line_termination,\n> \t\t\t N_(\"terminate entries with NUL byte\"), 0),\n> -\t\tOPT_BIT('l', \"long\", &ls_options, N_(\"include object size\"),\n> -\t\t\tLS_SHOW_SIZE),\n> -\t\tOPT_BIT(0, \"name-only\", &ls_options, N_(\"list only filenames\"),\n> -\t\t\tLS_NAME_ONLY),\n> -\t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n> -\t\t\tLS_NAME_ONLY),\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> +\t\t\t MODE_NAME_ONLY),\n> +\t\tOPT_CMDMODE(0, \"name-status\", &cmdmode, N_(\"list only filenames\"),\n> +\t\t\t MODE_NAME_ONLY),\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\t\t N_(\"use full path names\"), 0),\n> \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n\nVery nice to have the OPT_CMDMODE for asserting the usage, but this\nwould be even better if it were done as a separate commit. I.e. let's\nfirst do prep cleanups, then the new --object-name mode.\n\n> +test_expect_success 'usage: incompatible options: --name-status with --long' '\n> +\ttest_expect_code 129 git ls-tree --long --name-status\n> +'\n> +\n> +test_expect_success 'usage: incompatible options: --name-only with --long' '\n> +\ttest_expect_code 129 git ls-tree --long --name-only\n> +'\n> +\n> test_done\n> [...]\n> +test_expect_success 'usage: incompatible options: --name-only with --object-only' '\n> +\ttest_expect_code 129 git ls-tree --object-only --name-only\n> +'\n> +\n> +test_expect_success 'usage: incompatible options: --name-status with --object-only' '\n> +\ttest_expect_code 129 git ls-tree --object-only --name-status\n> +'\n> +\n> +test_expect_success 'usage: incompatible options: --long with --object-only' '\n> +\ttest_expect_code 129 git ls-tree --object-only --long\n> +'\n\nThese tests don't check for what you think they check, because you don't\nsupply a . So they're really just dying for the same reason a:\n\n git ls-tree\n\nWould.\n"},{"id":"444350","messageId":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"cover.1639721750.git.dyroneteng@gmail.com","subject":"[RFC PATCH 0/7] ls-tree --format","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:16Z","receivedAt":"2021-12-17T13:30:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Dec 17 2021, Teng Long wrote:\n\n> Many thanks to Junio and Ævar for your help and patient explanation.\n> I noticed Ævar suggest the solution with using `--format`, but in\n> this patch, the current approach continues. If this part of code needs\n> to be improved or we want to support \"--format\" in \"ls-tree\" in the\n> future, I'm more than glad to continue to contribute.\n\nFWIW here's the changes I had locally & cleaned up now that did the\nalternate --format approach.\n\nI think you'll probably want to steal some of this, e.g. you're\npatching the dead comment I removed in 1/6, 2-4/6 can be skipped, but\nI thought they were nice.\n\nBack when I last looked at this series, your --object-name patch was\nmuch shorter, but now it's about the same size as the generic --format\nsupport. So maybe it's worth considering implementing the more generic\npath.\n\nOne reason I didn't submit this before is that I couldn't get past the\nperformance regression this would inttroduce, i.e. if moved entirely\nto strbuf_expand(). Here though I'm keeping the old code, so it's no\nslower than \"master\", unlike your patch. But I haven't dug into why\nyours is slower:\n \n $ git hyperfine -L rev origin/master,tl/object-name,avar/ls-tree-format -s 'make CFLAGS=-O3' './git -C /run/user/1001/linux ls-tree -r HEAD' --warmup 10 -r 10\n Benchmark 1: ./git -C /run/user/1001/linux ls-tree -r HEAD' in 'origin/master\n Time (mean ± σ): 67.8 ms ± 0.3 ms [User: 48.8 ms, System: 18.9 ms]\n Range (min … max): 67.4 ms … 68.4 ms 10 runs\n \n Benchmark 2: ./git -C /run/user/1001/linux ls-tree -r HEAD' in 'tl/object-name\n Time (mean ± σ): 72.8 ms ± 0.4 ms [User: 50.6 ms, System: 22.1 ms]\n Range (min … max): 72.0 ms … 73.2 ms 10 runs\n \n Benchmark 3: ./git -C /run/user/1001/linux ls-tree -r HEAD' in 'avar/ls-tree-format\n Time (mean ± σ): 67.6 ms ± 0.4 ms [User: 50.5 ms, System: 17.0 ms]\n Range (min … max): 67.1 ms … 68.4 ms 10 runs\n \n Summary\n './git -C /run/user/1001/linux ls-tree -r HEAD' in 'avar/ls-tree-format' ran\n 1.00 ± 0.01 times faster than './git -C /run/user/1001/linux ls-tree -r HEAD' in 'origin/master'\n 1.08 ± 0.01 times faster than './git -C /run/user/1001/linux ls-tree -r HEAD' in 'tl/object-name'\n\nI then tacket a 6/6 at the end here to implement your --object-name in\nterms of --format (but didn't update the comimt message etc.). That's\nslower as expected:\n \n $ git hyperfine -L rev tl/object-name,avar/ls-tree-format -s 'make CFLAGS=-O3' './git -C /run/user/1001/linux ls-tree --object-only -r HEAD' --warmup 10 -r 10\n Benchmark 1: ./git -C /run/user/1001/linux ls-tree --object-only -r HEAD' in 'tl/object-name\n Time (mean ± σ): 58.7 ms ± 0.4 ms [User: 43.0 ms, System: 15.6 ms]\n Range (min … max): 58.4 ms … 59.6 ms 10 runs\n \n Benchmark 2: ./git -C /run/user/1001/linux ls-tree --object-only -r HEAD' in 'avar/ls-tree-format\n Time (mean ± σ): 65.6 ms ± 0.2 ms [User: 42.4 ms, System: 23.0 ms]\n Range (min … max): 65.1 ms … 65.9 ms 10 runs\n \n Summary\n './git -C /run/user/1001/linux ls-tree --object-only -r HEAD' in 'tl/object-name' ran\n 1.12 ± 0.01 times faster than './git -C /run/user/1001/linux ls-tree --object-only -r HEAD' in 'avar/ls-tree-format'\n\nBut it's not too bad, so maybe it's fine & worth making it more\ngeneric?\n\nAnyway. Just food for thought and and FYI in case you're\ninterested. Junio noted already that he'd like the --object-name\napproach first, so if you still want to pursue your current\nimplementation I don't mind.\n\nI do think you should be making performance testing a part of your\ntesting & cover letter writing though. A 8-10% slowdown isn't nothing,\nespecially for exactly the sort of plumbing command that'll likely to\nbe used to e.g. slurp up all paths in a very large repo.\n\nThese patches really aren't \"ready\". There's no docs, and as I noted\nin some earlier thread the tests for ls-tree are really\nlacking. E.g. I seem to have a rather obvious bug in how -t and the\n--format interact here, but no test catches it.\n\nWell, that one's me not having added a test, but I'm fairly sure there\nmight also be hidden bugs here due to lack of testing.\n\nTeng Long (1):\n ls-tree.c: support `--object-only` option for \"git-ls-tree\"\n\nÆvar Arnfjörð Bjarmason (6):\n ls-tree: remove commented-out code\n ls-tree: add missing braces to \"else\" arms\n ls-tree: use \"enum object_type\", not {blob,tree,commit}_type\n ls-tree: use \"size_t\", not \"int\" for \"struct strbuf\"'s \"len\"\n ls-tree: split up the \"init\" part of show_tree()\n ls-tree: add a --format= option\n\n Documentation/git-ls-tree.txt | 7 +-\n builtin/ls-tree.c | 226 ++++++++++++++++++++++++++++++----\n t/t3103-ls-tree-misc.sh | 8 ++\n t/t3104-ls-tree-oid.sh | 51 ++++++++\n t/t3105-ls-tree-format.sh | 49 ++++++++\n 5 files changed, 313 insertions(+), 28 deletions(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n create mode 100755 t/t3105-ls-tree-format.sh\n\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"},{"id":"444351","messageId":"RFC-patch-1.7-a4778c87c74-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","subject":"[RFC PATCH 1/7] ls-tree: remove commented-out code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:17Z","receivedAt":"2021-12-17T13:30:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove code added in f35a6d3bce7 (Teach core object handling functions\nabout gitlinks, 2007-04-09), later patched in 7d0b18a4da1 (Add output\nflushing before fork(), 2008-08-04), and then finally ending up in its\ncurrent form in d3bee161fef (tree.c: allow read_tree_recursive() to\ntraverse gitlink entries, 2009-01-25). All while being commented-out!\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-tree.c | 9 ---------\n 1 file changed, 9 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3a442631c71..5f7c84950ce 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -69,15 +69,6 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \tconst char *type = blob_type;\n \n \tif (S_ISGITLINK(mode)) {\n-\t\t/*\n-\t\t * Maybe we want to have some recursive version here?\n-\t\t *\n-\t\t * Something similar to this incomplete example:\n-\t\t *\n-\t\tif (show_subprojects(base, baselen, pathname))\n-\t\t\tretval = READ_TREE_RECURSIVE;\n-\t\t *\n-\t\t */\n \t\ttype = commit_type;\n \t} else if (S_ISDIR(mode)) {\n \t\tif (show_recursive(base->buf, base->len, pathname)) {\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"},{"id":"444352","messageId":"RFC-patch-2.7-8b34ee653ff-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","subject":"[RFC PATCH 2/7] ls-tree: add missing braces to \"else\" arms","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:18Z","receivedAt":"2021-12-17T13:30:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add missing {} to the \"else\" arms in show_tree() per the\nCodingGuidelines.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-tree.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 5f7c84950ce..0a28f32ccb9 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -92,14 +92,16 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \t\t\t\telse\n \t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n \t\t\t\t\t\t \"%\"PRIuMAX, (uintmax_t)size);\n-\t\t\t} else\n+\t\t\t} else {\n \t\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n+\t\t\t}\n \t\t\tprintf(\"%06o %s %s %7s\\t\", mode, type,\n \t\t\t find_unique_abbrev(oid, abbrev),\n \t\t\t size_text);\n-\t\t} else\n+\t\t} else {\n \t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n \t\t\t find_unique_abbrev(oid, abbrev));\n+\t\t}\n \t}\n \tbaselen = base->len;\n \tstrbuf_addstr(base, pathname);\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"},{"id":"444353","messageId":"RFC-patch-4.7-721f56ecdac-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","subject":"[RFC PATCH 4/7] ls-tree: use \"size_t\", not \"int\" for \"struct strbuf\"'s \"len\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:20Z","receivedAt":"2021-12-17T13:30:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"struct strbuf\"'s \"len\" member is a \"size_t\", not an \"int\", so\nlet's change our corresponding types accordingly. This also changes\nthe \"len\" and \"speclen\" variables, which are likewise used to store\nthe return value of strlen(), which returns \"size_t\", not \"int\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-tree.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 3f0225b097f..eecc7482d54 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -31,7 +31,7 @@ static const char * const ls_tree_usage[] = {\n \tNULL\n };\n \n-static int show_recursive(const char *base, int baselen, const char *pathname)\n+static int show_recursive(const char *base, size_t baselen, const char *pathname)\n {\n \tint i;\n \n@@ -43,7 +43,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n \n \tfor (i = 0; i < pathspec.nr; i++) {\n \t\tconst char *spec = pathspec.items[i].match;\n-\t\tint len, speclen;\n+\t\tsize_t len, speclen;\n \n \t\tif (strncmp(base, spec, baselen))\n \t\t\tcontinue;\n@@ -65,7 +65,7 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \t\tconst char *pathname, unsigned mode, void *context)\n {\n \tint retval = 0;\n-\tint baselen;\n+\tsize_t baselen;\n \tenum object_type type = OBJ_BLOB;\n \n \tif (S_ISGITLINK(mode)) {\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"},{"id":"444354","messageId":"RFC-patch-3.7-6219756cdd0-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","subject":"[RFC PATCH 3/7] ls-tree: use \"enum object_type\", not {blob,tree,commit}_type","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:19Z","receivedAt":"2021-12-17T13:30:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the ls-tree.c code to use type_name() on the enum instead of\nusing the string constants. This doesn't matter either way for\nperformance, but makes this a bit easier to read as we'll no longer\nneed a strcmp() here.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-tree.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 0a28f32ccb9..3f0225b097f 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -66,17 +66,17 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n {\n \tint retval = 0;\n \tint baselen;\n-\tconst char *type = blob_type;\n+\tenum object_type type = OBJ_BLOB;\n \n \tif (S_ISGITLINK(mode)) {\n-\t\ttype = commit_type;\n+\t\ttype = OBJ_COMMIT;\n \t} else if (S_ISDIR(mode)) {\n \t\tif (show_recursive(base->buf, base->len, pathname)) {\n \t\t\tretval = READ_TREE_RECURSIVE;\n \t\t\tif (!(ls_options & LS_SHOW_TREES))\n \t\t\t\treturn retval;\n \t\t}\n-\t\ttype = tree_type;\n+\t\ttype = OBJ_TREE;\n \t}\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n@@ -84,7 +84,7 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \tif (!(ls_options & LS_NAME_ONLY)) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n \t\t\tchar size_text[24];\n-\t\t\tif (!strcmp(type, blob_type)) {\n+\t\t\tif (type == OBJ_BLOB) {\n \t\t\t\tunsigned long size;\n \t\t\t\tif (oid_object_info(the_repository, oid, &size) == OBJ_BAD)\n \t\t\t\t\txsnprintf(size_text, sizeof(size_text),\n@@ -95,11 +95,11 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,\n \t\t\t} else {\n \t\t\t\txsnprintf(size_text, sizeof(size_text), \"-\");\n \t\t\t}\n-\t\t\tprintf(\"%06o %s %s %7s\\t\", mode, type,\n+\t\t\tprintf(\"%06o %s %s %7s\\t\", mode, type_name(type),\n \t\t\t find_unique_abbrev(oid, abbrev),\n \t\t\t size_text);\n \t\t} else {\n-\t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n+\t\t\tprintf(\"%06o %s %s\\t\", mode, type_name(type),\n \t\t\t find_unique_abbrev(oid, abbrev));\n \t\t}\n \t}\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"},{"id":"444355","messageId":"RFC-patch-6.7-eac299f06ff-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","subject":"[RFC PATCH 6/7] ls-tree: add a --format= option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:22Z","receivedAt":"2021-12-17T13:30:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a --format option to ls-tree. It has an existing default output,\nand then --long and --name-only options to emit the default output\nalong with the objectsize and, or to only emit object paths.\n\nRather than add --type-only, --object-only etc. we can just support a\n--format using a strbuf_expand() similar to \"for-each-ref\n--format\". We might still add such options in the future for\nconvenience.\n\nThe --format implementation is slower than the existing code, but this\nchange does not cause any performance regressions. We'll leave the\nexisting show_tree() unchanged, and only run show_tree_format() in if\na --format different than the hardcoded built-in ones corresponding to\nthe existing modes is provided.\n\n\"Slower\" here can bee seen via the the following \"hyperfine\"\ncommand. This uses GIT_TEST_LS_TREE_FORMAT_BACKEND= to force the\nuse of the new backend:\n\n $ hyperfine -L env false,true -L f \"-r,-r -l,-r --name-only,-r --format='%(objectname)'\" 'GIT_TEST_LS_TREE_FORMAT_BACKEND={env} ./git -C ~/g/linux ls-tree {f} HEAD' -r 10\n Benchmark 1: GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r HEAD\n Time (mean ± σ): 86.1 ms ± 0.6 ms [User: 65.2 ms, System: 20.9 ms]\n Range (min … max): 85.2 ms … 87.5 ms 10 runs\n\n Benchmark 2: GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r HEAD\n Time (mean ± σ): 122.5 ms ± 0.6 ms [User: 101.3 ms, System: 21.1 ms]\n Range (min … max): 121.8 ms … 123.4 ms 10 runs\n\n Benchmark 3: GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r -l HEAD\n Time (mean ± σ): 277.7 ms ± 1.3 ms [User: 234.6 ms, System: 43.0 ms]\n Range (min … max): 275.9 ms … 279.7 ms 10 runs\n\n Benchmark 4: GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r -l HEAD\n Time (mean ± σ): 332.8 ms ± 2.6 ms [User: 282.0 ms, System: 50.7 ms]\n Range (min … max): 329.6 ms … 338.2 ms 10 runs\n\n Benchmark 5: GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r --name-only HEAD\n Time (mean ± σ): 71.8 ms ± 0.4 ms [User: 54.1 ms, System: 17.6 ms]\n Range (min … max): 71.2 ms … 72.5 ms 10 runs\n\n Benchmark 6: GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r --name-only HEAD\n Time (mean ± σ): 86.6 ms ± 0.5 ms [User: 65.7 ms, System: 20.7 ms]\n Range (min … max): 85.9 ms … 87.4 ms 10 runs\n\n Benchmark 7: GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r --format='%(objectname)' HEAD\n Time (mean ± σ): 85.8 ms ± 0.6 ms [User: 66.2 ms, System: 19.5 ms]\n Range (min … max): 85.0 ms … 86.9 ms 10 runs\n\n Benchmark 8: GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r --format='%(objectname)' HEAD\n Time (mean ± σ): 85.3 ms ± 0.2 ms [User: 66.6 ms, System: 18.7 ms]\n Range (min … max): 85.0 ms … 85.7 ms 10 runs\n\n Summary\n 'GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r --name-only HEAD' ran\n 1.19 ± 0.01 times faster than 'GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r --format='%(objectname)' HEAD'\n 1.19 ± 0.01 times faster than 'GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r --format='%(objectname)' HEAD'\n 1.20 ± 0.01 times faster than 'GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r HEAD'\n 1.21 ± 0.01 times faster than 'GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r --name-only HEAD'\n 1.71 ± 0.01 times faster than 'GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r HEAD'\n 3.87 ± 0.03 times faster than 'GIT_TEST_LS_TREE_FORMAT_BACKEND=false ./git -C ~/g/linux ls-tree -r -l HEAD'\n 4.64 ± 0.05 times faster than 'GIT_TEST_LS_TREE_FORMAT_BACKEND=true ./git -C ~/g/linux ls-tree -r -l HEAD'\n\nI.e. something like the \"--long\" output would be much slower with\nthis, mainly due to how we need to allocate various things to do with\nquote.c instead of spewing the output directly to stdout.\n\nBut even a --format='%(objectname)' is fast with the new backend, so\nthis is viable as a replacement for adding new formats, and we'll pay\nfor this added complexity as a one-off, and not again every time a new\nformat needs to be added. See [1] for an example of what it would\notherwise take to add an --object-name flag.\n\n1. https://lore.kernel.org/git/2e449d1c792ff81da5f22c8bf65ed33c393d62f8.1639721750.git.dyroneteng@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-tree.c | 167 +++++++++++++++++++++++++++++++++++++-\n t/t3105-ls-tree-format.sh | 49 +++++++++++\n 2 files changed, 215 insertions(+), 1 deletion(-)\n create mode 100755 t/t3105-ls-tree-format.sh\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex df8312408da..efd85cab088 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -26,11 +26,34 @@ static struct pathspec pathspec;\n static int chomp_prefix;\n static const char *ls_tree_prefix;\n \n+/*\n+ * The format equivalents that show_tree() is prepared to handle.\n+ */\n+static const char *ls_tree_format_d = \"%(objectmode) %(objecttype) %(objectname)%x09%(path)\";\n+static const char *ls_tree_format_l = \"%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)\";\n+static const char *ls_tree_format_n = \"%(path)\";\n+\n static const char * const ls_tree_usage[] = {\n \tN_(\"git ls-tree [] [...]\"),\n \tNULL\n };\n \n+struct read_tree_ls_tree_data {\n+\tconst char *format;\n+\tstruct strbuf sb_scratch;\n+\tstruct strbuf sb_tmp;\n+};\n+\n+struct expand_ls_tree_data {\n+\tunsigned mode;\n+\tenum object_type type;\n+\tconst struct object_id *oid;\n+\tconst char *pathname;\n+\tconst char *basebuf;\n+\tstruct strbuf *sb_scratch;\n+\tstruct strbuf *sb_tmp;\n+};\n+\n static int show_recursive(const char *base, size_t baselen, const char *pathname)\n {\n \tint i;\n@@ -61,6 +84,76 @@ static int show_recursive(const char *base, size_t baselen, const char *pathname\n \treturn 0;\n }\n \n+static void expand_objectsize(struct strbuf *sb,\n+\t\t\t const struct object_id *oid,\n+\t\t\t const enum object_type type,\n+\t\t\t unsigned int padded)\n+{\n+\tif (type == OBJ_BLOB) {\n+\t\tunsigned long size;\n+\t\tif (oid_object_info(the_repository, oid, &size) < 0)\n+\t\t\tdie(_(\"could not get object info about '%s'\"), oid_to_hex(oid));\n+\t\tif (padded)\n+\t\t\tstrbuf_addf(sb, \"%7\"PRIuMAX, (uintmax_t)size);\n+\t\telse\n+\t\t\tstrbuf_addf(sb, \"%\"PRIuMAX, (uintmax_t)size);\n+\t} else if (padded) {\n+\t\tstrbuf_addf(sb, \"%7s\", \"-\");\n+\t} else {\n+\t\tstrbuf_addstr(sb, \"-\");\n+\t}\n+}\n+\n+static size_t expand_show_tree(struct strbuf *sb,\n+\t\t\t const char *start,\n+\t\t\t void *context)\n+{\n+\tstruct expand_ls_tree_data *data = context;\n+\tconst char *end;\n+\tconst char *p;\n+\tsize_t len;\n+\n+\tlen = strbuf_expand_literal_cb(sb, start, NULL);\n+\tif (len)\n+\t\treturn len;\n+\n+\tif (*start != '(')\n+\t\tdie(_(\"bad format as of '%s'\"), start);\n+\tend = strchr(start + 1, ')');\n+\tif (!end)\n+\t\tdie(_(\"ls-tree format element '%s' does not end in ')'\"),\n+\t\t start);\n+\tlen = end - start + 1;\n+\n+\tif (skip_prefix(start, \"(objectmode)\", &p)) {\n+\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n+\t} else if (skip_prefix(start, \"(objecttype)\", &p)) {\n+\t\tstrbuf_addstr(sb, type_name(data->type));\n+\t} else if (skip_prefix(start, \"(objectsize:padded)\", &p)) {\n+\t\texpand_objectsize(sb, data->oid, data->type, 1);\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_addstr(sb, find_unique_abbrev(data->oid, abbrev));\n+\t} else if (skip_prefix(start, \"(path)\", &p)) {\n+\t\tconst char *name = data->basebuf;\n+\t\tconst char *prefix = chomp_prefix ? ls_tree_prefix : NULL;\n+\n+\t\tif (prefix)\n+\t\t\tname = relative_path(name, prefix, data->sb_scratch);\n+\t\tquote_c_style(name, data->sb_tmp, NULL, 0);\n+\t\tstrbuf_add(sb, data->sb_tmp->buf, data->sb_tmp->len);\n+\n+\t\tstrbuf_reset(data->sb_tmp);\n+\t\t/* The relative_path() function resets \"scratch\" */\n+\t} else {\n+\t\tunsigned int errlen = (unsigned long)len;\n+\t\tdie(_(\"bad ls-tree format specifiec %%%.*s\"), errlen, start);\n+\t}\n+\n+\treturn len;\n+}\n+\n static int show_tree_init(enum object_type *type, struct strbuf *base,\n \t\t\t const char *pathname, unsigned mode, int *retval)\n {\n@@ -79,6 +172,38 @@ static int show_tree_init(enum object_type *type, struct strbuf *base,\n \treturn 0;\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)\n+{\n+\tstruct read_tree_ls_tree_data *data = context;\n+\tstruct expand_ls_tree_data my_data = {\n+\t\t.mode = mode,\n+\t\t.type = OBJ_BLOB,\n+\t\t.oid = oid,\n+\t\t.pathname = pathname,\n+\t\t.sb_scratch = &data->sb_scratch,\n+\t\t.sb_tmp = &data->sb_tmp,\n+\t};\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint retval = 0;\n+\tsize_t baselen;\n+\n+\tif (show_tree_init(&my_data.type, base, pathname, mode, &retval))\n+\t\treturn retval;\n+\n+\tbaselen = base->len;\n+\tstrbuf_addstr(base, pathname);\n+\tstrbuf_reset(&sb);\n+\tmy_data.basebuf = base->buf;\n+\n+\tstrbuf_expand(&sb, data->format, expand_show_tree, &my_data);\n+\tstrbuf_addch(&sb, line_termination);\n+\tfwrite(sb.buf, sb.len, 1, stdout);\n+\tstrbuf_setlen(base, baselen);\n+\n+\treturn retval;\n+}\n+\n static int show_tree(const struct object_id *oid, struct strbuf *base,\n \t\tconst char *pathname, unsigned mode, void *context)\n {\n@@ -125,6 +250,12 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tstruct object_id oid;\n \tstruct tree *tree;\n \tint i, full_tree = 0;\n+\tconst char *implicit_format = NULL;\n+\tconst char *format = NULL;\n+\tstruct read_tree_ls_tree_data read_tree_cb_data = {\n+\t\t.sb_scratch = STRBUF_INIT,\n+\t\t.sb_tmp = STRBUF_INIT,\n+\t};\n \tconst struct option ls_tree_options[] = {\n \t\tOPT_BIT('d', NULL, &ls_options, N_(\"only show trees\"),\n \t\t\tLS_TREE_ONLY),\n@@ -145,9 +276,12 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\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\t\t N_(\"format to use for the output\"), PARSE_OPT_NONEG),\n \t\tOPT__ABBREV(&abbrev),\n \t\tOPT_END()\n \t};\n+\tread_tree_fn_t fn = show_tree;\n \n \tgit_config(git_default_config, NULL);\n \tls_tree_prefix = prefix;\n@@ -164,6 +298,18 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tif ( (LS_TREE_ONLY|LS_RECURSIVE) ==\n \t ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options))\n \t\tls_options |= LS_SHOW_TREES;\n+\tif (ls_options & LS_NAME_ONLY)\n+\t\timplicit_format = ls_tree_format_n;\n+\tif (ls_options & LS_SHOW_SIZE)\n+\t\timplicit_format = ls_tree_format_l;\n+\n+\tif (format && implicit_format)\n+\t\tusage_msg_opt(_(\"providing --format cannot be combined with other format-altering options\"),\n+\t\t\t ls_tree_usage, ls_tree_options);\n+\tif (implicit_format)\n+\t\tformat = implicit_format;\n+\tif (!format)\n+\t\tformat = ls_tree_format_d;\n \n \tif (argc < 1)\n \t\tusage_with_options(ls_tree_usage, ls_tree_options);\n@@ -186,6 +332,25 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \ttree = parse_tree_indirect(&oid);\n \tif (!tree)\n \t\tdie(\"not a tree object\");\n+\n+\t/*\n+\t * The generic show_tree_fmt() is slower than show_tree(), so\n+\t * take the fast path if possible.\n+\t */\n+\tif (format && (!strcmp(format, ls_tree_format_d) ||\n+\t\t !strcmp(format, ls_tree_format_l) ||\n+\t\t !strcmp(format, ls_tree_format_n)))\n+\t\tfn = show_tree;\n+\telse if (format)\n+\t\tfn = show_tree_fmt;\n+\t/*\n+\t * Allow forcing the show_tree_fmt(), to test that it can\n+\t * handle the test suite.\n+\t */\n+\tif (git_env_bool(\"GIT_TEST_LS_TREE_FORMAT_BACKEND\", 0))\n+\t\tfn = show_tree_fmt;\n+\n+\tread_tree_cb_data.format = format;\n \treturn !!read_tree(the_repository, tree,\n-\t\t\t &pathspec, show_tree, NULL);\n+\t\t\t &pathspec, fn, &read_tree_cb_data);\n }\ndiff --git a/t/t3105-ls-tree-format.sh b/t/t3105-ls-tree-format.sh\nnew file mode 100755\nindex 00000000000..79817260ce8\n--- /dev/null\n+++ b/t/t3105-ls-tree-format.sh\n@@ -0,0 +1,49 @@\n+#!/bin/sh\n+\n+test_description='ls-tree --format'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+\n+test_expect_success 'ls-tree --format usage' '\n+\ttest_expect_code 129 git ls-tree --format=fmt -l &&\n+\ttest_expect_code 129 git ls-tree --format=fmt --name-only &&\n+\ttest_expect_code 129 git ls-tree --format=fmt --name-status\n+'\n+\n+test_expect_success 'setup' '\n+\tmkdir dir &&\n+\ttest_commit dir/sub-file &&\n+\ttest_commit top-file\n+'\n+\n+test_ls_tree_format () {\n+\tformat=$1 &&\n+\topts=$2 &&\t\t\n+\tshift 2 &&\n+\tgit ls-tree $opts -r HEAD >expect.raw &&\n+\tsed \"s/^/> /\" >expect $format\" -r HEAD >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'ls-tree --format=' '\n+\ttest_ls_tree_format \\\n+\t\t\"%(objectmode) %(objecttype) %(objectname)%x09%(path)\" \\\n+\t\t\"\"\n+'\n+\n+test_expect_success 'ls-tree --format=' '\n+\ttest_ls_tree_format \\\n+\t\t\"%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)\" \\\n+\t\t\"--long\"\n+'\n+\n+test_expect_success 'ls-tree --format=' '\n+\ttest_ls_tree_format \\\n+\t\t\"%(path)\" \\\n+\t\t\"--name-only\"\n+\n+'\n+\n+test_done\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"},{"id":"444356","messageId":"RFC-patch-5.7-8c9d8a8221d-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","subject":"[RFC PATCH 5/7] ls-tree: split up the \"init\" part of show_tree()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:21Z","receivedAt":"2021-12-17T13:30:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Split up the \"init\" part of the show_tree() function where we decide\nwhat the \"type\" is, and whether we'll return early. This makes things\na bit less readable for now, but we'll soon re-use this in a sibling\nfunction, and avoiding the duplication will be worth it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-tree.c | 30 +++++++++++++++++++-----------\n 1 file changed, 19 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex eecc7482d54..df8312408da 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -61,25 +61,33 @@ static int show_recursive(const char *base, size_t baselen, const char *pathname\n \treturn 0;\n }\n \n-static int show_tree(const struct object_id *oid, struct strbuf *base,\n-\t\tconst char *pathname, unsigned mode, void *context)\n+static int show_tree_init(enum object_type *type, struct strbuf *base,\n+\t\t\t const char *pathname, unsigned mode, int *retval)\n {\n-\tint retval = 0;\n-\tsize_t baselen;\n-\tenum object_type type = OBJ_BLOB;\n-\n \tif (S_ISGITLINK(mode)) {\n-\t\ttype = OBJ_COMMIT;\n+\t\t*type = OBJ_COMMIT;\n \t} else if (S_ISDIR(mode)) {\n \t\tif (show_recursive(base->buf, base->len, pathname)) {\n-\t\t\tretval = READ_TREE_RECURSIVE;\n+\t\t\t*retval = READ_TREE_RECURSIVE;\n \t\t\tif (!(ls_options & LS_SHOW_TREES))\n-\t\t\t\treturn retval;\n+\t\t\t\treturn 1;\n \t\t}\n-\t\ttype = OBJ_TREE;\n+\t\t*type = OBJ_TREE;\n \t}\n \telse if (ls_options & LS_TREE_ONLY)\n-\t\treturn 0;\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n+static int show_tree(const struct object_id *oid, struct strbuf *base,\n+\t\tconst char *pathname, unsigned mode, void *context)\n+{\n+\tint retval = 0;\n+\tsize_t baselen;\n+\tenum object_type type = OBJ_BLOB;\n+\n+\tif (show_tree_init(&type, base, pathname, mode, &retval))\n+\t\treturn retval;\n \n \tif (!(ls_options & LS_NAME_ONLY)) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"},{"id":"444357","messageId":"RFC-patch-7.7-5e34df4f8dd-20211217T131635Z-avarab@gmail.com","threadId":"56905","inReplyTo":"RFC-cover-0.7-00000000000-20211217T131635Z-avarab@gmail.com","subject":"[RFC PATCH 7/7] ls-tree.c: support `--object-only` option for \"git-ls-tree\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-17T13:30:23Z","receivedAt":"2021-12-17T13:30:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Teng Long \n\nWe usually pipe the output from `git ls-trees` to tools like\n`sed` or `cut` when we only want to extract some fields.\n\nWhen we want only the pathname component, we can pass\n`--name-only` option to omit such a pipeline, but there are no\noptions for extracting other fields.\n\nTeach the \"--object-only\" option to the command to only show the\nobject name. This option cannot be used together with\n\"--name-only\" or \"--long\" (mutually exclusive).\n\nSigned-off-by: Teng Long \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-ls-tree.txt | 7 ++++-\n builtin/ls-tree.c | 6 +++++\n t/t3103-ls-tree-misc.sh | 8 ++++++\n t/t3104-ls-tree-oid.sh | 51 +++++++++++++++++++++++++++++++++++\n 4 files changed, 71 insertions(+), 1 deletion(-)\n create mode 100755 t/t3104-ls-tree-oid.sh\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex db02d6d79a9..729370f2357 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] [--full-name] [--full-tree] [--abbrev[=]]\n+\t [--name-only] [--name-status] [--object-only] [--full-name] [--full-tree] [--abbrev[=]]\n \t [...]\n \n DESCRIPTION\n@@ -59,6 +59,11 @@ OPTIONS\n --name-only::\n --name-status::\n \tList only filenames (instead of the \"long\" output), one per line.\n+\tCannot be combined with `--object-only`.\n+\n+--object-only::\n+\tList only names of the objects, one per line. Cannot be combined\n+\twith `--name-only` or `--name-status`.\n \n --abbrev[=]::\n \tInstead of showing the full 40-byte hexadecimal object\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex efd85cab088..f19b0138362 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -20,6 +20,7 @@ static int line_termination = '\\n';\n #define LS_SHOW_TREES 4\n #define LS_NAME_ONLY 8\n #define LS_SHOW_SIZE 16\n+#define LS_OBJECT_ONLY 32\n static int abbrev;\n static int ls_options;\n static struct pathspec pathspec;\n@@ -31,6 +32,7 @@ static const char *ls_tree_prefix;\n */\n static const char *ls_tree_format_d = \"%(objectmode) %(objecttype) %(objectname)%x09%(path)\";\n static const char *ls_tree_format_l = \"%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)\";\n+static const char *ls_tree_format_o = \"%(objectname)\";\n static const char *ls_tree_format_n = \"%(path)\";\n \n static const char * const ls_tree_usage[] = {\n@@ -271,6 +273,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\tLS_NAME_ONLY),\n \t\tOPT_BIT(0, \"name-status\", &ls_options, N_(\"list only filenames\"),\n \t\t\tLS_NAME_ONLY),\n+\t\tOPT_BIT(0, \"object-only\", &ls_options, N_(\"list only objects\"),\n+\t\t\tLS_OBJECT_ONLY),\n \t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n@@ -302,6 +306,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\timplicit_format = ls_tree_format_n;\n \tif (ls_options & LS_SHOW_SIZE)\n \t\timplicit_format = ls_tree_format_l;\n+\tif (ls_options & LS_OBJECT_ONLY)\n+\t\timplicit_format = ls_tree_format_o;\n \n \tif (format && implicit_format)\n \t\tusage_msg_opt(_(\"providing --format cannot be combined with other format-altering options\"),\ndiff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh\nindex d18ba1bd84b..a8641706a6e 100755\n--- a/t/t3103-ls-tree-misc.sh\n+++ b/t/t3103-ls-tree-misc.sh\n@@ -23,4 +23,12 @@ test_expect_success 'ls-tree fails with non-zero exit code on broken tree' '\n \ttest_must_fail git ls-tree -r HEAD\n '\n \n+test_expect_success 'usage: incompatible options: --name-status with --long' '\n+\ttest_expect_code 129 git ls-tree --long --name-status\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-only with --long' '\n+\ttest_expect_code 129 git ls-tree --long --name-only\n+'\n+\n test_done\ndiff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh\nnew file mode 100755\nindex 00000000000..81304e7b13a\n--- /dev/null\n+++ b/t/t3104-ls-tree-oid.sh\n@@ -0,0 +1,51 @@\n+#!/bin/sh\n+\n+test_description='git ls-tree objects handling.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit A &&\n+\ttest_commit B &&\n+\tmkdir -p C &&\n+\ttest_commit C/D.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=$(git write-tree) &&\n+\techo $tree\n+'\n+\n+test_expect_success 'usage: --object-only' '\n+\tgit ls-tree --object-only $tree >current &&\n+\tgit ls-tree $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --object-only with -r' '\n+\tgit ls-tree --object-only -r $tree >current &&\n+\tgit ls-tree -r $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: --object-only with --abbrev' '\n+\tgit ls-tree --object-only --abbrev=6 $tree >current &&\n+\tgit ls-tree --abbrev=6 $tree >result &&\n+\tcut -f1 result | cut -d \" \" -f3 >expected &&\n+\ttest_cmp current expected\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-only with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --name-only\n+'\n+\n+test_expect_success 'usage: incompatible options: --name-status with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --name-status\n+'\n+\n+test_expect_success 'usage: incompatible options: --long with --object-only' '\n+\ttest_expect_code 129 git ls-tree --object-only --long\n+'\n+\n+test_done\n-- \n2.34.1.1119.g7a3fc8778ee\n\n"}]}