t1300: document some aesthetic failures of the config editor

The config-editing code used by "git config var value" is
built around the regular config callback parser, whose only
triggerable item is an actual key. As a result, it does not
know anything about section headers, which can result in
unnecessarily ugly output:

  1. When we delete the last key in a section, we should be
     able to delete the section header.

  2. When we add a key into a section, we should be able to
     reuse the same section header, even if that section did
     not have any keys in it already.

Unfortunately, fixing these is not trivial with the current
code. It would involve the config parser recording and
passing back information on each item it finds, including
headers, keys, and even comments (or even better, generating
an actual in-memory parse-tree).

Since these behaviors do not cause any functional problems
(i.e., the resulting config parses as expected, it is just
uglier than one would like), fixing them can wait until
somebody feels like substantially refactoring the parsing
code. In the meantime, let's document them as known issues
with some tests.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3c96fda548709835e10f283df8cab667c280af81..c4a7d84f46fd218af1824ac1f78a1e9f4cb15cec 100755 (executable)
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1087,4 +1087,39 @@ 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 &&
+       # some generic comment on the configuration file itself
+       # a comment specific to this "section" section.
+       [section]
+       # some intervening lines
+       # that should also be dropped
+
+       key = value
+       # please be careful when you update the above variable
+       EOF
+
+       cat >expect <<-\EOF &&
+       # some generic comment on the configuration file itself
+       EOF
+
+       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
+'
+
 test_done