* [PATCH 0/2] Migrate ls-tree to parse-opts @ 2009-11-14 4:34 Stephen Boyd 2009-11-14 4:34 ` [PATCH 1/2] t3101: test more ls-tree options Stephen Boyd 2009-11-14 4:34 ` [PATCH 2/2] ls-tree: migrate to parse-options Stephen Boyd 0 siblings, 2 replies; 10+ messages in thread From: Stephen Boyd @ 2009-11-14 4:34 UTC (permalink / raw) To: git; +Cc: Junio C Hamano This looked pretty straightforward. I guess --name-status is there for backward compatibility. Should it be there anymore? Stephen Boyd (2): t3101: test more ls-tree options ls-tree: migrate to parse-options builtin-ls-tree.c | 100 +++++++++++++++++-------------------------- t/t3101-ls-tree-dirname.sh | 89 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 62 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] t3101: test more ls-tree options 2009-11-14 4:34 [PATCH 0/2] Migrate ls-tree to parse-opts Stephen Boyd @ 2009-11-14 4:34 ` Stephen Boyd 2009-11-17 6:21 ` Junio C Hamano 2009-11-14 4:34 ` [PATCH 2/2] ls-tree: migrate to parse-options Stephen Boyd 1 sibling, 1 reply; 10+ messages in thread From: Stephen Boyd @ 2009-11-14 4:34 UTC (permalink / raw) To: git; +Cc: Junio C Hamano Add tests for --full-name, --full-tree, --abbrev, and --name-only. Signed-off-by: Stephen Boyd <bebarino@gmail.com> --- t/t3101-ls-tree-dirname.sh | 89 +++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 87 insertions(+), 2 deletions(-) diff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh index 51cb4a3..99458e4 100755 --- a/t/t3101-ls-tree-dirname.sh +++ b/t/t3101-ls-tree-dirname.sh @@ -39,8 +39,8 @@ test_expect_success \ tree=`git write-tree` && echo $tree' -_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' -_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" +_x5='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' +_x40="$_x5$_x5$_x5$_x5$_x5$_x5$_x5$_x5" test_output () { sed -e "s/ $_x40 / X /" <current >check test_cmp expected check @@ -141,4 +141,89 @@ test_expect_success 'ls-tree filter is leading path match' ' test_output ' +test_expect_success 'ls-tree --full-name' ' + ( + cd path0 && + git ls-tree --full-name $tree a + ) > current && + cat >expected <<\EOF && +040000 tree X path0/a +EOF + test_output +' + +test_expect_success 'ls-tree --full-tree' ' + ( + cd path1/b/c && + git ls-tree --full-tree $tree + ) > current && + cat >expected <<\EOF && +100644 blob X 1.txt +100644 blob X 2.txt +040000 tree X path0 +040000 tree X path1 +040000 tree X path2 +040000 tree X path3 +EOF + test_output +' + +test_expect_success 'ls-tree --full-tree -r' ' + ( + cd path3/ && + git ls-tree --full-tree -r $tree + ) > current && + cat >expected <<\EOF && +100644 blob X 1.txt +100644 blob X 2.txt +100644 blob X path0/a/b/c/1.txt +100644 blob X path1/b/c/1.txt +100644 blob X path2/1.txt +100644 blob X path3/1.txt +100644 blob X path3/2.txt +EOF + test_output +' + +test_expect_success 'ls-tree --abbrev=5' ' + git ls-tree --abbrev=5 $tree > current.abbrev && + sed -e "s/ $_x5 / X /" < current.abbrev > current && + cat >expected <<\EOF && +100644 blob X 1.txt +100644 blob X 2.txt +040000 tree X path0 +040000 tree X path1 +040000 tree X path2 +040000 tree X path3 +EOF + test_output +' + +test_expect_success 'ls-tree --name-only' ' + git ls-tree --name-only $tree > current + cat >expected <<\EOF && +1.txt +2.txt +path0 +path1 +path2 +path3 +EOF + test_output +' + +test_expect_success 'ls-tree --name-only -r' ' + git ls-tree --name-only -r $tree > current + cat >expected <<\EOF && +1.txt +2.txt +path0/a/b/c/1.txt +path1/b/c/1.txt +path2/1.txt +path3/1.txt +path3/2.txt +EOF + test_output +' + test_done -- 1.6.5.2.155.gbb47 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] t3101: test more ls-tree options 2009-11-14 4:34 ` [PATCH 1/2] t3101: test more ls-tree options Stephen Boyd @ 2009-11-17 6:21 ` Junio C Hamano 2009-11-17 6:26 ` Stephen Boyd 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2009-11-17 6:21 UTC (permalink / raw) To: Stephen Boyd; +Cc: git Thanks. I'd squash this patch in for the following reasons; Ok? - The --abbrev=n option only guarantees that at least n hexdigits are used but the implementation is allowed to use more to avoid ambiguities. - The point of test_output function is that it does its own filtering before comparison; it is misleading to feed it if the caller munges the input. - Redirection should be immediately followed by filename; this is just a matter of personal taste, but being consistent in the new code is better than resulting in blocks of lines with mixed styles (no, I won't welcome a patch to fix the existing "cmd > file", unless it is done as part of a patch aka "while at it" that fixes something of substance nearby at the same time). t/t3101-ls-tree-dirname.sh | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-) diff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh index 99458e4..6ef371a 100755 --- a/t/t3101-ls-tree-dirname.sh +++ b/t/t3101-ls-tree-dirname.sh @@ -39,8 +39,8 @@ test_expect_success \ tree=`git write-tree` && echo $tree' -_x5='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' -_x40="$_x5$_x5$_x5$_x5$_x5$_x5$_x5$_x5" +_x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' +_x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x05$_x05" test_output () { sed -e "s/ $_x40 / X /" <current >check test_cmp expected check @@ -145,7 +145,7 @@ test_expect_success 'ls-tree --full-name' ' ( cd path0 && git ls-tree --full-name $tree a - ) > current && + ) >current && cat >expected <<\EOF && 040000 tree X path0/a EOF @@ -156,7 +156,7 @@ test_expect_success 'ls-tree --full-tree' ' ( cd path1/b/c && git ls-tree --full-tree $tree - ) > current && + ) >current && cat >expected <<\EOF && 100644 blob X 1.txt 100644 blob X 2.txt @@ -172,7 +172,7 @@ test_expect_success 'ls-tree --full-tree -r' ' ( cd path3/ && git ls-tree --full-tree -r $tree - ) > current && + ) >current && cat >expected <<\EOF && 100644 blob X 1.txt 100644 blob X 2.txt @@ -186,8 +186,8 @@ EOF ' test_expect_success 'ls-tree --abbrev=5' ' - git ls-tree --abbrev=5 $tree > current.abbrev && - sed -e "s/ $_x5 / X /" < current.abbrev > current && + git ls-tree --abbrev=5 $tree >current && + sed -e "s/ $_x05[0-9a-f]* / X /" <current >check && cat >expected <<\EOF && 100644 blob X 1.txt 100644 blob X 2.txt @@ -196,11 +196,11 @@ test_expect_success 'ls-tree --abbrev=5' ' 040000 tree X path2 040000 tree X path3 EOF - test_output + test_cmp expected check ' test_expect_success 'ls-tree --name-only' ' - git ls-tree --name-only $tree > current + git ls-tree --name-only $tree >current cat >expected <<\EOF && 1.txt 2.txt @@ -213,7 +213,7 @@ EOF ' test_expect_success 'ls-tree --name-only -r' ' - git ls-tree --name-only -r $tree > current + git ls-tree --name-only -r $tree >current cat >expected <<\EOF && 1.txt 2.txt -- 1.6.5.3.283.g4b054 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] t3101: test more ls-tree options 2009-11-17 6:21 ` Junio C Hamano @ 2009-11-17 6:26 ` Stephen Boyd 0 siblings, 0 replies; 10+ messages in thread From: Stephen Boyd @ 2009-11-17 6:26 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Junio C Hamano wrote: > Thanks. I'd squash this patch in for the following reasons; Ok? Looks good. Thanks. I figured reusing the test_output function would be a bad idea. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ls-tree: migrate to parse-options 2009-11-14 4:34 [PATCH 0/2] Migrate ls-tree to parse-opts Stephen Boyd 2009-11-14 4:34 ` [PATCH 1/2] t3101: test more ls-tree options Stephen Boyd @ 2009-11-14 4:34 ` Stephen Boyd 2009-11-17 6:22 ` Junio C Hamano 1 sibling, 1 reply; 10+ messages in thread From: Stephen Boyd @ 2009-11-14 4:34 UTC (permalink / raw) To: git; +Cc: Junio C Hamano Signed-off-by: Stephen Boyd <bebarino@gmail.com> --- builtin-ls-tree.c | 100 +++++++++++++++++++++------------------------------- 1 files changed, 40 insertions(+), 60 deletions(-) diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c index 22008df..04408df 100644 --- a/builtin-ls-tree.c +++ b/builtin-ls-tree.c @@ -9,6 +9,7 @@ #include "commit.h" #include "quote.h" #include "builtin.h" +#include "parse-options.h" static int line_termination = '\n'; #define LS_RECURSIVE 1 @@ -22,8 +23,10 @@ static const char **pathspec; static int chomp_prefix; static const char *ls_tree_prefix; -static const char ls_tree_usage[] = - "git ls-tree [-d] [-r] [-t] [-l] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]"; +static const char * const ls_tree_usage[] = { + "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]", + NULL +}; static int show_recursive(const char *base, int baselen, const char *pathname) { @@ -117,76 +120,53 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) { unsigned char sha1[20]; struct tree *tree; + int full_tree = 0; + const struct option ls_tree_options[] = { + OPT_BIT('d', NULL, &ls_options, "only show trees", + LS_TREE_ONLY), + OPT_BIT('r', NULL, &ls_options, "recurse into subtrees", + LS_RECURSIVE), + OPT_BIT('t', NULL, &ls_options, "show trees when recursing", + LS_SHOW_TREES), + OPT_SET_INT('z', NULL, &line_termination, + "terminate entries with NUL byte", 0), + OPT_BIT('l', "long", &ls_options, "include object size", + LS_SHOW_SIZE), + OPT_BIT(0, "name-only", &ls_options, "list only filenames", + LS_NAME_ONLY), + OPT_BIT(0, "name-status", &ls_options, "list only filenames", + LS_NAME_ONLY), + OPT_SET_INT(0, "full-name", &chomp_prefix, + "use full path names", 0), + OPT_BOOLEAN(0, "full-tree", &full_tree, + "list entire tree; not just current directory " + "(implies --full-name)"), + OPT__ABBREV(&abbrev), + OPT_END() + }; git_config(git_default_config, NULL); ls_tree_prefix = prefix; if (prefix && *prefix) chomp_prefix = strlen(prefix); - while (1 < argc && argv[1][0] == '-') { - switch (argv[1][1]) { - case 'z': - line_termination = 0; - break; - case 'r': - ls_options |= LS_RECURSIVE; - break; - case 'd': - ls_options |= LS_TREE_ONLY; - break; - case 't': - ls_options |= LS_SHOW_TREES; - break; - case 'l': - ls_options |= LS_SHOW_SIZE; - break; - case '-': - if (!strcmp(argv[1]+2, "name-only") || - !strcmp(argv[1]+2, "name-status")) { - ls_options |= LS_NAME_ONLY; - break; - } - if (!strcmp(argv[1]+2, "long")) { - ls_options |= LS_SHOW_SIZE; - break; - } - if (!strcmp(argv[1]+2, "full-name")) { - chomp_prefix = 0; - break; - } - if (!strcmp(argv[1]+2, "full-tree")) { - ls_tree_prefix = prefix = NULL; - chomp_prefix = 0; - break; - } - if (!prefixcmp(argv[1]+2, "abbrev=")) { - abbrev = strtoul(argv[1]+9, NULL, 10); - if (abbrev && abbrev < MINIMUM_ABBREV) - abbrev = MINIMUM_ABBREV; - else if (abbrev > 40) - abbrev = 40; - break; - } - if (!strcmp(argv[1]+2, "abbrev")) { - abbrev = DEFAULT_ABBREV; - break; - } - /* otherwise fallthru */ - default: - usage(ls_tree_usage); - } - argc--; argv++; + + argc = parse_options(argc, argv, prefix, ls_tree_options, + ls_tree_usage, 0); + if (full_tree) { + ls_tree_prefix = prefix = NULL; + chomp_prefix = 0; } /* -d -r should imply -t, but -d by itself should not have to. */ if ( (LS_TREE_ONLY|LS_RECURSIVE) == ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options)) ls_options |= LS_SHOW_TREES; - if (argc < 2) - usage(ls_tree_usage); - if (get_sha1(argv[1], sha1)) - die("Not a valid object name %s", argv[1]); + if (argc < 1) + usage_with_options(ls_tree_usage, ls_tree_options); + if (get_sha1(argv[0], sha1)) + die("Not a valid object name %s", argv[0]); - pathspec = get_pathspec(prefix, argv + 2); + pathspec = get_pathspec(prefix, argv + 1); tree = parse_tree_indirect(sha1); if (!tree) die("not a tree object"); -- 1.6.5.2.155.gbb47 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ls-tree: migrate to parse-options 2009-11-14 4:34 ` [PATCH 2/2] ls-tree: migrate to parse-options Stephen Boyd @ 2009-11-17 6:22 ` Junio C Hamano 2009-11-17 6:28 ` Stephen Boyd 2009-11-17 7:02 ` Stephen Boyd 0 siblings, 2 replies; 10+ messages in thread From: Junio C Hamano @ 2009-11-17 6:22 UTC (permalink / raw) To: Stephen Boyd; +Cc: git Thanks. As parse-options infrastructure gives much better per-option help text, there is not much point to keep the list of options that can go stale in the usage text. I'd squash this to yours. Ok? --- builtin-ls-tree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c index 04408df..14a913a 100644 --- a/builtin-ls-tree.c +++ b/builtin-ls-tree.c @@ -24,7 +24,7 @@ static int chomp_prefix; static const char *ls_tree_prefix; static const char * const ls_tree_usage[] = { - "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]", + "git ls-tree <options> <tree-ish> [path...]", NULL }; -- 1.6.5.3.283.g4b054 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ls-tree: migrate to parse-options 2009-11-17 6:22 ` Junio C Hamano @ 2009-11-17 6:28 ` Stephen Boyd 2009-11-17 7:02 ` Stephen Boyd 1 sibling, 0 replies; 10+ messages in thread From: Stephen Boyd @ 2009-11-17 6:28 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Junio C Hamano wrote: > Thanks. > > As parse-options infrastructure gives much better per-option help text, > there is not much point to keep the list of options that can go stale > in the usage text. > > I'd squash this to yours. Ok? > I like this style too. Less usage update patches are better. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ls-tree: migrate to parse-options 2009-11-17 6:22 ` Junio C Hamano 2009-11-17 6:28 ` Stephen Boyd @ 2009-11-17 7:02 ` Stephen Boyd 2009-11-18 22:19 ` Junio C Hamano 1 sibling, 1 reply; 10+ messages in thread From: Stephen Boyd @ 2009-11-17 7:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Junio C Hamano wrote: > > @@ -24,7 +24,7 @@ static int chomp_prefix; > static const char *ls_tree_prefix; > > static const char * const ls_tree_usage[] = { > - "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]", > + "git ls-tree <options> <tree-ish> [path...]", > NULL > }; > Is it [<options>] or [<options>...] or <options> or even <options>... though? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ls-tree: migrate to parse-options 2009-11-17 7:02 ` Stephen Boyd @ 2009-11-18 22:19 ` Junio C Hamano 2009-11-18 23:10 ` Thiago Farina 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2009-11-18 22:19 UTC (permalink / raw) To: Stephen Boyd; +Cc: git Stephen Boyd <bebarino@gmail.com> writes: > Junio C Hamano wrote: >> >> @@ -24,7 +24,7 @@ static int chomp_prefix; >> static const char *ls_tree_prefix; >> static const char * const ls_tree_usage[] = { >> - "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]", >> + "git ls-tree <options> <tree-ish> [path...]", >> NULL >> }; >> > > Is it [<options>] or [<options>...] or <options> or even > <options>... though? Output from "git grep -e '<option' -- '*.c'" indicates that it should be "[<options>]"; thanks for spotting. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ls-tree: migrate to parse-options 2009-11-18 22:19 ` Junio C Hamano @ 2009-11-18 23:10 ` Thiago Farina 0 siblings, 0 replies; 10+ messages in thread From: Thiago Farina @ 2009-11-18 23:10 UTC (permalink / raw) To: Junio C Hamano; +Cc: Stephen Boyd, git On Wed, Nov 18, 2009 at 8:19 PM, Junio C Hamano <gitster@pobox.com> wrote: > Stephen Boyd <bebarino@gmail.com> writes: > >> Junio C Hamano wrote: >>> >>> @@ -24,7 +24,7 @@ static int chomp_prefix; >>> static const char *ls_tree_prefix; >>> static const char * const ls_tree_usage[] = { >>> - "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]", >>> + "git ls-tree <options> <tree-ish> [path...]", >>> NULL >>> }; >>> >> >> Is it [<options>] or [<options>...] or <options> or even >> <options>... though? > > Output from "git grep -e '<option' -- '*.c'" indicates that it > should be "[<options>]"; thanks for spotting. $ git grep -e '<option' -- '*.c' | wc -l 4 $ git grep -e '\[options' -- '*.c' | wc -l 42 $ git grep -e '\[<options' -- '*.c' | wc -l 2 Shouldn't be just "[options]"? ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-11-18 23:10 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-11-14 4:34 [PATCH 0/2] Migrate ls-tree to parse-opts Stephen Boyd 2009-11-14 4:34 ` [PATCH 1/2] t3101: test more ls-tree options Stephen Boyd 2009-11-17 6:21 ` Junio C Hamano 2009-11-17 6:26 ` Stephen Boyd 2009-11-14 4:34 ` [PATCH 2/2] ls-tree: migrate to parse-options Stephen Boyd 2009-11-17 6:22 ` Junio C Hamano 2009-11-17 6:28 ` Stephen Boyd 2009-11-17 7:02 ` Stephen Boyd 2009-11-18 22:19 ` Junio C Hamano 2009-11-18 23:10 ` Thiago Farina
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).