Re: [PATCH v2] grep: ignore --recurse-submodules if --no-index is given

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

"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Using `--recurse-submodules` should not have any effect if `--no-index`
> is used inside a repository, as Git will recurse into the checked out
> submodule directories just like into regular directories.
> ...
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 50ce8d9461..ae2d5bbafc 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -958,6 +958,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  			/* die the same way as if we did it at the beginning */
>  			setup_git_directory();
>  	}
> +	/* Ignore --recurse-submodules if --no-index is given or implied */
> +	if (!use_index)
> +		recurse_submodules = 0;

This is done quite early in the execution flow.  That makes any and
all existing checks that says "if recurse-submodules is set and we
are in no-index mode, do this" unneeded.

>  
>  	/*
>  	 * skip a -- separator; we know it cannot be

> @@ -1115,8 +1118,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		}
>  	}
>  
> -	if (recurse_submodules && (!use_index || untracked))
> -		die(_("option not supported with --recurse-submodules"));
> +	if (recurse_submodules && untracked)
> +		die(_("--untracked not supported with --recurse-submodules"));

And this is an example of a change to remove a check for such a
redundant condition.  If recurse_submodules is true (which is the
only time the RHS of &&- is evaluated), we know use_index cannot be
false, so the final outcome solely depends on the value of untracked.

Looks good.  And my quick reading of the current builtin/grep.c code
suggests that this is the only such combination that can be simplified.

Thanks.

>  	if (!show_in_pager && !opt.status_only)
>  		setup_pager();
> diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
> index 946f91fa57..828cb3ba58 100755
> --- a/t/t7814-grep-recurse-submodules.sh
> +++ b/t/t7814-grep-recurse-submodules.sh
> @@ -345,7 +345,16 @@ test_incompatible_with_recurse_submodules ()
>  }
>  
>  test_incompatible_with_recurse_submodules --untracked
> -test_incompatible_with_recurse_submodules --no-index
> +
> +test_expect_success 'grep --recurse-submodules --no-index ignores --recurse-submodules' '
> +	git grep --recurse-submodules --no-index -e "^(.|.)[\d]" >actual &&
> +	cat >expect <<-\EOF &&
> +	a:(1|2)d(3|4)
> +	submodule/a:(1|2)d(3|4)
> +	submodule/sub/a:(1|2)d(3|4)
> +	EOF
> +	test_cmp expect actual
> +'
>  
>  test_expect_success 'grep --recurse-submodules should pass the pattern type along' '
>  	# Fixed
>
> base-commit: bc7a3d4dc04dd719e7c8c35ebd7a6e6651c5c5b6