Subject: Re: [PATCH v5 2/2] cat-file: teach cat-file a '--literally' option

From: karthik nayak <[email-masked]>
Date: Wed, 25 Mar 2015 13:18:34 +0530

On 03/25/2015 01:12 PM, Eric Sunshine wrote:
> On Wed, Mar 25, 2015 at 3:22 AM, Karthik Nayak <[email-masked]> wrote:
> > Currently 'git cat-file' throws an error while trying to
> > print the type or size of a broken/corrupt object which is
> > created using 'git hash-object --literally'. This is
> > because these objects are usually of unknown types.
> >
> > Teach git cat-file a '--literally' option where it prints
> > the type or size of a broken/corrupt object without throwing
> > an error.
> >
> > Modify '-t' and '-s' options to call sha1_object_info_extended()
> > directly to support the '--literally' option.
> >
> > Helped-by: Junio C Hamano <[email-masked]>
> > Helped-by: Eric Sunshine <sunshine@sunshineco
> > Signed-off-by: Karthik Nayak <[email-masked]>
> > ---
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index df99df4..6fee461 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -405,5 +423,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
> >          if (batch.enabled)
> >                  return batch_objects(&batch);
> >
> > -       return cat_one_file(opt, exp_type, obj_name);
> > +       if (literally && (opt == 't' || opt == 's'))
> > +               return cat_one_file(opt, exp_type, obj_name, literally);
> > +       else if (literally)
> > +               die("git cat-file --literally: use with -s or -t");
> > +
> > +       return cat_one_file(opt, exp_type, obj_name, literally);
>
> Although I haven't read the patch closely yet, this unnecessarily
> complex logic caught my eye as I was scanning the changes. If you
> check for the illegal option combination first, then you can coalesce
> the two identical cat_one_file() invocations, and this entire hunk
> reduces to the more readable:
>
>      if (literally && opt != 't' && opt != 's')
>          die("git cat-file --literally: use with -s or -t");
>
>      return cat_one_file(opt, exp_type, obj_name, literally);
>
> >   }
> > --
> > 2.3.1.170.g5319d60.dirty
Hey Eric,
Yes that could be done.

When small fixes like these need to be done, Is it sufficient to reply to this mail
with the fixed patch, or should I resend the whole patch series?

Thanks,
Karthik