Hi Teng,

On Sat, 1 Jan 2022, Teng Long wrote:

quoted

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, &quoted, NULL, 0);
-               strbuf_addstr(line, quoted.buf);
+               strbuf_addbuf(line, &quoted);
        } 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

quoted

+     } 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, &quoted, 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