On Wed, Sep 19, 2012 at 12:36:56PM -0700, Junio C Hamano wrote:

> > Like this (totally untested) patch:
> >
> > diff --git a/builtin/blame.c b/builtin/blame.c
> > index 0e102bf..412d6dd 100644
> > --- a/builtin/blame.c
> > +++ b/builtin/blame.c
> > @@ -2365,6 +2365,10 @@ int cmd_blame(int argc, const char **argv, const 
> > char *prefix)
> >                     ctx.argv[0] = "--children";
> >                     reverse = 1;
> >             }
> > +           else if (!strcmp(ctx.argv[0], "--follow")) {
> > +                   error("unknown option `--follow`");
> > +                   usage_with_options(blame_opt_usage, options);
> > +           }
> >             parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
> >     }
> >  parse_done:
> This patch would not hurt existing users very much; blame is an
> unlikely thing to run in scripts, and it is easy to remove the
> misguided --follow from them.

I would not worry about such users. I am of the opinion that their
scripts are buggy for calling a useless and undocumented option that
just happened to not complain.

> So I am in general OK with it, but if we are to go that route, we
> should make sure that the documentation makes it clear that blame
> follows whole-file renames without any special instruction before
> doing so.  Otherwise, it again will send the same wrong message to
> people who try to use the "--follow" from their experience with
> "log", no?

I guess it depends on your perspective. I can see the argument that
blame is already doing what --follow would ask for, and thus it is a
no-op. I think of it more as --follow is nonsensical for blame. But I
do not think either is wrong per se, and there is no reason not to help
people who come to git thinking the former. So yes, I think
documentation in either case is probably a good thing.

I am a little lukewarm on my patch if only because of the precedent it
sets.  There are a trillion options that revision.c parses that are not
necessarily meaningful or implemented for sub-commands that piggy-back
on its option parser. I'm not sure we want to get into manually
detecting and disallowing each one in every caller.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to