On Fri, Apr 19, 2013 at 08:21:48PM +0800, Nazri Ramliy wrote:

> This is similar in spirit to to "make -C dir ..." and "tar -C dir ...".
> 
> Signed-off-by: Nazri Ramliy <ayieh...@gmail.com>
> ---
> Often I find myself needing to find out quickly the status of a repository 
> that
> is not in my currenct working directory, like this:
> 
>          $ (cd ~/foo; git log -1)
> 
> With this patch now i can simply do:
> 
>          $ git -C ~/.zsh log -1 
> 
> That's just one example. I think those who are familiar with the -C arguments
> to "make" and "tar" commands would get the "handiness" of having this option 
> in
> git.

This motivation should probably go into the commit message.

I think it's worth pausing for a moment and considering if we can do
this already with existing features.

You can _almost_ do this with "git --git-dir". But it expects the actual
git directory, not a starting point for finding the git directory. And
it remains in your same working dir. So with a bare repository, these
two are equivalent:

  $ git --git-dir=/path/to/foo.git ...
  $ git -C /path/to/foo.git ...

But with a non-bare repo, this does not work:

  $ git --git-dir=/path/to/non-bare ...

You must instead say:

  $ git --git-dir=/path/to/non-bare/.git ...

and even then, I think it will treat your current directory as the
working tree, not /path/to/non-bare.

So I think "-C" is a worthwhile addition compared to just "--git-dir".

It is redundant with "(cd foo && git ...)" in the shell, as you note,
but sometimes it is more convenient to use "-C" (especially if you are
exec-ing git from another program and want to avoid the shell entirely
for quoting reasons).

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 6a875f2..20bba86 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -379,6 +379,9 @@ displayed. See linkgit:git-help[1] for more information,
>  because `git --help ...` is converted internally into `git
>  help ...`.
>  
> +-C <directory>::
> +     Change to given directory before doing anything else.
> +

It might make sense to clarify this as "...anything else, including
determining the location of the git repository directory". If you think
hard about it, doing anything else would not really make much sense, but
spelling it out makes it clear what the option can be used for.

> +             if (!prefixcmp(cmd, "-C")) {

Should this be strcmp? You do not seem to handle "-Cfoo" below.

> +                     if (*argc < 2) {
> +                             fprintf(stderr, "No directory given for -C.\n" 
> );
> +                             usage(git_usage_string);
> +                     }

I know you are copying this from the other options in the same function,
but I wonder if they should all be calling "error()" (and dropping the
terminating ".") to better match our usual error messages.

> +                     if (chdir((*argv)[1]))
> +                             die_errno("Cannot change to '%s'", (*argv)[1]);
> +                     (*argv)++;
> +                     (*argc)--;

You would want to set "*envchanged = 1" here. The intent of that flag is
that git would need to throw away things it has looked up already (like
the git dir) in order to correctly utilize the options (and since we
haven't implemented that "throw away" step, it just complains and dies).

I didn't try it, but I suspect your patch would be broken with:

  $ git config alias.logfoo '-C /path/to/foo log'
  $ cd /some/other/repo
  $ git logfoo

It would still use /some/other/repo as a $GIT_DIR, having looked it up
before processing the "-C".

-Peff
--
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