Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 21, 2016, 18:15 UTC
Message-ID: <xmqqbmx8k8c0.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <f47dce15719954d6d4d8a550856757366871143e.1479737858.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 12 quoted lines

> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d8..202ac07 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,6 +432,13 @@ test_expect_success '-c with changed comment char' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_failure '-c with comment char defined in .git/config' '
> +	test_config core.commentchar = &&
> +	printf "= foo\n" >expect &&
> +	printf "foo" | git stripspace -c >actual &&

We'd want "\n" on this printf to match the one before as well, as this test is not about "does stripspace complete an incomplete line?", I think.

I could amend it while queuing, but I need to know if I am missing a reason why this must be an incomplete line before doing so.

> +	test_cmp expect actual
> +'
> +

Is this a recent regression? When applied on top of 'maint' or older, it seems to pass just fine.

    ... Goes and looks ...

Interesting. Peff's b9605bc4f2 ("config: only read .git/config from configured repos", 2016-09-12) is where this starts failing, which is understandable given the code change to builtin/stripspace.c in [2/3].

The analysis of the log message in [2/3] is wrong and needs updating, though. In the old world order it worked by accident to call git_config() without calling setup_git_directory(); after b9605bc4f2, that no longer is valid and is exposed as a bug.

Your [2/3] is a good fix for that change.

In any case, well spotted.

Show 20 quoted lines

>  test_expect_success 'avoid SP-HT sequence in commented line' '
>  	printf "#\tone\n#\n# two\n" >expect &&
>  	printf "\tone\n\ntwo\n" | git stripspace -c >actual &&
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index e38e296..e080399 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -976,6 +976,18 @@ test_expect_success 'rebase -i respects core.commentchar' '
>  	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
>  '
>  
> +test_expect_failure 'rebase -i respects core.commentchar=auto' '
> +	test_config core.commentchar auto &&
> +	write_script copy-edit-script.sh <<-\EOF &&
> +	cp "$1" edit-script
> +	EOF
> +	test_set_editor "$(pwd)/copy-edit-script.sh" &&
> +	test_when_finished "git rebase --abort || :" &&
> +	git rebase -i HEAD^ &&
> +	grep "^#" edit-script &&

This was added for debugging that was forgotten?

> +	test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)"

This says "There shouldn't be any line left once we remove '#'-commented lines, empty lines and pick insns.". OK.

The correction in [3/3] seems good.

Show 5 quoted lines

> +'
> +
>  test_expect_success 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
>  	test_when_finished "git branch -D torebase" &&
>  	git checkout -b torebase branch1 &&