Diff from v12: Current patch is based by the review patch of Ævar Arnfjörð Bjarmason in [1]. In terms of the review commits, I replied Ævar Arnfjörð Bjarmason [2] and listed the actions, therefore this v13 patch was implemented by these actions. Links: 1. https://public-inbox.org/git/cover-0.7-00000000000-20220310T134811Z-avarab@xxxxxxxxx/ 2. https://public-inbox.org/git/20220317095129.86790-1-dyroneteng@xxxxxxxxx/ Thanks. Johannes Schindelin (1): cocci: allow padding with `strbuf_addf()` Teng Long (5): ls-tree: rename "retval" to "recurse" in "show_tree()" ls-tree: simplify nesting if/else logic in "show_tree()" ls-tree: fix "--name-only" and "--long" combined use bug ls-tree: slightly refactor `show_tree()` ls-tree: support --object-only option for "git-ls-tree" Ævar Arnfjörð Bjarmason (10): ls-tree: remove commented-out code ls-tree: add missing braces to "else" arms ls-tree: use "enum object_type", not {blob,tree,commit}_type ls-tree: use "size_t", not "int" for "struct strbuf"'s "len" ls-tree: introduce struct "show_tree_data" ls-tree: introduce "--format" option ls-tree tests: add tests for --name-status ls-tree: detect and error on --name-only --name-status ls-tree: remove FIELD_*, just use MODE_* ls-tree: split up "fast path" callbacks Documentation/git-ls-tree.txt | 68 +++++- builtin/ls-tree.c | 367 ++++++++++++++++++++++++++------ contrib/coccinelle/strbuf.cocci | 6 +- t/t3101-ls-tree-dirname.sh | 55 ++--- t/t3103-ls-tree-misc.sh | 15 ++ t/t3104-ls-tree-format.sh | 76 +++++++ 6 files changed, 493 insertions(+), 94 deletions(-) create mode 100755 t/t3104-ls-tree-format.sh Range-diff against v12: -: ---------- > 1: 2fcff7e0d4 ls-tree: remove commented-out code -: ---------- > 2: 6fd1dd9383 ls-tree: add missing braces to "else" arms -: ---------- > 3: 208654b5e2 ls-tree: use "enum object_type", not {blob,tree,commit}_type -: ---------- > 4: 2637464fd8 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len" -: ---------- > 5: 99e6d47108 ls-tree: rename "retval" to "recurse" in "show_tree()" -: ---------- > 6: a8d9b78dea ls-tree: simplify nesting if/else logic in "show_tree()" -: ---------- > 7: 25a07e048f ls-tree: fix "--name-only" and "--long" combined use bug 1: f449146b4d ! 8: 55f1e10d7e ls-tree: slightly refactor `show_tree()` @@ builtin/ls-tree.c -#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) static int abbrev; static int ls_options; static struct pathspec pathspec; @@ builtin/ls-tree.c NULL }; -+static enum mutx_option { -+ MODE_UNSPECIFIED = 0, ++static enum ls_tree_cmdmode { ++ MODE_LONG = 1, + MODE_NAME_ONLY, -+ MODE_LONG, +} cmdmode; + +static int parse_shown_fields(void) 2: 80311adc7c ! 9: 85d0b0da1d ls-tree: introduce struct "show_tree_data" @@ builtin/ls-tree.c: static unsigned int shown_fields; static const char * const ls_tree_usage[] = { N_("git ls-tree [<options>] <tree-ish> [<path>...]"), NULL -@@ builtin/ls-tree.c: static enum mutx_option { - MODE_LONG, +@@ builtin/ls-tree.c: static enum ls_tree_cmdmode { + MODE_NAME_ONLY, } cmdmode; -static int parse_shown_fields(void) 3: 459080f549 = 10: eddcf903ad cocci: allow padding with `strbuf_addf()` 4: cb717d08be ! 11: 7451242daa ls-tree: introduce "--format" option @@ Commit message idea and suggestion, this commit makes modifications in terms of the original discussion on community [1]. + In [1] there was a "GIT_TEST_LS_TREE_FORMAT_BACKEND" variable to + ensure that we had test coverage for passing tests that would + otherwise use show_tree() through show_tree_fmt(), and thus that the + formatting mechanism could handle all the same cases as the + non-formatting options. + + Somewhere in subsequent re-rolls of that we seem to have drifted away + from what the goal of these tests should be. We're trying to ensure + correctness of show_tree_fmt(). We can't tell if we "hit [the] + fast-path" here, and instead of having an explicit test for that, we + can just add it to something our "test_ls_tree_format" tests for. + Here is the statistics about performance tests: 1. Default format (hitten the builtin formats): @@ Commit message Links: [1] https://public-inbox.org/git/RFC-patch-6.7-eac299f06ff-20211217T131635Z-avarab@xxxxxxxxx/ + [2] https://lore.kernel.org/git/cb717d08be87e3239117c6c667cb32caabaad33d.1646390152.git.dyroneteng@xxxxxxxxx/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx> @@ builtin/ls-tree.c: static const char *ls_tree_prefix; struct show_tree_data { unsigned mode; enum object_type type; -@@ builtin/ls-tree.c: static enum mutx_option { - MODE_LONG, +@@ builtin/ls-tree.c: static enum ls_tree_cmdmode { + MODE_NAME_ONLY, } cmdmode; +static void expand_objectsize(struct strbuf *line, const struct object_id *oid, @@ t/t3104-ls-tree-format.sh (new) + opts=$2 && + fmtopts=$3 && + shift 2 && -+ git ls-tree $opts -r HEAD >expect.raw && -+ sed "s/^/> /" >expect <expect.raw && -+ git ls-tree --format="> $format" -r $fmtopts HEAD >actual && -+ test_cmp expect actual ++ ++ test_expect_success "ls-tree '--format=<$format>' is like options '$opts $fmtopts'" ' ++ git ls-tree $opts -r HEAD >expect && ++ git ls-tree --format="$format" -r $fmtopts HEAD >actual && ++ test_cmp expect actual ++ ' ++ ++ test_expect_success "ls-tree '--format=<$format>' on optimized v.s. non-optimized path" ' ++ git ls-tree --format="$format" -r $fmtopts HEAD >expect && ++ git ls-tree --format="> $format" -r $fmtopts HEAD >actual.raw && ++ sed "s/^> //" >actual <actual.raw && ++ test_cmp expect actual ++ ' +} + -+test_expect_success 'ls-tree --format=<default-like>' ' -+ test_ls_tree_format \ -+ "%(objectmode) %(objecttype) %(objectname)%x09%(path)" \ -+ "" -+' ++test_ls_tree_format \ ++ "%(objectmode) %(objecttype) %(objectname)%x09%(path)" \ ++ "" + -+test_expect_success 'ls-tree --format=<long-like>' ' -+ test_ls_tree_format \ -+ "%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)" \ -+ "--long" -+' ++test_ls_tree_format \ ++ "%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)" \ ++ "--long" + -+test_expect_success 'ls-tree --format=<name-only-like>' ' -+ test_ls_tree_format \ -+ "%(path)" \ -+ "--name-only" -+' ++test_ls_tree_format \ ++ "%(path)" \ ++ "--name-only" + -+test_expect_success 'ls-tree combine --format=<default-like> and -t' ' -+ test_ls_tree_format \ ++test_ls_tree_format \ + "%(objectmode) %(objecttype) %(objectname)%x09%(path)" \ + "-t" \ + "-t" -+' + -+test_expect_success 'ls-tree combine --format=<default-like> and --full-name' ' -+ test_ls_tree_format \ ++test_ls_tree_format \ + "%(objectmode) %(objecttype) %(objectname)%x09%(path)" \ + "--full-name" \ + "--full-name" -+' + -+test_expect_success 'ls-tree combine --format=<default-like> and --full-tree' ' -+ test_ls_tree_format \ ++test_ls_tree_format \ + "%(objectmode) %(objecttype) %(objectname)%x09%(path)" \ + "--full-tree" \ + "--full-tree" -+' + -+test_expect_success 'ls-tree hit fast-path with --format=<default-like>' ' -+ git ls-tree -r HEAD >expect && -+ git ls-tree --format="%(objectmode) %(objecttype) %(objectname)%x09%(path)" -r HEAD >actual && -+ test_cmp expect actual -+' -+ -+test_expect_success 'ls-tree hit fast-path with --format=<name-only-like>' ' -+ git ls-tree -r --name-only HEAD >expect && -+ git ls-tree --format="%(path)" -r HEAD >actual && -+ test_cmp expect actual -+' +test_done 5: 55b1c4379d ! 12: 2bcd08ebd0 ls-tree: support --object-only option for "git-ls-tree" @@ Documentation/git-ls-tree.txt: OPTIONS Instead of showing the full 40-byte hexadecimal object ## builtin/ls-tree.c ## -@@ builtin/ls-tree.c: static int line_termination = '\n'; - #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; @@ builtin/ls-tree.c: static const char *format; static const char *default_format = "%(objectmode) %(objecttype) %(objectname)%x09%(path)"; static const char *long_format = "%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)"; @@ builtin/ls-tree.c: static const char *format; unsigned mode; enum object_type type; @@ builtin/ls-tree.c: static const char * const ls_tree_usage[] = { - static enum mutx_option { - MODE_UNSPECIFIED = 0, + static enum ls_tree_cmdmode { + MODE_LONG = 1, MODE_NAME_ONLY, + MODE_OBJECT_ONLY, - MODE_LONG, } cmdmode; + static void expand_objectsize(struct strbuf *line, const struct object_id *oid, @@ builtin/ls-tree.c: static int parse_shown_fields(unsigned int *shown_fields) *shown_fields = FIELD_PATH_NAME; return 0; @@ t/t3103-ls-tree-misc.sh: test_expect_success 'ls-tree fails with non-zero exit c test_done ## t/t3104-ls-tree-format.sh ## -@@ t/t3104-ls-tree-format.sh: test_expect_success 'ls-tree --format=<name-only-like>' ' - "--name-only" - ' +@@ t/t3104-ls-tree-format.sh: test_ls_tree_format \ + "%(path)" \ + "--name-only" -+test_expect_success 'ls-tree --format=<object-only-like>' ' -+ test_ls_tree_format \ -+ "%(objectname)" \ -+ "--object-only" -+' ++test_ls_tree_format \ ++ "%(objectname)" \ ++ "--object-only" + -+test_expect_success 'ls-tree --format=<object-only-like> --abbrev' ' -+ test_ls_tree_format \ -+ "%(objectname)" \ -+ "--object-only --abbrev" \ -+ "--abbrev" -+' ++test_ls_tree_format \ ++ "%(objectname)" \ ++ "--object-only --abbrev" \ ++ "--abbrev" + - test_expect_success 'ls-tree combine --format=<default-like> and -t' ' - test_ls_tree_format \ + test_ls_tree_format \ "%(objectmode) %(objecttype) %(objectname)%x09%(path)" \ -@@ t/t3104-ls-tree-format.sh: test_expect_success 'ls-tree hit fast-path with --format=<name-only-like>' ' - git ls-tree --format="%(path)" -r HEAD >actual && - test_cmp expect actual - ' -+ -+test_expect_success 'ls-tree hit fast-path with --format=<object-only-like>' ' -+ git ls-tree -r --object-only HEAD >expect && -+ git ls-tree --format="%(objectname)" -r HEAD >actual && -+ test_cmp expect actual -+' - test_done + "-t" \ -: ---------- > 13: a5c73de057 ls-tree tests: add tests for --name-status -: ---------- > 14: 89402a8518 ls-tree: detect and error on --name-only --name-status -: ---------- > 15: b8afca193a ls-tree: remove FIELD_*, just use MODE_* -: ---------- > 16: 010e3c0ece ls-tree: split up "fast path" callbacks -- 2.34.1.406.g2e0e55130e
