From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Thomas Rast <trast@student.ethz.ch>,
Junio C Hamano <gitster@pobox.com>,
Phil Haack <haacked@gmail.com>
Subject: [PATCH] t1300: document some aesthetic failures of the config editor
Date: Fri, 29 Mar 2013 13:50:58 -0400 [thread overview]
Message-ID: <20130329175058.GA13506@sigill.intra.peff.net> (raw)
In-Reply-To: <20130329172307.GA11099@sigill.intra.peff.net>
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>
---
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
next prev parent reply other threads:[~2013-03-29 17:51 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-29 16:29 Minor bug in git branch --set-upstream-to adding superfluous branch section to config Phil Haack
2013-03-29 17:00 ` Jeff King
2013-03-29 17:20 ` Thomas Rast
2013-03-29 17:23 ` Jeff King
2013-03-29 17:50 ` Jeff King [this message]
2013-03-29 18:51 ` [PATCH] t1300: document some aesthetic failures of the config editor Junio C Hamano
2013-03-29 19:51 ` Jeff King
2013-03-29 20:35 ` Junio C Hamano
2013-03-30 0:21 ` Jeff King
2018-03-28 16:33 ` Johannes Schindelin
2018-03-28 17:59 ` Jeff King
2013-03-29 20:00 ` Junio C Hamano
2013-03-29 17:27 ` Minor bug in git branch --set-upstream-to adding superfluous branch section to config Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130329175058.GA13506@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=haacked@gmail.com \
--cc=trast@student.ethz.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).