Re: [PATCH v3] Teach git to change to a given directory using -C option

[Nazri Ramliy <ayiehere@gmail.com>]

On Wed, Sep 4, 2013 at 6:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I do not care too deeply either way, but I am curious if there was a
> reason why you changed the earlier <directory> to <path>?  Somehow,
> when we _know_ a path has to be a directory, I find it easier on the
> readers to spell that out, instead of saying "this is a path",
> implying that it could be a directory, a regular file, or even
> non-existent.

Eric made me do it :). Personally I prefer the non-ambiguous "directory"
over "path". In fact, "directory" is used in the error message spat out
by git:

    $ git --work-tree
    No directory given for --work-tree.
    ...

    $ git --git-dir
    No directory given for --git-dir.
    ...
    $

I think changing "<path>" to "<directory>" wherever appropriate in
git.txt would be an improvement. Tangent: <dir> is shorter but that
might not be easy on the translators.

>> +-C <path>::
>> +     Run as if git was started in <path> instead of the current working
>> +     directory.  When multiple -C options are given, each subsequent
>> +     non-absolute "-C <path>" is interpreted relative to the preceding "-C
>> +     <path>".
>> +
>> +     This option affects options that expect path name like --git-dir and
>> +     --work-tree in that their interpretations of the path names would be
>> +     made relative to the working directory caused by the -C option. For
>> +     example the following invocations are equivalent:
>> +
>> +         git --git-dir=a.git --work-tree=b -C c status
>> +         git --git-dir=c/a.git --work-tree=c/b status
>> +
>
> Does the above format correctly without the usual "second and
> subsequent paragraphs are not indented, but has '+' in place of
> a blank line"?

No it doesn't. I'll fix it in the next reroll.

>> -             if (!prefixcmp(cmd, "--exec-path")) {
>> +             if (!strcmp(cmd, "-C")) {
>> +                     if (*argc < 2) {
>> +                             fprintf(stderr, "No directory given for -C.\n" );
>> +                             usage(git_usage_string);
>> +                     }
>> +                     if (chdir((*argv)[1]))
>> +                             die_errno("Cannot change to '%s'", (*argv)[1]);
>> +                     if (envchanged)
>> +                             *envchanged = 1;
>> +                     (*argv)++;
>> +                     (*argc)--;
>> +             } else if (!prefixcmp(cmd, "--exec-path")) {
>
> We usually do not prepend to an existing if/else if/ chain unless
> there is a very good reason (e.g. the new "if" condition is very
> often triggered and we are better off checking it early) exactly
> because doing so would make a patch that is ugly like the above.
> You are not touching the codepath that deal with --exec-path, but
> the resulting patch makes it appear as if you are doing something to
> it.

I agree with you here. I'll send the reroll later.

nazri