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

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

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

> 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.

>  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.

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