Re: [PATCH v4 4/9] versioncmp.c: refactor config reading next commit

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

Refactor the reading of the versionSort.suffix and
versionSort.prereleaseSuffix configuration variables to stay within
the bounds of our CodingGuidelines when it comes to line length, and
to avoid repeating ourselves.

Let's also split out the names of the config variables into variables
of our own, so we don't have to repeat ourselves, 

You do not have to repeat "we don't have to repeat" by mentioning it
twice in two paragraphs.

Moving the "initialized = 1" assignment allows us to move some of this
to the variable declarations in the subsequent commit.

Unclear until looking at these subsequent steps; let's see what
happens next ;-).

 	if (!initialized) {
-		const struct string_list *deprecated_prereleases;
+		const char *const newk = "versionsort.suffix";
+		const char *const oldk = "versionsort.prereleasesuffix";
+		const struct string_list *oldl;

With s/oldl/deprecated_prereleases/ the damage would even be
smaller.  It's not like a more descriptive name in this small scope
hurts line length or readability, is it?

+		prereleases = git_config_get_value_multi(newk);
+		oldl = git_config_get_value_multi(oldk);

+		if (prereleases && oldl)
+			warning("ignoring %s because %s is set", oldk, newk);
+		else if (!prereleases)
+			prereleases = oldl;

This makes it more clear than the original what is going on, even
though they are equivalent.  If we have both, we ignore the
fallback, and if we don't have what we need, we replace it with the
fallback, which could be NULL in which case we end up not having
any.

It is a very nice added bonus that we ended up with a shallow
nesting.