This patch series started out as a single patch trying to figure out what it
takes to fix that annoying bug that has been reported several times over the
years, where `git config --unset` would leave empty sections behind, and `git
config --add` would not reuse them.
Little did I know that this would turn not only into a full patch to fix this
issue, but into a full-blown series of nine patches.
The first patch is somewhat of a "while at it" bug fix that I first thought
would be a lot more critical than it actually is: It really only affects config
files that start with a section followed immediately (i.e. without a newline)
by a one-letter boolean setting (i.e. without a `= <value>` part). So while it
is a real bug fix, I doubt anybody ever got bitten by it.
Nonetheless, I would be confortable with this patch going into v2.17.0, even at
this late stage. The final verdict is Junio's, of course.
The next swath of patches add some tests, and adjust one test about which I
already complained at length yesterday, so I will spare you the same ordeal
today. These fixes are pretty straight-forward, and I always try to keep my
added tests as concise as possible, so please tell me if you find a way to make
them smaller (without giving up readability and debuggability).
Finally, the interesting part, where I do two things, essentially (with
preparatory steps for each thing):
1. I add the ability for `git config --unset/--unset-all` to detect that it
can remove a section that has just become empty (see below for some more
discussion of what I consider "become empty"), and
2. I add the ability for `git config [--add] key value` to re-use empty
sections.
Note that the --unset/--unset-all part is the hairy one, and I would love it if
people could concentrate on wrapping their heads around that function, and
obviously tell me how I can change it to make it more readable (or even point
out incorrect behavior).
Now, to the really important part: why does this patch series not conflict with
my very early statements that we cannot simply remove empty sections because we
may end up with stale comments?
Well, the patch in question takes pains to determine *iff* there are any
comments surrounding, or included in, the section. If any are found: previous
behavior. Under the assumption that the user edited the file, we keep it as
intact as possible (see below for some argument against this). If no comments
are found, and let's face it, this is probably *the* common case, as few people
edit their config files by hand these days (neither should they because it is
too easy to end up with an unparseable one), the now-empty section *is*
removed.
So what is the argument against this extra care to detect comments? Well, if
you have something like this:
[section]
; Here we comment about the variable called snarf
snarf = froop
and we run `git config --unset section.snarf`, we end up with this config:
[section]
; Here we comment about the variable called snarf
which obviously does not make sense. However, that is already established
behavior for quite a few years, and I do not even try to think of a way how
this could be solved.
Johannes Schindelin (9):
git_config_set: fix off-by-two
t1300: rename it to reflect that `repo-config` was deprecated
t1300: avoid relying on a bug
t1300: remove unreasonable expectation from TODO
t1300: `--unset-all` can leave an empty section behind (bug)
git_config_set: simplify the way the section name is remembered
git config --unset: remove empty sections (in normal situations)
git_config_set: use do_config_from_file() directly
git_config_set: reuse empty sections
config.c | 234 +++++++++++++++++++++++++---
t/{t1300-repo-config.sh => t1300-config.sh} | 36 ++++-
2 files changed, 250 insertions(+), 20 deletions(-)
rename t/{t1300-repo-config.sh => t1300-config.sh} (98%)
base-commit: 03df4959472e7d4b5117bb72ac86e1e2bcf21723
Published-As: https://github.com/dscho/git/releases/tag/empty-config-section-v1
Fetch-It-Via: git fetch https://github.com/dscho/git empty-config-section-v1
--
2.16.2.windows.1.26.g2cc3565eb4b
