Re: [PATCH v2 1/4] git: pass in repo for RUN_SETUP_GENTLY

[Junio C Hamano <gitster@pobox.com>]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> commands that have RUN_SETUP_GENTLY potentially need a repository.
> Modify the logic in run_builtin() to pass the repository to the builtin
> if a builtin has the RUN_SETUP_GENTLY property.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  git.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/git.c b/git.c
> index 2fbea24ec92..f58f169f3c7 100644
> --- a/git.c
> +++ b/git.c
> @@ -443,7 +443,7 @@ static int handle_alias(int *argcp, const char ***argv)
>  
>  static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct repository *repo)
>  {
> -	int status, help;
> +	int status, help, repo_exists;
>  	struct stat st;
>  	const char *prefix;
>  	int run_setup = (p->option & (RUN_SETUP | RUN_SETUP_GENTLY));
> @@ -455,9 +455,13 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
>  
>  	if (run_setup & RUN_SETUP) {
>  		prefix = setup_git_directory();
> +		repo_exists = 1;
>  	} else if (run_setup & RUN_SETUP_GENTLY) {
>  		int nongit_ok;
>  		prefix = setup_git_directory_gently(&nongit_ok);
> +
> +		if (!nongit_ok)
> +			repo_exists = 1;

Why not use the new variable and pass it directly to nongit_ok?  The
polarity of the new variable needs to be swapped, of course, but I
think it makes reading the code to call p->fn() easier to grok

i.e., 

 - rename repo_exists to "no_repo", and initialize it to non-zero.
 - remove "int nongit_ok"; pass &no_repo instead.
 - update the calling of p->fn() to

	p->fn(argc, argv, prefix, no_repo ? NULL : repo);

>  	} else {
>  		prefix = NULL;
>  	}
> @@ -480,7 +484,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
>  	trace2_cmd_name(p->cmd);
>  
>  	validate_cache_entries(repo->index);
> -	status = p->fn(argc, argv, prefix, (p->option & RUN_SETUP)? repo : NULL);
> +	status = p->fn(argc,
> +		       argv,
> +		       prefix,
> +		       repo_exists ? repo : NULL);

Keeping the call to a single line would be much better than spreding
it across four lines.

Thanks

>  	validate_cache_entries(repo->index);
>  
>  	if (status)