Re: [PATCH 11/15] scalar: allow reconfiguring an existing enlistment

On Fri, Sep 03 2021, Johannes Schindelin wrote:

Hi Ævar,

On Tue, 31 Aug 2021, Ævar Arnfjörð Bjarmason wrote:

quoted

On Mon, Aug 30 2021, Johannes Schindelin via GitGitGadget wrote:

quoted

This comes in handy during Scalar upgrades, or when config settings were
messed up by mistake.

quoted

[...]
 		const char *key;
 		const char *value;
+		int overwrite_on_reconfigure;

If you make this a "keep_on_reconfigure", then ...

I do not think that this would be a better name, or that renaming this
field would do anything except cause more work for me.

It would also result in more readable code, i.e. why add boilerplate ",
1" to a boolean field in this case if every single setting is set to
"1"? Doesn't it make more sense to invert the variable name & save on
the verbosity?

quoted

quoted

 	} config[] = {
-		{ "am.keepCR", "true" },
-		{ "core.FSCache", "true" },
-		{ "core.multiPackIndex", "true" },
-		{ "core.preloadIndex", "true" },
+		/* Required */
+		{ "am.keepCR", "true", 1 },
+		{ "core.FSCache", "true", 1 },
+		{ "core.multiPackIndex", "true", 1 },
+		{ "core.preloadIndex", "true", 1 },

You won't need the churn/boilerplate of adding "1" to everything here,
but can just change the initial patch to use designated initializers.

That along with a throwaway macro like:

#define SCALAR_CFG_TRUE(k) (.key = k, .value = "true")
#define SCALAR_CFG_FALSE(k) (.key = k, .value = "false")

Might (or might not) make this even easier to eyeball...

To me, it makes things less readable. There is an entire section with the
header `/* Optional */` below, and I want this list to stay as readable as
it is now.

Yeah, I think those macros are probably less readable too. I should have
phrased that as a "one could even...", but just the smaller change of
avoiding the ", 1" everywhere seems worthwhile.