From: Junio C Hamano <gitster@pobox•com>
To: Karthik Nayak <karthik.188@gmail•com>
Cc: git@vger•kernel.org, sunshine@sunshineco•com
Subject: Re: [PATCH v8 2/4] cat-file: teach cat-file a '--literally' option
Date: Wed, 15 Apr 2015 13:20:36 -0700 [thread overview]
Message-ID: <xmqqtwwhjg8b.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1429117174-4968-1-git-send-email-karthik.188@gmail.com> (Karthik Nayak's message of "Wed, 15 Apr 2015 22:29:34 +0530")
Karthik Nayak <karthik.188@gmail•com> writes:
> case 's':
> - type = sha1_object_info(sha1, &size);
> - if (type > 0) {
> - printf("%lu\n", size);
> - return 0;
> - }
> - break;
> + oi.sizep = &size;
> + if ((type = sha1_object_info_extended(sha1, &oi, flags)) < 0)
Do you still need to assign to type here?
> + die("git cat-file: could not get object info");
> + printf("%lu\n", size);
> + return 0;
> @@ -323,7 +332,7 @@ static int batch_objects(struct batch_options *opt)
> }
>
> static const char * const cat_file_usage[] = {
> - N_("git cat-file (-t | -s | -e | -p | <type> | --textconv) <object>"),
> + N_("git cat-file (-t [--literally]|-s [--literally]|-e|-p|<type>|--textconv) <object>"),
> N_("git cat-file (--batch | --batch-check) < <list-of-objects>"),
> NULL
> };
> @@ -359,6 +368,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
> int opt = 0;
> const char *exp_type = NULL, *obj_name = NULL;
> struct batch_options batch = {0};
> + int literally = 0;
>
> const struct option options[] = {
> OPT_GROUP(N_("<type> can be one of: blob, tree, commit, tag")),
> @@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
> OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
> OPT_SET_INT(0, "textconv", &opt,
> N_("for blob objects, run textconv on object's content"), 'c'),
> + OPT_BOOL( 0, "literally", &literally,
> + N_("allow -s and -t to work with broken/corrupt objects")),
> { OPTION_CALLBACK, 0, "batch", &batch, "format",
> N_("show info and content of objects fed from the standard input"),
> PARSE_OPT_OPTARG, batch_option_callback },
> @@ -380,7 +392,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
>
> git_config(git_cat_file_config, NULL);
>
> - if (argc != 3 && argc != 2)
> + if (argc < 2 || argc > 4)
> usage_with_options(cat_file_usage, options);
Hmm, why this change?
By allowing 4 args blindly like this, you will let something like
these to pass:
$ git cat-file --textconv -t HEAD
commit
$ git cat-file -p -t HEAD
commit
$ git cat-file -s -t HEAD
commit
$ git cat-file -t -s HEAD
879
It is fine if you are declaring that the last one wins when these
mutually-exclusive options are given. "git cat-file -e -s -t HEAD"
should also say "commit" if that is what you are doing, but instead
the code with this patch complains, which is bad.
It also is fine and is more in line with the spirit of the original
code if you keep the rule tight and only one of these mutually
exclusive options is allowed.
In either case, this check needs to be rethought.
next prev parent reply other threads:[~2015-04-15 20:20 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-15 16:55 [PATCH v8 0/4] cat-file: teach cat-file a '--literally' option karthik nayak
2015-04-15 16:59 ` [PATCH v8 1/4] sha1_file.c: support reading from a loose object of unknown type Karthik Nayak
2015-04-15 20:21 ` Junio C Hamano
2015-04-15 22:18 ` Jeff King
2015-04-17 14:23 ` Jeff King
2015-04-17 16:21 ` Junio C Hamano
2015-04-17 20:51 ` Jeff King
2015-04-17 21:10 ` Junio C Hamano
2015-04-20 18:43 ` karthik nayak
2015-04-20 18:51 ` Jeff King
2015-04-21 11:26 ` karthik nayak
2015-04-21 14:24 ` Jeff King
2015-04-17 18:45 ` karthik nayak
2015-04-17 18:49 ` Jeff King
2015-04-18 8:31 ` karthik nayak
2015-04-17 19:23 ` Junio C Hamano
2015-04-18 8:32 ` karthik nayak
2015-04-17 23:31 ` Eric Sunshine
2015-04-18 9:03 ` karthik nayak
2015-04-15 16:59 ` [PATCH v8 2/4] cat-file: teach cat-file a '--literally' option Karthik Nayak
2015-04-15 20:20 ` Junio C Hamano [this message]
2015-04-15 20:52 ` Junio C Hamano
2015-04-16 7:26 ` karthik nayak
2015-04-16 13:35 ` Junio C Hamano
2015-04-17 2:10 ` Karthik Nayak
2015-04-17 2:14 ` Junio C Hamano
2015-04-19 0:28 ` Charles Bailey
2015-04-20 5:30 ` Junio C Hamano
2015-04-20 7:44 ` Charles Bailey
2015-04-20 8:57 ` Karthik Nayak
2015-04-20 9:19 ` Charles Bailey
2015-04-20 15:52 ` karthik nayak
2015-04-21 10:16 ` Charles Bailey
2015-04-21 19:40 ` Eric Sunshine
2015-04-21 20:36 ` Junio C Hamano
2015-04-25 11:22 ` karthik nayak
2015-04-25 17:04 ` Junio C Hamano
2015-04-27 11:57 ` karthik nayak
2015-04-27 18:38 ` Eric Sunshine
2015-04-28 12:03 ` karthik nayak
2015-04-15 17:00 ` [PATCH v8 3/4] cat-file: add documentation for " Karthik Nayak
2015-04-15 17:00 ` [PATCH v8 4/4] t1006: add tests for git cat-file --literally Karthik Nayak
2015-04-18 0:00 ` Eric Sunshine
2015-04-18 5:22 ` karthik nayak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqtwwhjg8b.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=karthik.188@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox