Re: [PATCH] t1300: document some aesthetic failures of the config editor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 3c96fda..d62facb 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -1087,4 +1087,36 @@ test_expect_success 'barf on incomplete string' '
>  	grep " line 3 " error
>  '
>  
> +# good section hygiene
> +test_expect_failure 'unsetting the last key in a section removes header' '
> +	cat >.git/config <<-\EOF &&
> +	[section]
> +	# some intervening lines
> +	# that should be saved
> +	key = value
> +	EOF
> +
> +	cat >expect <<-\EOF &&
> +	# some intervening lines
> +	# that should be saved
> +	EOF

I do not know if I agree with this expectation.

Most likely these comments are about the section, and possibly even
are specific to section.key, not applicable to the section in
general).  If we _were_ to remove the section header at this point,
we should be removing the comment two out of three cases (if it is
about section.key, it should go when section.key goes; if it is
about section, it should go when section goes; if it is a more
generic comment about this configuration file, it should stay).

A better approach may be to only insist on the "when adding, reuse
an empty section header" side of the coin.  Then we do not have to
worry about "we keep cruft that talks about some section but what
the comment says is illegible now the crucial bit of information,
section name the comment talks about, is gone".

> +
> +	git config --unset section.key &&
> +	test_cmp expect .git/config
> +'
> +
> +test_expect_failure 'adding a key into an empty section reuses header' '
> +	cat >.git/config <<-\EOF &&
> +	[section]
> +	EOF
> +
> +	q_to_tab >expect <<-\EOF &&
> +	[section]
> +	Qkey = value
> +	EOF
> +
> +	git config section.key value
> +	test_cmp expect .git/config
> +'

This side I would agree it is unconditionally a good thing to do.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]