public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Nazri Ramliy <ayiehere@gmail•com>
Cc: Eric Sunshine <sunshine@sunshineco•com>,
	Git List <git@vger•kernel.org>, Jeff King <peff@peff•net>,
	Jonathan Nieder <jrnieder@gmail•com>
Subject: Re: [PATCH v3] Teach git to change to a given directory using -C option
Date: Tue, 03 Sep 2013 15:46:00 -0700	[thread overview]
Message-ID: <xmqq8uzdplqv.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: 20130903115944.GA29542@gmail.com

Nazri Ramliy <ayiehere@gmail•com> writes:

> -- >8 --
> Subject: [PATCH] Teach git to change to a given directory using -C option
>
> This is similar in spirit to to "make -C dir ..." and "tar -C dir ...".
>
> Currently it takes more effort (keypresses) to invoke git command in a
> different directory than the current one without leaving the current
> directory:
>
>     1. (cd ~/foo && git status)
>        git --git-dir=~/foo/.git --work-dir=~/foo status
>        GIT_DIR=~/foo/.git GIT_WORK_TREE=~/foo git status
>     2. (cd ../..; git grep foo)
>     3. for d in d1 d2 d3; do (cd $d && git svn rebase); done
>
> While doable the methods shown above are arguably more suitable for
> scripting than quick command line invocations.
>
> With this new option, the above can be done with fewer keystrokes:
>
>     1. git -C ~/foo status
>     2. git -C ../.. grep foo
>     3. for d in d1 d2 d3; do git -C $d svn rebase; done
>
> A new test script is added to verify the behavior of this option with
> other path-related options like --git-dir and --work-tree.
>
> Signed-off-by: Nazri Ramliy <ayiehere@gmail•com>
> ---

Thanks; will tentatively queue on 'pu' with some rephrasing of the
log message, but I have a few comments.

>  Documentation/git.txt | 16 +++++++++-
>  git.c                 | 15 ++++++++--
>  t/t0056-git-C.sh      | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 110 insertions(+), 3 deletions(-)
>  create mode 100755 t/t0056-git-C.sh
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 83edf30..6105cb0 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -9,7 +9,7 @@ git - the stupid content tracker
>  SYNOPSIS
>  --------
>  [verse]
> -'git' [--version] [--help] [-c <name>=<value>]
> +'git' [--version] [--help] [-C <path>] [-c <name>=<value>]

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.

>      [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
>      [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
>      [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
> @@ -395,6 +395,20 @@ displayed. See linkgit:git-help[1] for more information,
>  because `git --help ...` is converted internally into `git
>  help ...`.
>  
> +-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"?

> diff --git a/git.c b/git.c
> index 2025f77..52bce74 100644
> --- a/git.c
> +++ b/git.c
> @@ -7,7 +7,7 @@
>  #include "commit.h"
>  
>  const char git_usage_string[] =
> -	"git [--version] [--help] [-c name=value]\n"
> +	"git [--version] [--help] [-C <path>] [-c name=value]\n"
>  	"           [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n"
>  	"           [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]\n"
>  	"           [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]\n"
> @@ -54,7 +54,18 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>  		/*
>  		 * Check remaining flags.
>  		 */
> -		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.

  reply	other threads:[~2013-09-03 22:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-03 11:59 [PATCH v3] Teach git to change to a given directory using -C option Nazri Ramliy
2013-09-03 22:46 ` Junio C Hamano [this message]
2013-09-03 23:37   ` Eric Sunshine
2013-09-04  6:36   ` Nazri Ramliy
2013-09-04  6:45     ` Eric Sunshine
2013-09-04 12:20     ` Nazri Ramliy
2013-09-08 10:32       ` Eric Sunshine
2013-09-09  1:49         ` Nazri Ramliy
2013-09-09  5:01           ` Eric Sunshine
2013-09-09 13:47             ` Nazri Ramliy
2013-09-09 16:32               ` Junio C Hamano
2013-09-09 17:42                 ` Eric Sunshine
2013-09-09 17:59                   ` Junio C Hamano
2013-09-09 18:43                 ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq8uzdplqv.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=ayiehere@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jrnieder@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox