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 <p...@peff.net>
---
I had hoped this would be a quick fix, but it really isn't. I'm happy
enough if somebody wants to try to work on it, but I think the only
clean fix is going to involve a rewrite of the parsing code, so I'm
willing to let it go for now.

Junio, note that this conflicts semantically (but not textually) with
the tip of jc/apply-ws-fix-tab-in-indent, which refactors q_to_tab into
qz_to_tab_space.

 t/t1300-repo-config.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

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
+
+       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
-- 
1.8.2.13.g0f18d3c



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to