On Fri, Sep 03 2021, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Tue, 31 Aug 2021, Ævar Arnfjörð Bjarmason wrote:
>
>>
>> On Mon, Aug 30 2021, Johannes Schindelin via GitGitGadget wrote:
>>
>> > This comes in handy during Scalar upgrades, or when config settings were
>> > messed up by mistake.
>>
>> > [...]
>> > 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?
>>
>> > } 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.
