Re: [PATCH] completion: clear cached --options when sourcing the completion script

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Mar 26, 2018, 11:55 UTC
Message-ID: <CAM0VKjnVM6sjkZjQNLtr+rm34=pPfihWzrk6L+r=ky5UZ_JsrA@mail.gmail.com>
In-Reply-To: <xmqq7eq2k840.fsf@gitster-ct.c.googlers.com>

On Fri, Mar 23, 2018 at 6:35 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Junio C Hamano <gitster@pobox.com> writes:

>> I'd say we should just add !GETTEXT_POISON prereq to the problematic
>> tests.

> IOW, this is the minumum required.

Thanks for already committing the fix, I couldn't get around to it.

> By the way, shouldn't we be running the body of these new tests
> inside a subshell?  Otherwise a dot-sourcing by an earlier test of
> these new ones _will_ affect all the subsequent tests.

I don't think it matters. All new tests first initialize the variable they are interested in before sourcing the completion script, to avoid false successes caused by the variable being empty to begin with.

And it shouldn't matter, either, because in the end the users will source the completion script into their main shell process, and it should just work no matter how many times it gets sourced.

Show 16 quoted lines

>  t/t9902-completion.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 4c86adadf2..b7f5b1e632 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1511,7 +1511,7 @@ test_expect_success 'sourcing the completion script clears cached porcelain comm
>         verbose test -z "$__git_porcelain_commands"
>  '
>
> -test_expect_success 'sourcing the completion script clears cached merge strategies' '
> +test_expect_success !GETTEXT_POISON 'sourcing the completion script clears cached merge strategies' '
>         __git_compute_merge_strategies &&
>         verbose test -n "$__git_merge_strategies" &&
>         . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&