On 27.08.25 01:38, John Naylor wrote:
> Looks good overall. Some style suggestions:
>
> + print $ofh qq[\t\t]
> + . ($entry->{check_hook} || 'NULL') . qq[, ]
> + . ($entry->{assign_hook} || 'NULL') . qq[, ]
> + . ($entry->{show_hook} || 'NULL') . qq[\n];
>
> The string construction in this script is rather verbose. I'd do something like:
>
> printf $ofh "\t\t%s, %s, %s\n",
> $entry->{check_hook} || 'NULL',
> $entry->{assign_hook} || 'NULL',
> $entry->{show_hook} || 'NULL';
>
> + print $ofh "#ifdef " . $entry->{ifdef} . "\n" if $entry->{ifdef};
>
> Likewise:
>
> print $ofh "#ifdef $entry->{ifdef}\n" if $entry->{ifdef};
>
> + print $ofh qq[\t\t\tgettext_noop("]
> + . escape($entry->{long_desc}) . qq[")];
>
> If the "escape" function was a "quote" function that also did its own
> escaping, there'd be less need for these literal quotes, and so maybe
> no need for the "qq[]"'s here.
Ok, good suggestions. I addressed all those, and did another cleanup
pass over the script. (The formatting is from pgperltidy.)
> + boot_val => '""',
>
> + boot_val => '"ISO, MDY"',
>
> A "quote" function could also insert these for config_string GUCs.
The default values might not be string literals, so writing them out
unquoted and having the script quoting them would not work in general.
So I left this. Maybe this is something to fine-tune in the future.
