From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Fri, 17 Dec 2021 13:09:48 GMT
Subject: Re: [PATCH v6 1/1] ls-tree.c: support `--object-only` option for "git-ls-tree"
Message-ID: <211217.86o85f8jey.gmgdl@evledraar.gmail.com>
In-Reply-To: <2e449d1c792ff81da5f22c8bf65ed33c393d62f8.1639721750.git.dyroneteng@gmail.com>


On Fri, Dec 17 2021, Teng Long wrote:

> [...]
>  int cmd_ls_tree(int argc, const char **argv, const char *prefix)
>  {
>  	struct object_id oid;
> @@ -133,12 +189,14 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
>  			LS_SHOW_TREES),
>  		OPT_SET_INT('z', NULL, &line_termination,
>  			    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('l', "long", &cmdmode, N_("include object size"),
> +			    MODE_LONG),
> +		OPT_CMDMODE(0, "name-only", &cmdmode, N_("list only filenames"),
> +			    MODE_NAME_ONLY),
> +		OPT_CMDMODE(0, "name-status", &cmdmode, N_("list only filenames"),
> +			    MODE_NAME_ONLY),
> +		OPT_CMDMODE(0, "object-only", &cmdmode, N_("list only objects"),
> +			    MODE_OBJECT_ONLY),
>  		OPT_SET_INT(0, "full-name", &chomp_prefix,
>  			    N_("use full path names"), 0),
>  		OPT_BOOL(0, "full-tree", &full_tree,

Very nice to have the OPT_CMDMODE for asserting the usage, but this
would be even better if it were done as a separate commit. I.e. let's
first do prep cleanups, then the new --object-name mode.

> +test_expect_success 'usage: incompatible options: --name-status with --long' '
> +	test_expect_code 129 git ls-tree --long --name-status
> +'
> +
> +test_expect_success 'usage: incompatible options: --name-only with --long' '
> +	test_expect_code 129 git ls-tree --long --name-only
> +'
> +
>  test_done
> [...]
> +test_expect_success 'usage: incompatible options: --name-only with --object-only' '
> +	test_expect_code 129 git ls-tree --object-only --name-only
> +'
> +
> +test_expect_success 'usage: incompatible options: --name-status with --object-only' '
> +	test_expect_code 129 git ls-tree --object-only --name-status
> +'
> +
> +test_expect_success 'usage: incompatible options: --long with --object-only' '
> +	test_expect_code 129 git ls-tree --object-only --long
> +'

These tests don't check for what you think they check, because you don't
supply a <tree-ish>. So they're really just dying for the same reason a:

    git ls-tree

Would.