Thread (194 messages) 194 messages, 11 authors, 2022-04-07

Re: [PATCH v2 1/1] ls-tree.c: support `--oid-only` option for "git-ls-tree"

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-11-19 13:40:29

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".
quoted hunk ↗ jump to hunk
@@ -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),
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/ (local) :)

Better to use:

    test_expect_code 128 ... # (or was it 129?)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help