Hi Teng,
On Sat, 1 Jan 2022, Teng Long wrote:
> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
> index 009ffeb15d..6e3e5a4d06 100644
> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -56,23 +56,75 @@ enum {
>
> static int cmdmode = MODE_UNSPECIFIED;
>
> -static int parse_shown_fields(void)
> +static const char *format;
> +static const char *default_format = "%(mode) %(type) %(object)%x09%(file)";
> +static const char *long_format = "%(mode) %(type) %(object) %(size:padded)%x09%(file)";
> +static const char *name_only_format = "%(file)";
> +static const char *object_only_format = "%(object)";
> +
> +static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
> + const enum object_type type, unsigned int padded)
> {
> - if (cmdmode == MODE_NAME_ONLY) {
> - shown_bits = SHOW_FILE_NAME;
> - return 0;
> + if (type == OBJ_BLOB) {
> + unsigned long size;
> + if (oid_object_info(the_repository, oid, &size) < 0)
> + die(_("could not get object info about '%s'"),
> + oid_to_hex(oid));
> + if (padded)
> + strbuf_addf(line, "%7" PRIuMAX, (uintmax_t)size);
> + else
> + strbuf_addf(line, "%" PRIuMAX, (uintmax_t)size);
> + } else if (padded) {
> + strbuf_addf(line, "%7s", "-");
This, along with two other similar instances, triggers the
`static-analysis` job in the CI failure of `seen`. The suggested diff is:
-- snip --
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 6e3e5a4d0634..8301d1a15f9a 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -75,7 +75,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
else
strbuf_addf(line, "%" PRIuMAX, (uintmax_t)size);
} else if (padded) {
- strbuf_addf(line, "%7s", "-");
+ strbuf_addstr(line, "-");
} else {
strbuf_addstr(line, "-");
}
@@ -110,7 +110,7 @@ static size_t expand_show_tree(struct strbuf *line, const char *start,
} else if (skip_prefix(start, "(size)", &p)) {
expand_objectsize(line, data->oid, data->type, 0);
} else if (skip_prefix(start, "(object)", &p)) {
- strbuf_addstr(line, find_unique_abbrev(data->oid, abbrev));
+ strbuf_add_unique_abbrev(line, data->oid, abbrev);
} else if (skip_prefix(start, "(file)", &p)) {
const char *name = data->base->buf;
const char *prefix = chomp_prefix ? ls_tree_prefix : NULL;
@@ -119,7 +119,7 @@ static size_t expand_show_tree(struct strbuf *line, const char *start,
strbuf_addstr(data->base, data->pathname);
name = relative_path(data->base->buf, prefix, &sb);
quote_c_style(name, "ed, NULL, 0);
- strbuf_addstr(line, quoted.buf);
+ strbuf_addbuf(line, "ed);
} else {
errlen = (unsigned long)len;
die(_("bad ls-tree format: %%%.*s"), errlen, start);
-- snap --
But I think that the first hunk indicates a deeper issue, as `%7s`
probably meant to pad the dash to seven dashes (which that format won't
accomplish, but `strbuf_addchars()` would)?
Ciao,
Dscho
> + } else {
> + strbuf_addstr(line, "-");
> }
> - if (cmdmode == MODE_OBJECT_ONLY) {
> - shown_bits = SHOW_OBJECT_NAME;
> - return 0;
> +}
> +
> +static size_t expand_show_tree(struct strbuf *line, const char *start,
> + void *context)
> +{
> + struct shown_data *data = context;
> + const char *end;
> + const char *p;
> + unsigned int errlen;
> + size_t len;
> + len = strbuf_expand_literal_cb(line, start, NULL);
> + if (len)
> + return len;
> +
> + if (*start != '(')
> + die(_("bad ls-tree format: as '%s'"), start);
> +
> + end = strchr(start + 1, ')');
> + if (!end)
> + die(_("bad ls-tree format: element '%s' does not end in ')'"), start);
> +
> + len = end - start + 1;
> + if (skip_prefix(start, "(mode)", &p)) {
> + strbuf_addf(line, "%06o", data->mode);
> + } else if (skip_prefix(start, "(type)", &p)) {
> + strbuf_addstr(line, type_name(data->type));
> + } else if (skip_prefix(start, "(size:padded)", &p)) {
> + expand_objectsize(line, data->oid, data->type, 1);
> + } else if (skip_prefix(start, "(size)", &p)) {
> + expand_objectsize(line, data->oid, data->type, 0);
> + } else if (skip_prefix(start, "(object)", &p)) {
> + strbuf_addstr(line, find_unique_abbrev(data->oid, abbrev));
> + } else if (skip_prefix(start, "(file)", &p)) {
> + const char *name = data->base->buf;
> + const char *prefix = chomp_prefix ? ls_tree_prefix : NULL;
> + struct strbuf quoted = STRBUF_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_addstr(data->base, data->pathname);
> + name = relative_path(data->base->buf, prefix, &sb);
> + quote_c_style(name, "ed, NULL, 0);
> + strbuf_addstr(line, quoted.buf);
> + } else {
> + errlen = (unsigned long)len;
> + die(_("bad ls-tree format: %%%.*s"), errlen, start);
> }
> - 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;
> + return len;
> }
>
> static int show_recursive(const char *base, size_t baselen,
> @@ -106,6 +158,75 @@ static int show_recursive(const char *base, size_t baselen,
> return 0;
> }
>
> +static int show_tree_init(enum object_type *type, struct strbuf *base,
> + const char *pathname, unsigned mode, int *retval)
> +{
> + if (S_ISGITLINK(mode)) {
> + *type = OBJ_COMMIT;
> + } else if (S_ISDIR(mode)) {
> + if (show_recursive(base->buf, base->len, pathname)) {
> + *retval = READ_TREE_RECURSIVE;
> + if (!(ls_options & LS_SHOW_TREES))
> + return 1;
> + }
> + *type = OBJ_TREE;
> + }
> + else if (ls_options & LS_TREE_ONLY)
> + return 1;
> + return 0;
> +}
> +
> +static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
> + const char *pathname, unsigned mode, void *context)
> +{
> + size_t baselen;
> + int retval = 0;
> + struct strbuf line = STRBUF_INIT;
> + struct shown_data data = {
> + .mode = mode,
> + .type = OBJ_BLOB,
> + .oid = oid,
> + .pathname = pathname,
> + .base = base,
> + };
> +
> + if (show_tree_init(&data.type, base, pathname, mode, &retval))
> + return retval;
> +
> + baselen = base->len;
> + strbuf_expand(&line, format, expand_show_tree, &data);
> + strbuf_addch(&line, line_termination);
> + fwrite(line.buf, line.len, 1, stdout);
> + strbuf_setlen(base, baselen);
> + return retval;
> +}
> +
> +static int parse_shown_fields(void)
> +{
> + if (cmdmode == MODE_NAME_ONLY ||
> + (format && !strcmp(format, name_only_format))) {
> + shown_bits = SHOW_FILE_NAME;
> + return 1;
> + }
> +
> + if (cmdmode == MODE_OBJECT_ONLY ||
> + (format && !strcmp(format, object_only_format))) {
> + shown_bits = SHOW_OBJECT_NAME;
> + return 1;
> + }
> +
> + if (!ls_options || (ls_options & LS_RECURSIVE)
> + || (ls_options & LS_SHOW_TREES)
> + || (ls_options & LS_TREE_ONLY)
> + || (format && !strcmp(format, default_format)))
> + shown_bits = SHOW_DEFAULT;
> +
> + if (cmdmode == MODE_LONG ||
> + (format && !strcmp(format, long_format)))
> + shown_bits = SHOW_DEFAULT | SHOW_SIZE;
> + return 1;
> +}
> +
> static int show_default(struct shown_data *data)
> {
> size_t baselen = data->base->len;
> @@ -137,24 +258,6 @@ static int show_default(struct shown_data *data)
> return 1;
> }
>
> -static int show_tree_init(enum object_type *type, struct strbuf *base,
> - const char *pathname, unsigned mode, int *retval)
> -{
> - if (S_ISGITLINK(mode)) {
> - *type = OBJ_COMMIT;
> - } else if (S_ISDIR(mode)) {
> - if (show_recursive(base->buf, base->len, pathname)) {
> - *retval = READ_TREE_RECURSIVE;
> - if (!(ls_options & LS_SHOW_TREES))
> - return 1;
> - }
> - *type = OBJ_TREE;
> - }
> - else if (ls_options & LS_TREE_ONLY)
> - return 1;
> - return 0;
> -}
> -
> static int show_tree(const struct object_id *oid, struct strbuf *base,
> const char *pathname, unsigned mode, void *context)
> {
> @@ -196,6 +299,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
> struct object_id oid;
> struct tree *tree;
> int i, full_tree = 0;
> + read_tree_fn_t fn = show_tree;
> const struct option ls_tree_options[] = {
> OPT_BIT('d', NULL, &ls_options, N_("only show trees"),
> LS_TREE_ONLY),
> @@ -218,6 +322,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
> OPT_BOOL(0, "full-tree", &full_tree,
> N_("list entire tree; not just current directory "
> "(implies --full-name)")),
> + OPT_STRING_F(0, "format", &format, N_("format"),
> + N_("format to use for the output"),
> + PARSE_OPT_NONEG),
> OPT__ABBREV(&abbrev),
> OPT_END()
> };
> @@ -238,6 +345,10 @@ 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 (format && cmdmode)
> + usage_msg_opt(
> + _("--format can't be combined with other format-altering options"),
> + ls_tree_usage, ls_tree_options);
> if (argc < 1)
> usage_with_options(ls_tree_usage, ls_tree_options);
> if (get_oid(argv[0], &oid))
> @@ -261,6 +372,18 @@ 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);
> +
> + /*
> + * The generic show_tree_fmt() is slower than show_tree(), so
> + * take the fast path if possible.
> + */
> + if (format && (!strcmp(format, default_format) ||
> + !strcmp(format, long_format) ||
> + !strcmp(format, name_only_format) ||
> + !strcmp(format, object_only_format)))
> + fn = show_tree;
> + else if (format)
> + fn = show_tree_fmt;
> +
> + return !!read_tree(the_repository, tree, &pathspec, fn, NULL);
> }
> diff --git a/t/t3105-ls-tree-format.sh b/t/t3105-ls-tree-format.sh
> new file mode 100755
> index 0000000000..92b4d240e8
> --- /dev/null
> +++ b/t/t3105-ls-tree-format.sh
> @@ -0,0 +1,55 @@
> +#!/bin/sh
> +
> +test_description='ls-tree --format'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> +. ./test-lib.sh
> +
> +test_expect_success 'ls-tree --format usage' '
> + test_expect_code 129 git ls-tree --format=fmt -l &&
> + test_expect_code 129 git ls-tree --format=fmt --name-only &&
> + test_expect_code 129 git ls-tree --format=fmt --name-status &&
> + test_expect_code 129 git ls-tree --format=fmt --object-only
> +'
> +
> +test_expect_success 'setup' '
> + mkdir dir &&
> + test_commit dir/sub-file &&
> + test_commit top-file
> +'
> +
> +test_ls_tree_format () {
> + format=$1 &&
> + opts=$2 &&
> + shift 2 &&
> + git ls-tree $opts -r HEAD >expect.raw &&
> + sed "s/^/> /" >expect <expect.raw &&
> + git ls-tree --format="> $format" -r HEAD >actual &&
> + test_cmp expect actual
> +}
> +
> +test_expect_success 'ls-tree --format=<default-like>' '
> + test_ls_tree_format \
> + "%(mode) %(type) %(object)%x09%(file)" \
> + ""
> +'
> +
> +test_expect_success 'ls-tree --format=<long-like>' '
> + test_ls_tree_format \
> + "%(mode) %(type) %(object) %(size:padded)%x09%(file)" \
> + "--long"
> +'
> +
> +test_expect_success 'ls-tree --format=<name-only-like>' '
> + test_ls_tree_format \
> + "%(file)" \
> + "--name-only"
> +'
> +
> +test_expect_success 'ls-tree --format=<object-only-like>' '
> + test_ls_tree_format \
> + "%(object)" \
> + "--object-only"
> +'
> +
> +test_done
> --
> 2.33.0.rc1.1802.gbb1c3936fb.dirty
>
>
>
