[PATCH v13 00/16] ls-tree: "--object-only" and "--format" opts
- From
Teng Long <dyroneteng@gmail.com>
- Date
- Mar 21, 2022, 07:33 UTC
- Message-ID
- <cover.1647846935.git.dyroneteng@gmail.com>
- In-Reply-To
- <cover.1646390152.git.dyroneteng@gmail.com>
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@gmail.com/ 2. https://public-inbox.org/git/20220317095129.86790-1-dyroneteng@gmail.com/
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" callbacksDocumentation/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@gmail.com/
+ [2] https://lore.kernel.org/git/cb717d08be87e3239117c6c667cb32caabaad33d.1646390152.git.dyroneteng@gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
@@ 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