[PATCH 0/3] support `--oid-only` in `ls-tree`

Sometimes, we only want to get the objects from output of `ls-tree`
and commands like `sed` or `cut` is usually used to intercept the
origin output to achieve this purpose in practical.

The patch contains three commits

    1. Implementation of the option.
    2. Add new tests in "t3104".
    3. Documentation modifications.

I'm appreciate if someone help to review the patch.

Thanks.

Teng Long (3):
  ls-tree.c: support `--oid-only` option for "git-ls-tree"
  t3104: add related tests for `--oid-only` option
  git-ls-tree.txt: description of the 'oid-only' option

 Documentation/git-ls-tree.txt |  8 +++--
 builtin/ls-tree.c             | 11 +++++++
 t/t3104-ls-tree-oid.sh        | 55 +++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

-- 
2.33.1.9.g5fbd2fc599.dirty

This commit supply an option names `--oid-only` to let `git ls-tree`
only print out the OID of the object. `--oid-only` and `--name-only`
are mutually exclusive in use.

Signed-off-by: Teng Long <redacted>
---
 builtin/ls-tree.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3a442631c7..1f82229649 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -20,6 +20,7 @@ static int line_termination = '\n';
 #define LS_SHOW_TREES 4
 #define LS_NAME_ONLY 8
 #define LS_SHOW_SIZE 16
+#define LS_OID_ONLY 32
 static int abbrev;
 static int ls_options;
 static struct pathspec pathspec;

@@ -90,6 +91,14 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
+	if ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))
+		die(_("cannot specify --oid-only and --name-only at the same time"));
+
+	if (ls_options & LS_OID_ONLY) {
+		printf("%s\n", find_unique_abbrev(oid, abbrev));
+		return 0;
+	}
+
 	if (!(ls_options & LS_NAME_ONLY)) {
 		if (ls_options & LS_SHOW_SIZE) {
 			char size_text[24];

@@ -139,6 +148,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 			LS_NAME_ONLY),
 		OPT_BIT(0, "name-status", &ls_options, N_("list only filenames"),
 			LS_NAME_ONLY),
+		OPT_BIT(0, "oid-only", &ls_options, N_("list only oids"),
+			LS_OID_ONLY),
 		OPT_SET_INT(0, "full-name", &chomp_prefix,
 			    N_("use full path names"), 0),
 		OPT_BOOL(0, "full-tree", &full_tree,

-- 
2.33.1.9.g5fbd2fc599.dirty

Signed-off-by: Teng Long <redacted>
---
 t/t3104-ls-tree-oid.sh | 55 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100755 t/t3104-ls-tree-oid.sh

diff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh
new file mode 100755
index 0000000000..78ab9127c7
--- /dev/null
+++ b/t/t3104-ls-tree-oid.sh

@@ -0,0 +1,55 @@
+#!/bin/sh
+
+test_description='git ls-tree oids handling.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo 111 >1.txt &&
+	echo 222 >2.txt &&
+	mkdir -p path0/a/b/c &&
+	echo 333 >path0/a/b/c/3.txt &&
+	find *.txt path* \( -type f -o -type l \) -print |
+	xargs git update-index --add &&
+	tree=$(git write-tree) &&
+	echo $tree
+'
+
+
+test_expect_success 'specify with --oid-only' '
+	git ls-tree --oid-only $tree >current &&
+	cat >expected <<\EOF &&
+58c9bdf9d017fcd178dc8c073cbfcbb7ff240d6c
+c200906efd24ec5e783bee7f23b5d7c941b0c12c
+4e3849a078083863912298a25db30997cb8ca6d6
+EOF
+	test_cmp current expected
+'
+
+test_expect_success 'specify with --oid-only and -r' '
+	git ls-tree --oid-only -r $tree >current &&
+	cat >expected <<\EOF &&
+58c9bdf9d017fcd178dc8c073cbfcbb7ff240d6c
+c200906efd24ec5e783bee7f23b5d7c941b0c12c
+55bd0ac4c42e46cd751eb7405e12a35e61425550
+EOF
+	test_cmp current expected
+'
+
+test_expect_success 'specify with --oid-only and --abbrev' '
+	git ls-tree --oid-only --abbrev=6 $tree >current &&
+	cat >expected <<\EOF &&
+58c9bd
+c20090
+4e3849
+EOF
+	test_cmp current expected
+'
+
+test_expect_success 'cannot specify --name-only and --oid-only as the same time' '
+	test_must_fail git ls-tree --oid-only --name-only $tree >current 2>&1 >/dev/null &&
+	echo "fatal: cannot specify --oid-only and --name-only at the same time" > expected &&
+	test_cmp current expected
+'
+
+test_done

-- 
2.33.1.9.g5fbd2fc599.dirty

Signed-off-by: Teng Long <redacted>
---
 Documentation/git-ls-tree.txt | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index db02d6d79a..bc711dc00a 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt

@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git ls-tree' [-d] [-r] [-t] [-l] [-z]
-	    [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]]
+	    [--name-only] [--name-status] [--oid-only]
+	    [--full-name] [--full-tree] [--abbrev[=<n>]]
 	    <tree-ish> [<path>...]
 
 DESCRIPTION

@@ -59,7 +60,10 @@ OPTIONS
 --name-only::
 --name-status::
 	List only filenames (instead of the "long" output), one per line.
-
+	Cannot be used with `--oid-only` together.
+--oid-only::
+	List only OIDs of the objects, one per line. Cannot be used with
+	`--name-only` or `--name-status` together.
 --abbrev[=<n>]::
 	Instead of showing the full 40-byte hexadecimal object
 	lines, show the shortest prefix that is at least '<n>'

-- 
2.33.1.9.g5fbd2fc599.dirty

lorn Mon, Nov 15 2021, Teng Long wrote:

If you make these an OPT_CMDMODE you get this behavior for free. See
e.g. my
https://lore.kernel.org/git/patch-v2-06.10-d945fc94774-20211112T221506Z-avarab@gmail.com/

On Mon, Nov 15 2021, Teng Long wrote:

Sometimes, we only want to get the objects from output of `ls-tree`
and commands like `sed` or `cut` is usually used to intercept the
origin output to achieve this purpose in practical.

The patch contains three commits

    1. Implementation of the option.
    2. Add new tests in "t3104".
    3. Documentation modifications.

I'm appreciate if someone help to review the patch.

I've looked it over, they look correct mostly, the test code in 2/3
looks a bit too complex (using find?).

But I'd much rather see this be done with adding strbuf_expand() to
ls-tree. I.e. its docs say that it can emit:

    <mode> SP <type> SP <object> TAB <file>

Or, with -l:

    <mode> SP <type> SP <object> SP <object size> TAB <file>

If you use strbuf_expand() you can just define a default format of:

    %(objectmode) SP %(objecttype) SP %(objectname) TAB %(path)

Then make the existing -l option a shorthand for tweaking that to:

    %(objectmode) SP %(objecttype) SP %(objectsize) SP %(objectname) TAB %(path)

Then you can get what you want out of this with a simple:

    git ls-tree --format="%(objectname)"

See e.g. git-cat-file for an existing use of strbuf_expand().

On 2021-11-15 19:51:52+0800, Teng Long [off-list ref] wrote:

Failed with:

	GIT_TEST_DEFAULT_HASH=sha256 ./t3104-ls-tree-oid.sh

I think we can use:

	git ls-tree $tree | awk '{print $3}' >expected

+	test_cmp current expected
+'
+
+test_expect_success 'specify with --oid-only and -r' '
+	git ls-tree --oid-only -r $tree >current &&
+	cat >expected <<\EOF &&
+58c9bdf9d017fcd178dc8c073cbfcbb7ff240d6c
+c200906efd24ec5e783bee7f23b5d7c941b0c12c
+55bd0ac4c42e46cd751eb7405e12a35e61425550
+EOF
+	test_cmp current expected
+'

Ditto for this test and below tests.

+
+test_expect_success 'specify with --oid-only and --abbrev' '
+	git ls-tree --oid-only --abbrev=6 $tree >current &&
+	cat >expected <<\EOF &&
+58c9bd
+c20090
+4e3849
+EOF
+	test_cmp current expected
+'
+
+test_expect_success 'cannot specify --name-only and --oid-only as the same time' '
+	test_must_fail git ls-tree --oid-only --name-only $tree >current 2>&1 >/dev/null &&

The last redirection '>/dev/null' does nothing, me think.

+	echo "fatal: cannot specify --oid-only and --name-only at the same time" > expected &&

Style nit:

	use '>expected' instead of '> expected'

+	test_cmp current expected
+'
+
+test_done
-- 
2.33.1.9.g5fbd2fc599.dirty

-- 
Danh

On Mon, Nov 15, 2021 at 07:51:51PM +0800, Teng Long wrote:

This seems reasonable to me. Letting them overwrite each other (i.e.,
"last one wins") would also be fine, but we can always loosen to that
behavior later if we choose.

This is a somewhat funny place to put the check, though. It will be run
for every entry in the tree (so is a tiny bit less efficient, but also
would not trigger for an empty tree). It probably should go in
cmd_ls_tree(), perhaps here:

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 1f82229649..3c9ea00489 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -91,9 +91,6 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
-	if ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))
-		die(_("cannot specify --oid-only and --name-only at the same time"));
-
 	if (ls_options & LS_OID_ONLY) {
 		printf("%s\n", find_unique_abbrev(oid, abbrev));
 		return 0;

@@ -175,6 +172,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	    ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options))
 		ls_options |= LS_SHOW_TREES;
 
+	if ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))
+		die(_("cannot specify --oid-only and --name-only at the same time"));
+
 	if (argc < 1)
 		usage_with_options(ls_tree_usage, ls_tree_options);
 	if (get_oid(argv[0], &oid))

Ævar also mentioned using OPT_CMDMODE(), which I think would naturally
move the logic in a similar way.

-Peff

On Mon, Nov 15, 2021 at 07:51:50PM +0800, Teng Long wrote:

Sometimes, we only want to get the objects from output of `ls-tree`
and commands like `sed` or `cut` is usually used to intercept the
origin output to achieve this purpose in practical.

The patch contains three commits

    1. Implementation of the option.
    2. Add new tests in "t3104".
    3. Documentation modifications.

I'm appreciate if someone help to review the patch.

This seems like a good feature to have. I think it would make sense to
squash the three patches into a single one. The documentation and test
patches do not stand on their own, which is why there was nothing useful
to say in their commit messages.

The implementation looks generally sensible (modulo the comments already
given). I was surprised that there was not an existing ls-tree script
that these would fit into. But there really isn't; t3101 covers
--name-only and other output, but is really focused on the pathnames
(though I think it would be OK to refactor it to cover output more
generally).

-Peff

On Mon, Nov 15 2021, Jeff King wrote:

On Mon, Nov 15, 2021 at 04:13:24PM +0100, Ævar Arnfjörð Bjarmason wrote:

quoted

But I'd much rather see this be done with adding strbuf_expand() to
ls-tree. I.e. its docs say that it can emit:

I had a similar thought, but that's a much bigger task. I think it would
be reasonable to add --oid-only to match the existing --name-only, etc.
If we later add a custom --format option, then it can easily be folded
in and explained as "this is an alias for --format=%(objectname)", just
like --name-only would become "this is an alias for --format=%(path)".

A quick patch to do it below, seems to work, passes all tests, but I
don't know how much I'd trust it. It's also quite an add use of
strbuf_expa(). We print to stdout directly since
write_name_quoted_relative() really wants to write to stdout, and not
give you a buffer. But I guess it makes sense in a way.

The hardcoded %7s for %(objectsize) is a bit nasty, but I don't know if
we've got anything existing that handles format specifiers with
strbuf_expand() that we could steal.

I really wouldn't trust this code much, I found it when writing it that
our tests for ls-tree are really lacking, e.g. we may not have a single
test for "-l" anywhere (or maybe I didn't look enough, I was just
running t/*ls*tree* while hacking it.

I do thin that we should consider just going with --format in either
case if we agree that this is a good direction. I.e. could just support
3-4 hardcoded formats now and die if anything else is specified.

Then we'd be future-proof with the same interface expanding later, and
wouldn't need to support options that we're only carrying because we
didn't implement the more generic format support.

(Assume my Signed-off-by, if there's any interest...)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3a442631c71..e89daad4229 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -31,6 +31,20 @@ static const  char * const ls_tree_usage[] = {
 	NULL
 };
 
+static const char *ls_tree_format_d = "%(objectmode) %(objecttype) %(objectname)	%(path)";
+static const char *ls_tree_format_l = "%(objectmode) %(objecttype) %(objectname) %(objectsize)	%(path)";
+static const char *ls_tree_format_n = "%(path)";
+
+struct expand_ls_tree_data {
+	const char *format;
+	unsigned mode;
+	const char *type;
+	const struct object_id *oid;
+	int abbrev;
+	const char *pathname;
+	const char *basebuf;
+};
+
 static int show_recursive(const char *base, int baselen, const char *pathname)
 {
 	int i;

@@ -61,9 +75,69 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
 	return 0;
 }
 
+static size_t expand_show_tree(struct strbuf *sb,
+			       const char *start,
+			       void *context)
+{
+	struct expand_ls_tree_data *data = context;
+	const char *end;
+	const char *p;
+	size_t len;
+	const char *type = blob_type;
+
+	if (sb->len) {
+		fputs(sb->buf, stdout);
+		strbuf_reset(sb);
+	}
+
+	if (*start != '(')
+		die(_("bad format as of '%s'"), start);
+	end = strchr(start + 1, ')');
+	if (!end)
+		die(_("ls-tree format element '%s' does not end in ')'"),
+		    start);
+	len = end - start + 1;
+
+	if (skip_prefix(start, "(objectmode)", &p)) {
+		printf("%06o", data->mode);
+	} else if (skip_prefix(start, "(objecttype)", &p)) {
+		fputs(data->type, stdout);
+	} else if (skip_prefix(start, "(objectsize)", &p)) {
+		char size_text[24];
+		const struct object_id *oid = data->oid;
+
+		if (!strcmp(type, blob_type)) {
+			unsigned long size;
+			if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
+				xsnprintf(size_text, sizeof(size_text),
+					  "BAD");
+			else
+				xsnprintf(size_text, sizeof(size_text),
+					  "%"PRIuMAX, (uintmax_t)size);
+		} else {
+			xsnprintf(size_text, sizeof(size_text), "-");
+		}
+		printf("%7s", size_text);
+	} else if (skip_prefix(start, "(objectname)", &p)) {
+		fputs(find_unique_abbrev(data->oid, data->abbrev), stdout);
+	} else if (skip_prefix(start, "(path)", &p)) {
+		write_name_quoted_relative(data->basebuf,
+					   chomp_prefix ? ls_tree_prefix : NULL,
+					   stdout, line_termination);
+
+	} else {
+		unsigned int errlen = (unsigned long)len;
+		die(_("bad ls-tree format specifiec %%%.*s"), errlen, start);	
+	}
+
+	return len;
+}
+
 static int show_tree(const struct object_id *oid, struct strbuf *base,
 		const char *pathname, unsigned mode, void *context)
 {
+	struct expand_ls_tree_data *data = context;
+	struct strbuf sb = STRBUF_INIT;
 	int retval = 0;
 	int baselen;
 	const char *type = blob_type;

@@ -90,31 +164,18 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
-	if (!(ls_options & LS_NAME_ONLY)) {
-		if (ls_options & LS_SHOW_SIZE) {
-			char size_text[24];
-			if (!strcmp(type, blob_type)) {
-				unsigned long size;
-				if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
-					xsnprintf(size_text, sizeof(size_text),
-						  "BAD");
-				else
-					xsnprintf(size_text, sizeof(size_text),
-						  "%"PRIuMAX, (uintmax_t)size);
-			} else
-				xsnprintf(size_text, sizeof(size_text), "-");
-			printf("%06o %s %s %7s\t", mode, type,
-			       find_unique_abbrev(oid, abbrev),
-			       size_text);
-		} else
-			printf("%06o %s %s\t", mode, type,
-			       find_unique_abbrev(oid, abbrev));
-	}
 	baselen = base->len;
 	strbuf_addstr(base, pathname);
-	write_name_quoted_relative(base->buf,
-				   chomp_prefix ? ls_tree_prefix : NULL,
-				   stdout, line_termination);
+
+	strbuf_reset(&sb);
+	data->mode = mode;
+	data->type = type;
+	data->oid = oid;
+	data->abbrev = abbrev;
+	data->pathname = pathname;
+	data->basebuf = base->buf;
+	strbuf_expand(&sb, data->format, expand_show_tree, data);
+
 	strbuf_setlen(base, baselen);
 	return retval;
 }

@@ -147,6 +208,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};
+	struct expand_ls_tree_data ls_tree_cb_data = {
+		.format = ls_tree_format_d,
+	};
 
 	git_config(git_default_config, NULL);
 	ls_tree_prefix = prefix;

@@ -161,8 +225,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	}
 	/* -d -r should imply -t, but -d by itself should not have to. */
 	if ( (LS_TREE_ONLY|LS_RECURSIVE) ==
-	    ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options))
+	    ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options)) {
 		ls_options |= LS_SHOW_TREES;
+	}
+	if (ls_options & LS_NAME_ONLY)
+		ls_tree_cb_data.format = ls_tree_format_n;
+
+	if (ls_options & LS_SHOW_SIZE)
+		ls_tree_cb_data.format = ls_tree_format_l;
 
 	if (argc < 1)
 		usage_with_options(ls_tree_usage, ls_tree_options);

@@ -185,6 +255,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	tree = parse_tree_indirect(&oid);
 	if (!tree)
 		die("not a tree object");
+
 	return !!read_tree(the_repository, tree,
-			   &pathspec, show_tree, NULL);
+			   &pathspec, show_tree, &ls_tree_cb_data);
 }

On Mon, Nov 15, 2021 at 02:16:27PM -0500, Jeff King wrote:

On Mon, Nov 15, 2021 at 07:51:51PM +0800, Teng Long wrote:

quoted

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3a442631c7..1f82229649 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -20,6 +20,7 @@ static int line_termination = '\n';
 #define LS_SHOW_TREES 4
 #define LS_NAME_ONLY 8
 #define LS_SHOW_SIZE 16
+#define LS_OID_ONLY 32
 static int abbrev;
 static int ls_options;
 static struct pathspec pathspec;

@@ -90,6 +91,14 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
+	if ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))
+		die(_("cannot specify --oid-only and --name-only at the same time"));

This seems reasonable to me. Letting them overwrite each other (i.e.,
"last one wins") would also be fine, but we can always loosen to that
behavior later if we choose.

Oh, and whichever direction we go, it would probably make sense for
--long to be handled in the same way. I.e.:

  git ls-tree --long --oid-only

does not really make sense. Though we currently just ignore --long for:

  git ls-tree --long --name-only

which is arguably a bug.

-Peff

On Mon, Nov 15, 2021 at 04:13:24PM +0100, Ævar Arnfjörð Bjarmason wrote:

But I'd much rather see this be done with adding strbuf_expand() to
ls-tree. I.e. its docs say that it can emit:

I had a similar thought, but that's a much bigger task. I think it would
be reasonable to add --oid-only to match the existing --name-only, etc.
If we later add a custom --format option, then it can easily be folded
in and explained as "this is an alias for --format=%(objectname)", just
like --name-only would become "this is an alias for --format=%(path)".

-Peff

on Mon, 15 Nov 2021 22:54:00 +0700, Đoàn Trần Công Danh wrote:

Thanks for helping to review this patch.

Failed with:

        GIT_TEST_DEFAULT_HASH=sha256 ./t3104-ls-tree-oid.sh

You totally right and the tests should pass both sha1 and sha256.

I think we can use:

        git ls-tree $tree | awk '{print $3}' >expected
...
...

Ditto for this test and below tests.

Yes, correct and better.

But should also escape the dollar character to work.

The last redirection '>/dev/null' does nothing, me think.
Style nit:

use '>expected' instead of '> expected'

Yeah, that's my bad and will fix.

If you make these an OPT_CMDMODE you get this behavior for free. See
e.g. my
https://lore.kernel.org/git/patch-v2-06.10-d945fc94774-20211112T221506Z-avarab@gmail.com/	   

Thank you very much for providing this input.

So I try to read this patch your mentioned and try to repeat the idea in my understanding.

First, OPT_CMDMODE() can be used for:

       1. Easy for checking the combined command options, such as "mutually exclusive" conditions.

       2. Die and output the error message consistently when the incompatible options are found.

       3. Brings better extensibilites, no need to change a lot of if/elses.

Then, you suggest to consider about to use OPT_CMDMODE instead of the current implementations.

Did I understand your suggestion right and comprehensive?

On Thu, Nov 18 2021, Teng Long wrote:

quoted

If you make these an OPT_CMDMODE you get this behavior for free. See
e.g. my
https://lore.kernel.org/git/patch-v2-06.10-d945fc94774-20211112T221506Z-avarab@gmail.com/	   

Thank you very much for providing this input.

So I try to read this patch your mentioned and try to repeat the idea in my understanding.

First, OPT_CMDMODE() can be used for:

       1. Easy for checking the combined command options, such as "mutually exclusive" conditions.

       2. Die and output the error message consistently when the incompatible options are found.

       3. Brings better extensibilites, no need to change a lot of if/elses.

Then, you suggest to consider about to use OPT_CMDMODE instead of the current implementations.

Did I understand your suggestion right and comprehensive?

Yes, all of that is correct.

It's a way of defining N options, --foo, --bar, --baz, where combining
any of them is an error.

We usually use it for a "command mode" (hence the name), but it can be
used when the command has flags that are mutually exclusive.

I think (but am not sure, and didn't check) that you can even use it for
--foo AND --bar that are exclusive, and --other --flags that are also
mutually exclusive (but could be combined with one of --foo or --bar),
you just need to provide another variable for it to set.

But I haven't tested that or used it like that, maybe it doesn't work
for some reason I'm forgetting...

On Mon, 15 Nov 2021 14:16:27 -0500, Jeff King wrote:

This is a somewhat funny place to put the check, though. It will be run
for every entry in the tree (so is a tiny bit less efficient, but also
would not trigger for an empty tree). It probably should go in
cmd_ls_tree(), perhaps here:

Yes, it's better here as a fail-fast case.

According to the suggestion of the new location I think why not put the logic
further head, after the parse_options() return, like:

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 1f82229649..003a9ade54 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -166,6 +166,10 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 
        argc = parse_options(argc, argv, prefix, ls_tree_options,
                             ls_tree_usage, 0);
+
+       if ((ls_options & LS_NAME_ONLY) && (ls_options & LS_OID_ONLY))
+               die(_("cannot specify --oid-only and --name-only at the same time"));
+
        if (full_tree) {
                ls_tree_prefix = prefix = NULL;
                chomp_prefix = 0;

Will it bring other new problems?

Thank you.

A quick patch to do it below, seems to work, passes all tests, but I
don't know how much I'd trust it. It's also quite an add use of
strbuf_expa(). We print to stdout directly since
write_name_quoted_relative() really wants to write to stdout, and not
give you a buffer. But I guess it makes sense in a way.

Thanks for the patch and the inputs about "strbuf_expa()".

Then we'd be future-proof with the same interface expanding later, and
wouldn't need to support options that we're only carrying because we
didn't implement the more generic format support.

I agree but like Peff said it maybe another bigger task. I think I will
firstly solve the existing problems in next patch.

I will consider about the generic format support but not sure whether
it will continue to iterate in this patchset.

(Assume my Signed-off-by, if there's any interest...)

Of course I will.

Thank you very much for your advice and guidance again.

This patch series supports for only outputing the "objects" (OID)
with a new option names `--oid-only`.

Changes with the first patch are :

        1. Three commits are squashed to 1 commit (Peff's advice)
        2. The tests issues (Đoàn Trần Công Danh's advice)
        3. Use `OPT_CMDMODE()` for mutually exclusive control
           (Ævar Arnfjörð Bjarmason's advice)

Some discussions are not included in Patch 2 :

        1. `git ls-tree --long --name-only` and
           `git ls-tree --long --oid-only` which is arguably a bug
           (Peff's advice)
        2. Support `--format` for `git-ls-tree`
           (Ævar Arnfjörð Bjarmason's advice)

The reason why these 2 discussions not included is I'm not sure whether
I should continue on the current patchset or start a new one. And for the
second, I think current implementation is clear and simple to use, meeting
the needs of the moment. Maybe I will to support `--format` option, but
before that, I'm appreciate if there are more suggestions appear.

Thanks.

Teng Long (1):
  ls-tree.c: support `--oid-only` option for "git-ls-tree"

 Documentation/git-ls-tree.txt |  8 +++++--
 builtin/ls-tree.c             | 27 ++++++++++++++++-------
 t/t3104-ls-tree-oid.sh        | 40 +++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

Range-diff against v1:
1:  c4479178d7 < -:  ---------- ls-tree.c: support `--oid-only` option for "git-ls-tree"
2:  853ebbcf88 < -:  ---------- t3104: add related tests for `--oid-only` option
3:  33c68c1f11 < -:  ---------- git-ls-tree.txt: description of the 'oid-only' option
-:  ---------- > 1:  8b68568d6c ls-tree.c: support `--oid-only` option for "git-ls-tree"
-- 
2.33.1.10.g1f74a882e4

Sometimes, we only want to get the objects from output of `ls-tree`
and commands like `sed` or `cut` is usually used to intercept the
origin output to achieve this purpose in practical.

This commit supply an option names `--oid-only` to let `git ls-tree`
only print out the OID of the object. `--oid-only` and `--name-only`
are mutually exclusive in use.

Reviewed-by: Jeff King <redacted>
Reviewed-by: Ævar Arnfjörð Bjarmason <redacted>
Reviewed-by: Đoàn Trần Công Danh <redacted>
Signed-off-by: Teng Long <redacted>
---
 Documentation/git-ls-tree.txt |  8 +++++--
 builtin/ls-tree.c             | 27 ++++++++++++++++-------
 t/t3104-ls-tree-oid.sh        | 40 +++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index db02d6d79a..bc711dc00a 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt

@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git ls-tree' [-d] [-r] [-t] [-l] [-z]
-	    [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]]
+	    [--name-only] [--name-status] [--oid-only]
+	    [--full-name] [--full-tree] [--abbrev[=<n>]]
 	    <tree-ish> [<path>...]
 
 DESCRIPTION

@@ -59,7 +60,10 @@ OPTIONS
 --name-only::
 --name-status::
 	List only filenames (instead of the "long" output), one per line.
-
+	Cannot be used with `--oid-only` together.
+--oid-only::
+	List only OIDs of the objects, one per line. Cannot be used with
+	`--name-only` or `--name-status` together.
 --abbrev[=<n>]::
 	Instead of showing the full 40-byte hexadecimal object
 	lines, show the shortest prefix that is at least '<n>'

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3a442631c7..1e4a82e669 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -18,19 +18,26 @@ static int line_termination = '\n';
 #define LS_RECURSIVE 1
 #define LS_TREE_ONLY 2
 #define LS_SHOW_TREES 4
-#define LS_NAME_ONLY 8
-#define LS_SHOW_SIZE 16
+#define LS_SHOW_SIZE 8
 static int abbrev;
 static int ls_options;
 static struct pathspec pathspec;
 static int chomp_prefix;
 static const char *ls_tree_prefix;
 
-static const  char * const ls_tree_usage[] = {
+static const char * const ls_tree_usage[] = {
 	N_("git ls-tree [<options>] <tree-ish> [<path>...]"),
 	NULL
 };
 
+enum {
+	MODE_UNSPECIFIED = 0,
+	MODE_NAME_ONLY,
+	MODE_OID_ONLY
+};
+
+static int cmdmode = MODE_UNSPECIFIED;
+
 static int show_recursive(const char *base, int baselen, const char *pathname)
 {
 	int i;

@@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
-	if (!(ls_options & LS_NAME_ONLY)) {
+	if (cmdmode == 2) {
+		printf("%s\n", find_unique_abbrev(oid, abbrev));
+		return 0;
+	}
+
+	if (cmdmode == 0) {
 		if (ls_options & LS_SHOW_SIZE) {
 			char size_text[24];
 			if (!strcmp(type, blob_type)) {

@@ -135,10 +147,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 			    N_("terminate entries with NUL byte"), 0),
 		OPT_BIT('l', "long", &ls_options, N_("include object size"),
 			LS_SHOW_SIZE),
-		OPT_BIT(0, "name-only", &ls_options, N_("list only filenames"),
-			LS_NAME_ONLY),
-		OPT_BIT(0, "name-status", &ls_options, N_("list only filenames"),
-			LS_NAME_ONLY),
+		OPT_CMDMODE('n', "name-only", &cmdmode, N_("list only filenames"), MODE_NAME_ONLY),
+		OPT_CMDMODE('s', "name-status", &cmdmode, N_("list only filenames"), MODE_NAME_ONLY),
+		OPT_CMDMODE('o', "oid-only", &cmdmode, N_("list only oids"), MODE_OID_ONLY),
 		OPT_SET_INT(0, "full-name", &chomp_prefix,
 			    N_("use full path names"), 0),
 		OPT_BOOL(0, "full-tree", &full_tree,

diff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh
new file mode 100755
index 0000000000..4c02cdd3c3
--- /dev/null
+++ b/t/t3104-ls-tree-oid.sh

@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='git ls-tree oids handling.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo 111 >1.txt &&
+	echo 222 >2.txt &&
+	mkdir -p path0/a/b/c &&
+	echo 333 >path0/a/b/c/3.txt &&
+	find *.txt path* \( -type f -o -type l \) -print |
+	xargs git update-index --add &&
+	tree=$(git write-tree) &&
+	echo $tree
+'
+
+test_expect_success 'usage: --oid-only' '
+	git ls-tree --oid-only $tree >current &&
+	git ls-tree $tree | awk "{print \$3}" >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with -r' '
+	git ls-tree --oid-only -r $tree >current &&
+	git ls-tree -r $tree | awk "{print \$3}" >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with --abbrev' '
+	git ls-tree --oid-only --abbrev=6 $tree >current &&
+	git ls-tree --abbrev=6 $tree | awk "{print \$3}" > expected &&
+	test_cmp current expected
+'
+
+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '
+	test_incompatible_usage git ls-tree --oid-only --name-only
+'
+
+test_done

-- 
2.33.1.10.g1f74a882e4

On Fri, Nov 19 2021, Teng Long wrote:

Reviewed-by: Jeff King <redacted>
Reviewed-by: Ævar Arnfjörð Bjarmason <redacted>
Reviewed-by: Đoàn Trần Công Danh <redacted>

Please don't add the Reviewed-by headers yourself, either Junio
accumulates them, or if someone explicitly mentions that you can add it
with their name it's OK.

It doesn't just mean this person reviewed this series in some ML thread,
but "this person is 100% OK with this in its current form".

 	List only filenames (instead of the "long" output), one per line.
-
+	Cannot be used with `--oid-only` together.

Better: "Cannot be combined with OPT."

+--oid-only::
+	List only OIDs of the objects, one per line. Cannot be used with
+	`--name-only` or `--name-status` together.

Better: "Cannot be combined with OPT or OPT2."

+enum {
+	MODE_UNSPECIFIED = 0,
+	MODE_NAME_ONLY,
+	MODE_OID_ONLY
+};
+
+static int cmdmode = MODE_UNSPECIFIED;

Better:

static enum {
	MODE_NAME_ONLY = 1,
        ...
} cmdmode = MODE_NAME_ONLY;

I.e. no need for the MODE_UNSPECIFIED just to skip past "0".

Better to preserve the wrapping here, to stay within 79 columns.

+test_expect_success 'setup' '
+	echo 111 >1.txt &&
+	echo 222 >2.txt &&

Just use:

    test_commit A &&
    test_commit B

etc?

+	mkdir -p path0/a/b/c &&
+	echo 333 >path0/a/b/c/3.txt &&
+	find *.txt path* \( -type f -o -type l \) -print |
+	xargs git update-index --add &&
+	tree=$(git write-tree) &&
+	echo $tree

Stray echo? Unclear why this test setup is so complex, shouldn't this just be (continued from above):

    mkdir -p C &&
    test_commit C/D.txt

To test nested dirs?

+'
+
+test_expect_success 'usage: --oid-only' '
+	git ls-tree --oid-only $tree >current &&
+	git ls-tree $tree | awk "{print \$3}" >expected &&

just cut -f1 instead of awk? Also don't put "git" on the LHS of a pipe,
it might hide segfaults. Also applies to the below.

+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with -r' '
+	git ls-tree --oid-only -r $tree >current &&
+	git ls-tree -r $tree | awk "{print \$3}" >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with --abbrev' '
+	git ls-tree --oid-only --abbrev=6 $tree >current &&
+	git ls-tree --abbrev=6 $tree | awk "{print \$3}" > expected &&
+	test_cmp current expected
+'
+
+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '
+	test_incompatible_usage git ls-tree --oid-only --name-only
+'

Hrm, did you copy this use of test_incompatible_usage from
t1006-cat-file.sh without providing the function?

More data for:
https://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/ :)

Better to use:

    test_expect_code 128 ... # (or was it 129?)

On Fri, 19 Nov 2021 14:30:52 +0100, Ævar Arnfjörð Bjarmason wrote

Please don't add the Reviewed-by headers yourself, either Junio
accumulates them, or if someone explicitly mentions that you can add it
with their name it's OK.

I think I misunderstood the meanings of the header before.
Thanks for the important tips.

Better: "Cannot be combined with OPT."
Better: "Cannot be combined with OPT or OPT2."
...
Better to preserve the wrapping here, to stay within 79 columns.

Will apply.

Just use:

    test_commit A &&
    test_commit B

etc?
...
Stray echo? Unclear why this test setup is so complex, shouldn't this just be (continued from above):

    mkdir -p C &&
    test_commit C/D.txt

To test nested dirs?

Will apply.

just cut -f1 instead of awk? Also don't put "git" on the LHS of a pipe,
it might hide segfaults. Also applies to the below.

Will apply, and could you please describe the problem with more details?
(appreciate if there is an executable example)

Thank you.

Diffs from previous patch:

      1. Remove "Reviewed-by" headers in commit message.
      2. Optimize option descriptions in Doc.
         (Ævar Arnfjörð Bjarmason' advice)
      3. Optimize and bugfix in "t3104".
      	 (Ævar Arnfjörð Bjarmason' advice)
      4. The formatting problems of line wrappers (over 79 col)

All the advices are from Ævar Arnfjörð Bjarmason and Junio C Hamano,
thank you very much.

Althought some advices are apply in this path, but some questions
remains, they are in link [1].

[1] https://public-inbox.org/git/20211122074538.87255-1-dyroneteng@gmail.com/

Teng Long (1):
  ls-tree.c: support `--oid-only` option for "git-ls-tree"

 Documentation/git-ls-tree.txt |  8 +++++--
 builtin/ls-tree.c             | 27 ++++++++++++++++-------
 t/t3104-ls-tree-oid.sh        | 40 +++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

Range-diff against v2:
1:  8b68568d6c ! 1:  6c15b4c176 ls-tree.c: support `--oid-only` option for "git-ls-tree"
    @@ Commit message
         only print out the OID of the object. `--oid-only` and `--name-only`
         are mutually exclusive in use.
     
    -    Reviewed-by: Jeff King [off-list ref]
    -    Reviewed-by: Ævar Arnfjörð Bjarmason [off-list ref]
    -    Reviewed-by: Đoàn Trần Công Danh [off-list ref]
         Signed-off-by: Teng Long [off-list ref]
     
      ## Documentation/git-ls-tree.txt ##
-- 
2.33.1.10.g438dd9044d.dirty

Sometimes, we only want to get the objects from output of `ls-tree`
and commands like `sed` or `cut` is usually used to intercept the
origin output to achieve this purpose in practical.

This commit supply an option names `--oid-only` to let `git ls-tree`
only print out the OID of the object. `--oid-only` and `--name-only`
are mutually exclusive in use.

Signed-off-by: Teng Long <redacted>
---
 Documentation/git-ls-tree.txt |  8 +++++--
 builtin/ls-tree.c             | 27 ++++++++++++++++-------
 t/t3104-ls-tree-oid.sh        | 40 +++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index db02d6d79a..bc711dc00a 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt

@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git ls-tree' [-d] [-r] [-t] [-l] [-z]
-	    [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]]
+	    [--name-only] [--name-status] [--oid-only]
+	    [--full-name] [--full-tree] [--abbrev[=<n>]]
 	    <tree-ish> [<path>...]
 
 DESCRIPTION

@@ -59,7 +60,10 @@ OPTIONS
 --name-only::
 --name-status::
 	List only filenames (instead of the "long" output), one per line.
-
+	Cannot be used with `--oid-only` together.
+--oid-only::
+	List only OIDs of the objects, one per line. Cannot be used with
+	`--name-only` or `--name-status` together.
 --abbrev[=<n>]::
 	Instead of showing the full 40-byte hexadecimal object
 	lines, show the shortest prefix that is at least '<n>'

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3a442631c7..1e4a82e669 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -18,19 +18,26 @@ static int line_termination = '\n';
 #define LS_RECURSIVE 1
 #define LS_TREE_ONLY 2
 #define LS_SHOW_TREES 4
-#define LS_NAME_ONLY 8
-#define LS_SHOW_SIZE 16
+#define LS_SHOW_SIZE 8
 static int abbrev;
 static int ls_options;
 static struct pathspec pathspec;
 static int chomp_prefix;
 static const char *ls_tree_prefix;
 
-static const  char * const ls_tree_usage[] = {
+static const char * const ls_tree_usage[] = {
 	N_("git ls-tree [<options>] <tree-ish> [<path>...]"),
 	NULL
 };
 
+enum {
+	MODE_UNSPECIFIED = 0,
+	MODE_NAME_ONLY,
+	MODE_OID_ONLY
+};
+
+static int cmdmode = MODE_UNSPECIFIED;
+
 static int show_recursive(const char *base, int baselen, const char *pathname)
 {
 	int i;

@@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
-	if (!(ls_options & LS_NAME_ONLY)) {
+	if (cmdmode == 2) {
+		printf("%s\n", find_unique_abbrev(oid, abbrev));
+		return 0;
+	}
+
+	if (cmdmode == 0) {
 		if (ls_options & LS_SHOW_SIZE) {
 			char size_text[24];
 			if (!strcmp(type, blob_type)) {

@@ -135,10 +147,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 			    N_("terminate entries with NUL byte"), 0),
 		OPT_BIT('l', "long", &ls_options, N_("include object size"),
 			LS_SHOW_SIZE),
-		OPT_BIT(0, "name-only", &ls_options, N_("list only filenames"),
-			LS_NAME_ONLY),
-		OPT_BIT(0, "name-status", &ls_options, N_("list only filenames"),
-			LS_NAME_ONLY),
+		OPT_CMDMODE('n', "name-only", &cmdmode, N_("list only filenames"), MODE_NAME_ONLY),
+		OPT_CMDMODE('s', "name-status", &cmdmode, N_("list only filenames"), MODE_NAME_ONLY),
+		OPT_CMDMODE('o', "oid-only", &cmdmode, N_("list only oids"), MODE_OID_ONLY),
 		OPT_SET_INT(0, "full-name", &chomp_prefix,
 			    N_("use full path names"), 0),
 		OPT_BOOL(0, "full-tree", &full_tree,

diff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh
new file mode 100755
index 0000000000..4c02cdd3c3
--- /dev/null
+++ b/t/t3104-ls-tree-oid.sh

@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='git ls-tree oids handling.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo 111 >1.txt &&
+	echo 222 >2.txt &&
+	mkdir -p path0/a/b/c &&
+	echo 333 >path0/a/b/c/3.txt &&
+	find *.txt path* \( -type f -o -type l \) -print |
+	xargs git update-index --add &&
+	tree=$(git write-tree) &&
+	echo $tree
+'
+
+test_expect_success 'usage: --oid-only' '
+	git ls-tree --oid-only $tree >current &&
+	git ls-tree $tree | awk "{print \$3}" >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with -r' '
+	git ls-tree --oid-only -r $tree >current &&
+	git ls-tree -r $tree | awk "{print \$3}" >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with --abbrev' '
+	git ls-tree --oid-only --abbrev=6 $tree >current &&
+	git ls-tree --abbrev=6 $tree | awk "{print \$3}" > expected &&
+	test_cmp current expected
+'
+
+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '
+	test_incompatible_usage git ls-tree --oid-only --name-only
+'
+
+test_done

-- 
2.33.1.10.g438dd9044d.dirty

On Mon, Nov 22 2021, Teng Long wrote:

On Fri, 19 Nov 2021 14:30:52 +0100, Ævar Arnfjörð Bjarmason wrote

quoted

just cut -f1 instead of awk? Also don't put "git" on the LHS of a pipe,
it might hide segfaults. Also applies to the below.

Will apply, and could you please describe the problem with more details?
(appreciate if there is an executable example)

Run this in a terminal:

    git stawtus | cat; echo $?;

The LHS of the pipe fails, but the exit code of that command is
hidden. So we prefer:

    git stawtus >out && # fails
    [...]

[ Sorry if you receive this mail twice, it seems like it didn't get
through the first time. ]

On Mon, Nov 22, 2021 at 9:50 AM Teng Long [off-list ref] wrote:

Shouldn't the synopsis also indicate that the options are exclusive, e.g.
  [--name-only | --oid-only]  ?

Besides adding the new --oid-only mode, you also add one letter acronyms for
  [-n | --name-only]
  [-s | --name-status ]
and one letter abbreviation
  [-o | --oid-only ]
which are all undocumented in the help page. If we want the short one
letter version,
they should be documented. For me, it is at least questionable why we
introduce them
and more so in a commit adding --oid-only.

I don't see the test using any symbolic links. Why are we searching for
symbolic links with "-type l" here?

-Peter

+       xargs git update-index --add &&
+       tree=$(git write-tree) &&
+       echo $tree
+'

+
+test_expect_success 'usage: --oid-only' '
+       git ls-tree --oid-only $tree >current &&
+       git ls-tree $tree | awk "{print \$3}" >expected &&
+       test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with -r' '
+       git ls-tree --oid-only -r $tree >current &&
+       git ls-tree -r $tree | awk "{print \$3}" >expected &&
+       test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with --abbrev' '
+       git ls-tree --oid-only --abbrev=6 $tree >current &&
+       git ls-tree --abbrev=6 $tree | awk "{print \$3}" > expected &&
+       test_cmp current expected
+'
+
+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '
+       test_incompatible_usage git ls-tree --oid-only --name-only
+'
+
+test_done
--
2.33.1.10.g438dd9044d.dirty

On 2021-11-22 16:07:28+0800, Teng Long [off-list ref] wrote:

Please indicate those options are incompatible (as someone else said):

	[--name-only | --name-status | --oid-only]

I think it's better to use the enum name:

	if (cmdmode == MODE_OID_ONLY) {

+		printf("%s\n", find_unique_abbrev(oid, abbrev));
+		return 0;
+	}
+
+	if (cmdmode == 0) {

Ditto:

	if (cmdmode == MODE_UNSPECIFIED) {

Speaking about this, where will MODE_NAME_ONLY be used?

It seems like you haven't updated the test code from v2

-- 
Danh

Thanks for the discussions on v3 (even I send a patch with
wrong contents and the right cover). So I looked at them, and
I think I have to send a new patch first, so this includes:

    1. Commit message modifications (Junio C Hamano's advice)
    2. Documentation modifications (Peter Baumann's advice)
    3. To use the MODE enum name instead (Đoàn Trần Công Danh's advice)

The other discussions I will reply today.

Teng Long (1):
  ls-tree.c: support `--oid-only` option for "git-ls-tree"

 Documentation/git-ls-tree.txt | 18 ++++++++++++---
 builtin/ls-tree.c             | 30 +++++++++++++++++-------
 t/t3104-ls-tree-oid.sh        | 43 +++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 11 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

Range-diff against v3:
1:  8b68568d6c ! 1:  63876dbeb7 ls-tree.c: support `--oid-only` option for "git-ls-tree"
    @@ Commit message
     
         Sometimes, we only want to get the objects from output of `ls-tree`
         and commands like `sed` or `cut` is usually used to intercept the
    -    origin output to achieve this purpose in practical.
    +    origin output to achieve this purpose in practice.
     
    -    This commit supply an option names `--oid-only` to let `git ls-tree`
    -    only print out the OID of the object. `--oid-only` and `--name-only`
    -    are mutually exclusive in use.
    +    This commit teach the "--oid-only" option to tell the command to
    +    only show the object name, just like "--name-only" option tells the
    +    command to only show the path component, for each entry. These two
    +    options are mutually exclusive.
     
    -    Reviewed-by: Jeff King [off-list ref]
    -    Reviewed-by: Ævar Arnfjörð Bjarmason [off-list ref]
    -    Reviewed-by: Đoàn Trần Công Danh [off-list ref]
         Signed-off-by: Teng Long [off-list ref]
     
      ## Documentation/git-ls-tree.txt ##
    -@@ Documentation/git-ls-tree.txt: SYNOPSIS
    +@@ Documentation/git-ls-tree.txt: git-ls-tree - List the contents of a tree object
    + SYNOPSIS
      --------
      [verse]
    - 'git ls-tree' [-d] [-r] [-t] [-l] [-z]
    +-'git ls-tree' [-d] [-r] [-t] [-l] [-z]
     -	    [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]]
    -+	    [--name-only] [--name-status] [--oid-only]
    ++'git ls-tree' [-d] [-r] [-t] [-l] [-z] [-n] [-s] [-o]
    ++	    [--name-only | --oid-only]
    ++	    [--name-status | --oid-only]
     +	    [--full-name] [--full-tree] [--abbrev[=<n>]]
      	    <tree-ish> [<path>...]
      
      DESCRIPTION
     @@ Documentation/git-ls-tree.txt: OPTIONS
    + 	\0 line termination on output and do not quote filenames.
    + 	See OUTPUT FORMAT below for more information.
    + 
    ++-n::
      --name-only::
    - --name-status::
    +---name-status::
      	List only filenames (instead of the "long" output), one per line.
    --
    -+	Cannot be used with `--oid-only` together.
    ++	Cannot be combined with `--oid-only`.
    ++
    ++-s::
    ++--name-status::
    ++	Consistent behavior with `--name-only`.
    ++
    ++-o::
     +--oid-only::
    -+	List only OIDs of the objects, one per line. Cannot be used with
    -+	`--name-only` or `--name-status` together.
    ++	List only names of the objects, one per line. Cannot be combined
    ++	with `--name-only` or `--name-status`.
    + 
      --abbrev[=<n>]::
      	Instead of showing the full 40-byte hexadecimal object
    - 	lines, show the shortest prefix that is at least '<n>'
     
      ## builtin/ls-tree.c ##
     @@ builtin/ls-tree.c: static int line_termination = '\n';
    @@ builtin/ls-tree.c: static int show_tree(const struct object_id *oid, struct strb
      		return 0;
      
     -	if (!(ls_options & LS_NAME_ONLY)) {
    -+	if (cmdmode == 2) {
    ++	if (cmdmode == MODE_OID_ONLY) {
     +		printf("%s\n", find_unique_abbrev(oid, abbrev));
     +		return 0;
     +	}
     +
    -+	if (cmdmode == 0) {
    ++	if (cmdmode == MODE_UNSPECIFIED) {
      		if (ls_options & LS_SHOW_SIZE) {
      			char size_text[24];
      			if (!strcmp(type, blob_type)) {
    @@ builtin/ls-tree.c: int cmd_ls_tree(int argc, const char **argv, const char *pref
     -			LS_NAME_ONLY),
     -		OPT_BIT(0, "name-status", &ls_options, N_("list only filenames"),
     -			LS_NAME_ONLY),
    -+		OPT_CMDMODE('n', "name-only", &cmdmode, N_("list only filenames"), MODE_NAME_ONLY),
    -+		OPT_CMDMODE('s', "name-status", &cmdmode, N_("list only filenames"), MODE_NAME_ONLY),
    -+		OPT_CMDMODE('o', "oid-only", &cmdmode, N_("list only oids"), MODE_OID_ONLY),
    ++		OPT_CMDMODE('n', "name-only", &cmdmode,
    ++			    N_("list only filenames"), MODE_NAME_ONLY),
    ++		OPT_CMDMODE('s', "name-status", &cmdmode,
    ++			    N_("list only filenames"), MODE_NAME_ONLY),
    ++		OPT_CMDMODE('o', "oid-only", &cmdmode,
    ++			    N_("list only oids"), MODE_OID_ONLY),
      		OPT_SET_INT(0, "full-name", &chomp_prefix,
      			    N_("use full path names"), 0),
      		OPT_BOOL(0, "full-tree", &full_tree,
    @@ t/t3104-ls-tree-oid.sh (new)
     +. ./test-lib.sh
     +
     +test_expect_success 'setup' '
    -+	echo 111 >1.txt &&
    -+	echo 222 >2.txt &&
    -+	mkdir -p path0/a/b/c &&
    -+	echo 333 >path0/a/b/c/3.txt &&
    ++	test_commit A &&
    ++	test_commit B &&
    ++	mkdir -p C &&
    ++	test_commit C/D.txt &&
     +	find *.txt path* \( -type f -o -type l \) -print |
     +	xargs git update-index --add &&
     +	tree=$(git write-tree) &&
    @@ t/t3104-ls-tree-oid.sh (new)
     +
     +test_expect_success 'usage: --oid-only' '
     +	git ls-tree --oid-only $tree >current &&
    -+	git ls-tree $tree | awk "{print \$3}" >expected &&
    ++	git ls-tree $tree >result &&
    ++	cut -f1 result | cut -d " " -f3 >expected &&
     +	test_cmp current expected
     +'
     +
     +test_expect_success 'usage: --oid-only with -r' '
     +	git ls-tree --oid-only -r $tree >current &&
    -+	git ls-tree -r $tree | awk "{print \$3}" >expected &&
    ++	git ls-tree -r $tree >result &&
    ++	cut -f1 result | cut -d " " -f3 >expected &&
     +	test_cmp current expected
     +'
     +
     +test_expect_success 'usage: --oid-only with --abbrev' '
     +	git ls-tree --oid-only --abbrev=6 $tree >current &&
    -+	git ls-tree --abbrev=6 $tree | awk "{print \$3}" > expected &&
    ++	git ls-tree --abbrev=6 $tree >result &&
    ++	cut -f1 result | cut -d " " -f3 >expected &&
     +	test_cmp current expected
     +'
     +
    -+test_expect_failure 'usage: incompatible options: --name-only with --oid-only' '
    -+	test_incompatible_usage git ls-tree --oid-only --name-only
    ++test_expect_success 'usage: incompatible options: --name-only with --oid-only' '
    ++	test_expect_code 129 git ls-tree --oid-only --name-only
     +'
     +
     +test_done
-- 
2.33.1.10.g75523f744f.dirty

Sometimes, we only want to get the objects from output of `ls-tree`
and commands like `sed` or `cut` is usually used to intercept the
origin output to achieve this purpose in practice.

This commit teach the "--oid-only" option to tell the command to
only show the object name, just like "--name-only" option tells the
command to only show the path component, for each entry. These two
options are mutually exclusive.

Signed-off-by: Teng Long <redacted>
---
 Documentation/git-ls-tree.txt | 18 ++++++++++++---
 builtin/ls-tree.c             | 30 +++++++++++++++++-------
 t/t3104-ls-tree-oid.sh        | 43 +++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 11 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index db02d6d79a..fd2a871ca5 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt

@@ -9,8 +9,10 @@ git-ls-tree - List the contents of a tree object
 SYNOPSIS
 --------
 [verse]
-'git ls-tree' [-d] [-r] [-t] [-l] [-z]
-	    [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]]
+'git ls-tree' [-d] [-r] [-t] [-l] [-z] [-n] [-s] [-o]
+	    [--name-only | --oid-only]
+	    [--name-status | --oid-only]
+	    [--full-name] [--full-tree] [--abbrev[=<n>]]
 	    <tree-ish> [<path>...]
 
 DESCRIPTION

@@ -56,9 +58,19 @@ OPTIONS
 	\0 line termination on output and do not quote filenames.
 	See OUTPUT FORMAT below for more information.
 
+-n::
 --name-only::
---name-status::
 	List only filenames (instead of the "long" output), one per line.
+	Cannot be combined with `--oid-only`.
+
+-s::
+--name-status::
+	Consistent behavior with `--name-only`.
+
+-o::
+--oid-only::
+	List only names of the objects, one per line. Cannot be combined
+	with `--name-only` or `--name-status`.
 
 --abbrev[=<n>]::
 	Instead of showing the full 40-byte hexadecimal object

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3a442631c7..0c2153a5ad 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -18,19 +18,26 @@ static int line_termination = '\n';
 #define LS_RECURSIVE 1
 #define LS_TREE_ONLY 2
 #define LS_SHOW_TREES 4
-#define LS_NAME_ONLY 8
-#define LS_SHOW_SIZE 16
+#define LS_SHOW_SIZE 8
 static int abbrev;
 static int ls_options;
 static struct pathspec pathspec;
 static int chomp_prefix;
 static const char *ls_tree_prefix;
 
-static const  char * const ls_tree_usage[] = {
+static const char * const ls_tree_usage[] = {
 	N_("git ls-tree [<options>] <tree-ish> [<path>...]"),
 	NULL
 };
 
+enum {
+	MODE_UNSPECIFIED = 0,
+	MODE_NAME_ONLY,
+	MODE_OID_ONLY
+};
+
+static int cmdmode = MODE_UNSPECIFIED;
+
 static int show_recursive(const char *base, int baselen, const char *pathname)
 {
 	int i;

@@ -90,7 +97,12 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
-	if (!(ls_options & LS_NAME_ONLY)) {
+	if (cmdmode == MODE_OID_ONLY) {
+		printf("%s\n", find_unique_abbrev(oid, abbrev));
+		return 0;
+	}
+
+	if (cmdmode == MODE_UNSPECIFIED) {
 		if (ls_options & LS_SHOW_SIZE) {
 			char size_text[24];
 			if (!strcmp(type, blob_type)) {

@@ -135,10 +147,12 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 			    N_("terminate entries with NUL byte"), 0),
 		OPT_BIT('l', "long", &ls_options, N_("include object size"),
 			LS_SHOW_SIZE),
-		OPT_BIT(0, "name-only", &ls_options, N_("list only filenames"),
-			LS_NAME_ONLY),
-		OPT_BIT(0, "name-status", &ls_options, N_("list only filenames"),
-			LS_NAME_ONLY),
+		OPT_CMDMODE('n', "name-only", &cmdmode,
+			    N_("list only filenames"), MODE_NAME_ONLY),
+		OPT_CMDMODE('s', "name-status", &cmdmode,
+			    N_("list only filenames"), MODE_NAME_ONLY),
+		OPT_CMDMODE('o', "oid-only", &cmdmode,
+			    N_("list only oids"), MODE_OID_ONLY),
 		OPT_SET_INT(0, "full-name", &chomp_prefix,
 			    N_("use full path names"), 0),
 		OPT_BOOL(0, "full-tree", &full_tree,

diff --git a/t/t3104-ls-tree-oid.sh b/t/t3104-ls-tree-oid.sh
new file mode 100755
index 0000000000..2d349f6e46
--- /dev/null
+++ b/t/t3104-ls-tree-oid.sh

@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='git ls-tree oids handling.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit A &&
+	test_commit B &&
+	mkdir -p C &&
+	test_commit C/D.txt &&
+	find *.txt path* \( -type f -o -type l \) -print |
+	xargs git update-index --add &&
+	tree=$(git write-tree) &&
+	echo $tree
+'
+
+test_expect_success 'usage: --oid-only' '
+	git ls-tree --oid-only $tree >current &&
+	git ls-tree $tree >result &&
+	cut -f1 result | cut -d " " -f3 >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with -r' '
+	git ls-tree --oid-only -r $tree >current &&
+	git ls-tree -r $tree >result &&
+	cut -f1 result | cut -d " " -f3 >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: --oid-only with --abbrev' '
+	git ls-tree --oid-only --abbrev=6 $tree >current &&
+	git ls-tree --abbrev=6 $tree >result &&
+	cut -f1 result | cut -d " " -f3 >expected &&
+	test_cmp current expected
+'
+
+test_expect_success 'usage: incompatible options: --name-only with --oid-only' '
+	test_expect_code 129 git ls-tree --oid-only --name-only
+'
+
+test_done

-- 
2.33.1.10.g75523f744f.dirty

Diffs from patch v4:

* Change `--oid-only` to `--object-only`.
  Word "oid" may not be easily understood for users.

* The commit message was modified in terms of Junio's advice.

* Use "OPT_CMDMODE()" to make `--name-only`, `--object-only` and
  `--long` mutually exclusive with each other.

* After options been parsed, translate options to bitmask, then use
  cleaner bitwise to determine which fields will be shown.

* Add tests for mutually exclusive options.

* Documentation modifications about the change of option name.

Thanks.

Teng Long (1):
  ls-tree.c: support `--object-only` option for "git-ls-tree"

 Documentation/git-ls-tree.txt |   7 +-
 builtin/ls-tree.c             | 125 ++++++++++++++++++++++++----------
 t/t3103-ls-tree-misc.sh       |   8 +++
 t/t3104-ls-tree-oid.sh        |  51 ++++++++++++++
 4 files changed, 154 insertions(+), 37 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

Range-diff against v4:
-:  ---------- > 1:  38d55a878c ls-tree.c: support `--object-only` option for "git-ls-tree"
-- 
2.33.1.10.gd2a07a0ec5.dirty

We usually pipe the output from `git ls-trees` to tools like
`sed` or `cut` when we only want to extract some fields.

When we want only the pathname component, we can pass
`--name-only` option to omit such a pipeline, but there are no
options for extracting other fields.

Teach the "--object-only" option to the command to only show the
object name. This option cannot be used together with
"--name-only" or "--long" (mutually exclusive).

Signed-off-by: Teng Long <redacted>
---
 Documentation/git-ls-tree.txt |   7 +-
 builtin/ls-tree.c             | 125 ++++++++++++++++++++++++----------
 t/t3103-ls-tree-misc.sh       |   8 +++
 t/t3104-ls-tree-oid.sh        |  51 ++++++++++++++
 4 files changed, 154 insertions(+), 37 deletions(-)
 create mode 100755 t/t3104-ls-tree-oid.sh

diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index db02d6d79a..729370f235 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt

@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-tree' [-d] [-r] [-t] [-l] [-z]
-	    [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]]
+	    [--name-only] [--name-status] [--object-only] [--full-name] [--full-tree] [--abbrev[=<n>]]
 	    <tree-ish> [<path>...]
 
 DESCRIPTION

@@ -59,6 +59,11 @@ OPTIONS
 --name-only::
 --name-status::
 	List only filenames (instead of the "long" output), one per line.
+	Cannot be combined with `--object-only`.
+
+--object-only::
+	List only names of the objects, one per line. Cannot be combined
+	with `--name-only` or `--name-status`.
 
 --abbrev[=<n>]::
 	Instead of showing the full 40-byte hexadecimal object

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3a442631c7..beaa8bf13b 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c

@@ -16,21 +16,38 @@
 
 static int line_termination = '\n';
 #define LS_RECURSIVE 1
-#define LS_TREE_ONLY 2
-#define LS_SHOW_TREES 4
-#define LS_NAME_ONLY 8
-#define LS_SHOW_SIZE 16
+#define LS_TREE_ONLY 1 << 1
+#define LS_SHOW_TREES 1 << 2
+#define LS_NAME_ONLY 1 << 3
+#define LS_SHOW_SIZE 1 << 4
+#define LS_OBJECT_ONLY 1 << 5
 static int abbrev;
 static int ls_options;
 static struct pathspec pathspec;
 static int chomp_prefix;
 static const char *ls_tree_prefix;
+static unsigned int shown_bits = 0;
+#define SHOW_DEFAULT 29 /* 11101 size is not shown to output by default */
+#define SHOW_MODE 1 << 4
+#define SHOW_TYPE 1 << 3
+#define SHOW_OBJECT_NAME 1 << 2
+#define SHOW_SIZE 1 << 1
+#define SHOW_FILE_NAME 1
 
 static const  char * const ls_tree_usage[] = {
 	N_("git ls-tree [<options>] <tree-ish> [<path>...]"),
 	NULL
 };
 
+enum {
+	MODE_UNSPECIFIED = 0,
+	MODE_NAME_ONLY,
+	MODE_OBJECT_ONLY,
+	MODE_LONG
+};
+
+static int cmdmode = MODE_UNSPECIFIED;
+
 static int show_recursive(const char *base, int baselen, const char *pathname)
 {
 	int i;

@@ -66,6 +83,7 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 {
 	int retval = 0;
 	int baselen;
+	int follow = 0;
 	const char *type = blob_type;
 
 	if (S_ISGITLINK(mode)) {

@@ -74,8 +92,8 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 		 *
 		 * Something similar to this incomplete example:
 		 *
-		if (show_subprojects(base, baselen, pathname))
-			retval = READ_TREE_RECURSIVE;
+		 * if (show_subprojects(base, baselen, pathname))
+		 *	retval = READ_TREE_RECURSIVE;
 		 *
 		 */
 		type = commit_type;

@@ -90,35 +108,67 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
-	if (!(ls_options & LS_NAME_ONLY)) {
-		if (ls_options & LS_SHOW_SIZE) {
-			char size_text[24];
-			if (!strcmp(type, blob_type)) {
-				unsigned long size;
-				if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
-					xsnprintf(size_text, sizeof(size_text),
-						  "BAD");
-				else
-					xsnprintf(size_text, sizeof(size_text),
-						  "%"PRIuMAX, (uintmax_t)size);
-			} else
-				xsnprintf(size_text, sizeof(size_text), "-");
-			printf("%06o %s %s %7s\t", mode, type,
-			       find_unique_abbrev(oid, abbrev),
-			       size_text);
+	if (shown_bits & SHOW_MODE) {
+		printf("%06o",mode);
+		follow = 1;
+	}
+	if (shown_bits & SHOW_TYPE) {
+		printf("%s%s", follow == 1 ? " " : "", type);
+		follow = 1;
+	}
+	if (shown_bits & SHOW_OBJECT_NAME) {
+		printf("%s%s", follow == 1 ? " " : "",
+		       find_unique_abbrev(oid, abbrev));
+		if (!(shown_bits ^ SHOW_OBJECT_NAME))
+			printf("%c", line_termination);
+		follow = 1;
+	}
+	if (shown_bits & SHOW_SIZE) {
+		char size_text[24];
+		if (!strcmp(type, blob_type)) {
+			unsigned long size;
+			if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
+				xsnprintf(size_text, sizeof(size_text), "BAD");
+			else
+				xsnprintf(size_text, sizeof(size_text),
+					  "%"PRIuMAX, (uintmax_t)size);
 		} else
-			printf("%06o %s %s\t", mode, type,
-			       find_unique_abbrev(oid, abbrev));
+			xsnprintf(size_text, sizeof(size_text), "-");
+		printf("%s%7s", follow == 1 ? " " : "", size_text);
+		follow = 1;
+	}
+	if (shown_bits & SHOW_FILE_NAME) {
+		if (follow)
+			printf("\t");
+		baselen = base->len;
+		strbuf_addstr(base, pathname);
+		write_name_quoted_relative(base->buf,
+					   chomp_prefix ? ls_tree_prefix : NULL,
+					   stdout, line_termination);
+		strbuf_setlen(base, baselen);
 	}
-	baselen = base->len;
-	strbuf_addstr(base, pathname);
-	write_name_quoted_relative(base->buf,
-				   chomp_prefix ? ls_tree_prefix : NULL,
-				   stdout, line_termination);
-	strbuf_setlen(base, baselen);
 	return retval;
 }
 
+static int parse_shown_fields(void)
+{
+	if (cmdmode == MODE_NAME_ONLY) {
+		shown_bits = SHOW_FILE_NAME;
+		return 0;
+	}
+	if (cmdmode == MODE_OBJECT_ONLY) {
+		shown_bits = SHOW_OBJECT_NAME;
+		return 0;
+	}
+	if (!ls_options || (ls_options & LS_RECURSIVE)
+	    || (ls_options & LS_SHOW_TREES)
+	    || (ls_options & LS_TREE_ONLY))
+		shown_bits = SHOW_DEFAULT;
+	if (cmdmode == MODE_LONG)
+		shown_bits = SHOW_DEFAULT | SHOW_SIZE;
+	return 1;
+}
+
 int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 {
 	struct object_id oid;