Re: [PATCH v3] Teach git to change to a given directory using -C option
- From
Nazri Ramliy <ayiehere@gmail.com>
- Date
- Sep 4, 2013, 06:36 UTC
- Message-ID
- <CAEY4ZpN4xgt+gJSVeTfDNC--xt3N+M7vVLFBC7FTWBQtjvN8tw@mail.gmail.com>
- In-Reply-To
- <xmqq8uzdplqv.fsf@gitster.dls.corp.google.com>
On Wed, Sep 4, 2013 at 6:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> 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.
Show 18 quoted lines
>> +-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.
Show 21 quoted lines
>> - 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