Merge branch 'ab/cat-file'

Assorted updates to "git cat-file", especially "-h".

* ab/cat-file:
  cat-file: s/_/-/ in typo'd usage_msg_optf() message
  cat-file: don't whitespace-pad "(...)" in SYNOPSIS and usage output
  cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters)
  object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY
  cat-file: correct and improve usage information
  cat-file: fix remaining usage bugs
  cat-file: make --batch-all-objects a CMDMODE
  cat-file: move "usage" variable to cmd_cat_file()
  cat-file docs: fix SYNOPSIS and "-h" output
  parse-options API: add a usage_msg_optf()
  cat-file tests: test messaging on bad objects/paths
  cat-file tests: test bad usage

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 27b27e2b300c49bb07f348291a8b04af3d128f30..bef76f4dd060dd7fdd4efee44f53b34a3c546c13 100644 (file)
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,8 +9,14 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p | <type> | --textconv | --filters ) [--path=<path>] <object>
-'git cat-file' (--batch[=<format>] | --batch-check[=<format>]) [ --textconv | --filters ] [--follow-symlinks]
+'git cat-file' <type> <object>
+'git cat-file' (-e | -p) <object>
+'git cat-file' (-t | -s) [--allow-unknown-type] <object>
+'git cat-file' (--batch | --batch-check) [--batch-all-objects]
+            [--buffer] [--follow-symlinks] [--unordered]
+            [--textconv | --filters]
+'git cat-file' (--textconv | --filters)
+            [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]
 
 DESCRIPTION
 -----------

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index d94050e6c188ff4594a065da87695c67c560ac94..7b3f42950ec88e56e2eac6ac31910f983501a399 100644 (file)
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -73,14 +73,17 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
        struct object_info oi = OBJECT_INFO_INIT;
        struct strbuf sb = STRBUF_INIT;
        unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
+       unsigned get_oid_flags = GET_OID_RECORD_PATH | GET_OID_ONLY_TO_DIE;
        const char *path = force_path;
+       const int opt_cw = (opt == 'c' || opt == 'w');
+       if (!path && opt_cw)
+               get_oid_flags |= GET_OID_REQUIRE_PATH;
 
        if (unknown_type)
                flags |= OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
 
-       if (get_oid_with_context(the_repository, obj_name,
-                                GET_OID_RECORD_PATH,
-                                &oid, &obj_context))
+       if (get_oid_with_context(the_repository, obj_name, get_oid_flags, &oid,
+                                &obj_context))
                die("Not a valid object name %s", obj_name);
 
        if (!path)
@@ -112,9 +115,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                return !has_object_file(&oid);
 
        case 'w':
-               if (!path)
-                       die("git cat-file --filters %s: <object> must be "
-                           "<sha1:path>", obj_name);
 
                if (filter_object(path, obj_context.mode,
                                  &oid, &buf, &size))
@@ -122,10 +122,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                break;
 
        case 'c':
-               if (!path)
-                       die("git cat-file --textconv %s: <object> must be <sha1:path>",
-                           obj_name);
-
                if (textconv_object(the_repository, path, obj_context.mode,
                                    &oid, 1, &buf, &size))
                        break;
@@ -618,12 +614,6 @@ static int batch_objects(struct batch_options *opt)
        return retval;
 }
 
-static const char * const cat_file_usage[] = {
-       N_("git cat-file (-t [--allow-unknown-type] | -s [--allow-unknown-type] | -e | -p | <type> | --textconv | --filters) [--path=<path>] <object>"),
-       N_("git cat-file (--batch[=<format>] | --batch-check[=<format>]) [--follow-symlinks] [--textconv | --filters]"),
-       NULL
-};
-
 static int git_cat_file_config(const char *var, const char *value, void *cb)
 {
        if (userdiff_config(var, value) < 0)
@@ -654,90 +644,138 @@ static int batch_option_callback(const struct option *opt,
 int cmd_cat_file(int argc, const char **argv, const char *prefix)
 {
        int opt = 0;
+       int opt_cw = 0;
+       int opt_epts = 0;
        const char *exp_type = NULL, *obj_name = NULL;
        struct batch_options batch = {0};
        int unknown_type = 0;
 
+       const char * const usage[] = {
+               N_("git cat-file <type> <object>"),
+               N_("git cat-file (-e | -p) <object>"),
+               N_("git cat-file (-t | -s) [--allow-unknown-type] <object>"),
+               N_("git cat-file (--batch | --batch-check) [--batch-all-objects]\n"
+                  "             [--buffer] [--follow-symlinks] [--unordered]\n"
+                  "             [--textconv | --filters]"),
+               N_("git cat-file (--textconv | --filters)\n"
+                  "             [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]"),
+               NULL
+       };
        const struct option options[] = {
-               OPT_GROUP(N_("<type> can be one of: blob, tree, commit, tag")),
-               OPT_CMDMODE('t', NULL, &opt, N_("show object type"), 't'),
-               OPT_CMDMODE('s', NULL, &opt, N_("show object size"), 's'),
+               /* Simple queries */
+               OPT_GROUP(N_("Check object existence or emit object contents")),
                OPT_CMDMODE('e', NULL, &opt,
-                           N_("exit with zero when there's no error"), 'e'),
-               OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
-               OPT_CMDMODE(0, "textconv", &opt,
-                           N_("for blob objects, run textconv on object's content"), 'c'),
-               OPT_CMDMODE(0, "filters", &opt,
-                           N_("for blob objects, run filters on object's content"), 'w'),
-               OPT_STRING(0, "path", &force_path, N_("blob"),
-                          N_("use a specific path for --textconv/--filters")),
+                           N_("check if <object> exists"), 'e'),
+               OPT_CMDMODE('p', NULL, &opt, N_("pretty-print <object> content"), 'p'),
+
+               OPT_GROUP(N_("Emit [broken] object attributes")),
+               OPT_CMDMODE('t', NULL, &opt, N_("show object type (one of 'blob', 'tree', 'commit', 'tag', ...)"), 't'),
+               OPT_CMDMODE('s', NULL, &opt, N_("show object size"), 's'),
                OPT_BOOL(0, "allow-unknown-type", &unknown_type,
                          N_("allow -s and -t to work with broken/corrupt objects")),
-               OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
-               OPT_CALLBACK_F(0, "batch", &batch, "format",
-                       N_("show info and content of objects fed from the standard input"),
+               /* Batch mode */
+               OPT_GROUP(N_("Batch objects requested on stdin (or --batch-all-objects)")),
+               OPT_CALLBACK_F(0, "batch", &batch, N_("format"),
+                       N_("show full <object> or <rev> contents"),
                        PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
                        batch_option_callback),
-               OPT_CALLBACK_F(0, "batch-check", &batch, "format",
-                       N_("show info about objects fed from the standard input"),
+               OPT_CALLBACK_F(0, "batch-check", &batch, N_("format"),
+                       N_("like --batch, but don't emit <contents>"),
                        PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
                        batch_option_callback),
+               OPT_CMDMODE(0, "batch-all-objects", &opt,
+                           N_("with --batch[-check]: ignores stdin, batches all known objects"), 'b'),
+               /* Batch-specific options */
+               OPT_GROUP(N_("Change or optimize batch output")),
+               OPT_BOOL(0, "buffer", &batch.buffer_output, N_("buffer --batch output")),
                OPT_BOOL(0, "follow-symlinks", &batch.follow_symlinks,
-                        N_("follow in-tree symlinks (used with --batch or --batch-check)")),
-               OPT_BOOL(0, "batch-all-objects", &batch.all_objects,
-                        N_("show all objects with --batch or --batch-check")),
+                        N_("follow in-tree symlinks")),
                OPT_BOOL(0, "unordered", &batch.unordered,
-                        N_("do not order --batch-all-objects output")),
+                        N_("do not order objects before emitting them")),
+               /* Textconv options, stand-ole*/
+               OPT_GROUP(N_("Emit object (blob or tree) with conversion or filter (stand-alone, or with batch)")),
+               OPT_CMDMODE(0, "textconv", &opt,
+                           N_("run textconv on object's content"), 'c'),
+               OPT_CMDMODE(0, "filters", &opt,
+                           N_("run filters on object's content"), 'w'),
+               OPT_STRING(0, "path", &force_path, N_("blob|tree"),
+                          N_("use a <path> for (--textconv | --filters); Not with 'batch'")),
                OPT_END()
        };
 
        git_config(git_cat_file_config, NULL);
 
        batch.buffer_output = -1;
-       argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);
-
-       if (opt) {
-               if (batch.enabled && (opt == 'c' || opt == 'w'))
-                       batch.cmdmode = opt;
-               else if (argc == 1)
-                       obj_name = argv[0];
-               else
-                       usage_with_options(cat_file_usage, options);
-       }
-       if (!opt && !batch.enabled) {
-               if (argc == 2) {
-                       exp_type = argv[0];
-                       obj_name = argv[1];
-               } else
-                       usage_with_options(cat_file_usage, options);
-       }
-       if (batch.enabled) {
-               if (batch.cmdmode != opt || argc)
-                       usage_with_options(cat_file_usage, options);
-               if (batch.cmdmode && batch.all_objects)
-                       die("--batch-all-objects cannot be combined with "
-                           "--textconv nor with --filters");
-       }
 
-       if ((batch.follow_symlinks || batch.all_objects) && !batch.enabled) {
-               usage_with_options(cat_file_usage, options);
-       }
+       argc = parse_options(argc, argv, prefix, options, usage, 0);
+       opt_cw = (opt == 'c' || opt == 'w');
+       opt_epts = (opt == 'e' || opt == 'p' || opt == 't' || opt == 's');
 
-       if (force_path && opt != 'c' && opt != 'w') {
-               error("--path=<path> needs --textconv or --filters");
-               usage_with_options(cat_file_usage, options);
-       }
+       /* --batch-all-objects? */
+       if (opt == 'b')
+               batch.all_objects = 1;
 
-       if (force_path && batch.enabled) {
-               error("options '--path=<path>' and '--batch' cannot be used together");
-               usage_with_options(cat_file_usage, options);
-       }
+       /* Option compatibility */
+       if (force_path && !opt_cw)
+               usage_msg_optf(_("'%s=<%s>' needs '%s' or '%s'"),
+                              usage, options,
+                              "--path", _("path|tree-ish"), "--filters",
+                              "--textconv");
 
+       /* Option compatibility with batch mode */
+       if (batch.enabled)
+               ;
+       else if (batch.follow_symlinks)
+               usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
+                              "--follow-symlinks");
+       else if (batch.buffer_output >= 0)
+               usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
+                              "--buffer");
+       else if (batch.all_objects)
+               usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
+                              "--batch-all-objects");
+
+       /* Batch defaults */
        if (batch.buffer_output < 0)
                batch.buffer_output = batch.all_objects;
 
-       if (batch.enabled)
+       /* Return early if we're in batch mode? */
+       if (batch.enabled) {
+               if (opt_cw)
+                       batch.cmdmode = opt;
+               else if (opt && opt != 'b')
+                       usage_msg_optf(_("'-%c' is incompatible with batch mode"),
+                                      usage, options, opt);
+               else if (argc)
+                       usage_msg_opt(_("batch modes take no arguments"), usage,
+                                     options);
+
                return batch_objects(&batch);
+       }
+
+       if (opt) {
+               if (!argc && opt == 'c')
+                       usage_msg_optf(_("<rev> required with '%s'"),
+                                      usage, options, "--textconv");
+               else if (!argc && opt == 'w')
+                       usage_msg_optf(_("<rev> required with '%s'"),
+                                      usage, options, "--filters");
+               else if (!argc && opt_epts)
+                       usage_msg_optf(_("<object> required with '-%c'"),
+                                      usage, options, opt);
+               else if (argc == 1)
+                       obj_name = argv[0];
+               else
+                       usage_msg_opt(_("too many arguments"), usage, options);
+       } else if (!argc) {
+               usage_with_options(usage, options);
+       } else if (argc != 2) {
+               usage_msg_optf(_("only two arguments allowed in <type> <object> mode, not %d"),
+                             usage, options, argc);
+       } else if (argc) {
+               exp_type = argv[0];
+               obj_name = argv[1];
+       }
 
        if (unknown_type && opt != 't' && opt != 's')
                die("git cat-file --allow-unknown-type: use with -s or -t");

diff --git a/builtin/stash.c b/builtin/stash.c
index 86cd0b456e7752d7ffdb578ce892ca17c4e9c6a9..9638c56303e8e40d5a57f042b551f571c8ff6363 100644 (file)
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1819,8 +1819,8 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
        else if (!strcmp(argv[0], "save"))
                return !!save_stash(argc, argv, prefix);
        else if (*argv[0] != '-')
-               usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
-                             git_stash_usage, options);
+               usage_msg_optf(_("unknown subcommand: %s"),
+                              git_stash_usage, options, argv[0]);
 
        /* Assume 'stash push' */
        strvec_push(&args, "push");

diff --git a/cache.h b/cache.h
index 281f00ab1b161dc71d0bcdae91222e9429ba0342..149022c82f982fe8f81ff1e11b1db774af40b5b1 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1375,6 +1375,7 @@ struct object_context {
 #define GET_OID_FOLLOW_SYMLINKS 0100
 #define GET_OID_RECORD_PATH     0200
 #define GET_OID_ONLY_TO_DIE    04000
+#define GET_OID_REQUIRE_PATH  010000
 
 #define GET_OID_DISAMBIGUATORS \
        (GET_OID_COMMIT | GET_OID_COMMITTISH | \

diff --git a/object-name.c b/object-name.c
index fdff4601b2c70cc7e4585096534382ddc5024990..92862eeb1ac74f0a3eeaca91ffac9771331bbf22 100644 (file)
--- a/object-name.c
+++ b/object-name.c
@@ -1795,13 +1795,13 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
        const char *cp;
        int only_to_die = flags & GET_OID_ONLY_TO_DIE;
 
-       if (only_to_die)
-               flags |= GET_OID_QUIETLY;
-
        memset(oc, 0, sizeof(*oc));
        oc->mode = S_IFINVALID;
        strbuf_init(&oc->symlink_path, 0);
        ret = get_oid_1(repo, name, namelen, oid, flags);
+       if (!ret && flags & GET_OID_REQUIRE_PATH)
+               die(_("<object>:<path> required, only <object> '%s' given"),
+                   name);
        if (!ret)
                return ret;
        /*
@@ -1932,7 +1932,7 @@ void maybe_die_on_misspelt_object_name(struct repository *r,
 {
        struct object_context oc;
        struct object_id oid;
-       get_oid_with_context_1(r, name, GET_OID_ONLY_TO_DIE,
+       get_oid_with_context_1(r, name, GET_OID_ONLY_TO_DIE | GET_OID_QUIETLY,
                               prefix, &oid, &oc);
 }
 

diff --git a/parse-options.c b/parse-options.c
index a8283037be966596184862aa358bff6fd37194f3..2437ad3bcdd5b6ff06bf3f46fb00a1106958cca3 100644 (file)
--- a/parse-options.c
+++ b/parse-options.c
@@ -1079,3 +1079,16 @@ void NORETURN usage_msg_opt(const char *msg,
        die_message("%s\n", msg); /* The extra \n is intentional */
        usage_with_options(usagestr, options);
 }
+
+void NORETURN usage_msg_optf(const char * const fmt,
+                            const char * const *usagestr,
+                            const struct option *options, ...)
+{
+       struct strbuf msg = STRBUF_INIT;
+       va_list ap;
+       va_start(ap, options);
+       strbuf_vaddf(&msg, fmt, ap);
+       va_end(ap);
+
+       usage_msg_opt(msg.buf, usagestr, options);
+}

diff --git a/parse-options.h b/parse-options.h
index e22846d3b7be06fd2af2f81532633d6d773f0bab..cbeb97b90e37332b38db30f0433107fabba6cd8e 100644 (file)
--- a/parse-options.h
+++ b/parse-options.h
@@ -225,6 +225,16 @@ NORETURN void usage_msg_opt(const char *msg,
                            const char * const *usagestr,
                            const struct option *options);
 
+/**
+ * usage_msg_optf() is like usage_msg_opt() except that the first
+ * argument is a format string, and optional format arguments follow
+ * after the 3rd option.
+ */
+__attribute__((format (printf,1,4)))
+void NORETURN usage_msg_optf(const char *fmt,
+                            const char * const *usagestr,
+                            const struct option *options, ...);
+
 /*
  * Use these assertions for callbacks that expect to be called with NONEG and
  * NOARG respectively, and do not otherwise handle the "unset" and "arg"

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 39382fa1958152ae0cb88edda55cd63c33a15b6e..145eee11df97c9cf63542e49e42f80ecabab1b73 100755 (executable)
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -4,6 +4,98 @@ test_description='git cat-file'
 
 . ./test-lib.sh
 
+test_cmdmode_usage () {
+       test_expect_code 129 "$@" 2>err &&
+       grep "^error:.*is incompatible with" err
+}
+
+for switches in \
+       '-e -p' \
+       '-p -t' \
+       '-t -s' \
+       '-s --textconv' \
+       '--textconv --filters' \
+       '--batch-all-objects -e'
+do
+       test_expect_success "usage: cmdmode $switches" '
+               test_cmdmode_usage git cat-file $switches
+       '
+done
+
+test_incompatible_usage () {
+       test_expect_code 129 "$@" 2>err &&
+       grep -E "^(fatal|error):.*(requires|incompatible with|needs)" err
+}
+
+for opt in --batch --batch-check
+do
+       test_expect_success "usage: incompatible options: --path with $opt" '
+               test_incompatible_usage git cat-file --path=foo $opt
+       '
+done
+
+test_missing_usage () {
+       test_expect_code 129 "$@" 2>err &&
+       grep -E "^fatal:.*required" err
+}
+
+short_modes="-e -p -t -s"
+cw_modes="--textconv --filters"
+
+for opt in $cw_modes
+do
+       test_expect_success "usage: $opt requires another option" '
+               test_missing_usage git cat-file $opt
+       '
+done
+
+for opt in $short_modes
+do
+       test_expect_success "usage: $opt requires another option" '
+               test_missing_usage git cat-file $opt
+       '
+
+       for opt2 in --batch \
+               --batch-check \
+               --follow-symlinks \
+               "--path=foo HEAD:some-path.txt"
+       do
+               test_expect_success "usage: incompatible options: $opt and $opt2" '
+                       test_incompatible_usage git cat-file $opt $opt2
+               '
+       done
+done
+
+test_too_many_arguments () {
+       test_expect_code 129 "$@" 2>err &&
+       grep -E "^fatal: too many arguments$" err
+}
+
+for opt in $short_modes $cw_modes
+do
+       args="one two three"
+       test_expect_success "usage: too many arguments: $opt $args" '
+               test_too_many_arguments git cat-file $opt $args
+       '
+
+       for opt2 in --buffer --follow-symlinks
+       do
+               test_expect_success "usage: incompatible arguments: $opt with batch option $opt2" '
+                       test_incompatible_usage git cat-file $opt $opt2
+               '
+       done
+done
+
+for opt in --buffer \
+       --follow-symlinks \
+       --batch-all-objects
+do
+       test_expect_success "usage: bad option combination: $opt without batch mode" '
+               test_incompatible_usage git cat-file $opt &&
+               test_incompatible_usage git cat-file $opt commit HEAD
+       '
+done
+
 echo_without_newline () {
     printf '%s' "$*"
 }

diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
index eacd49ade636f5be6166e05d4e200b9a324073be..b067983ba1c6adf152131c96a67fc7a709e6666b 100755 (executable)
--- a/t/t8007-cat-file-textconv.sh
+++ b/t/t8007-cat-file-textconv.sh
@@ -19,6 +19,48 @@ test_expect_success 'setup ' '
        GIT_AUTHOR_NAME=Number2 git commit -a -m Second --date="2010-01-01 20:00:00"
 '
 
+test_expect_success 'usage: <bad rev>' '
+       cat >expect <<-\EOF &&
+       fatal: Not a valid object name HEAD2
+       EOF
+       test_must_fail git cat-file --textconv HEAD2 2>actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'usage: <bad rev>:<bad path>' '
+       cat >expect <<-\EOF &&
+       fatal: invalid object name '\''HEAD2'\''.
+       EOF
+       test_must_fail git cat-file --textconv HEAD2:two.bin 2>actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'usage: <rev>:<bad path>' '
+       cat >expect <<-\EOF &&
+       fatal: path '\''two.bin'\'' does not exist in '\''HEAD'\''
+       EOF
+       test_must_fail git cat-file --textconv HEAD:two.bin 2>actual &&
+       test_cmp expect actual
+'
+
+
+test_expect_success 'usage: <rev> with no <path>' '
+       cat >expect <<-\EOF &&
+       fatal: <object>:<path> required, only <object> '\''HEAD'\'' given
+       EOF
+       test_must_fail git cat-file --textconv HEAD 2>actual &&
+       test_cmp expect actual
+'
+
+
+test_expect_success 'usage: <bad rev>:<good (in HEAD) path>' '
+       cat >expect <<-\EOF &&
+       fatal: invalid object name '\''HEAD2'\''.
+       EOF
+       test_must_fail git cat-file --textconv HEAD2:one.bin 2>actual &&
+       test_cmp expect actual
+'
+
 cat >expected <<EOF
 bin: test version 2
 EOF